fix(client create): never mint over a live cluster — fail closed on unreadable discovery

internal/cli/client.go

@@ -465,13 +465,28 @@ func adoptLiveInClusterClient(
 ) (*api.ProvisionedClient, bool, error) {
 	live, err := readInClusterClient(ctx, cluster.KubeconfigOptions{Path: opts.kubeconfigPath, Context: opts.contextOverride})
 	if err != nil {
-		// Best-effort: couldn't inspect the cluster for a live client. Fall through
-		// to a plain create (the backend's cluster_id get-or-create still applies).
-		ilog.Logf("in-cluster client discovery failed (non-fatal): %v", err)
+		// We couldn't inspect the cluster for an existing client. Whether that's
+		// safe to ignore depends on reachability: clusterID != "" means we DID read
+		// the cluster's kube-system UID over the same kubeconfig, so the cluster is
+		// reachable and this is an RBAC/transient read failure — NOT proof it's
+		// empty. Minting here could create a duplicate over a live client that then
+		// never deploys and permanently strands the cluster anchor (the phantom-1060
+		// class). Fail closed. Only a genuinely unreachable cluster (clusterID == "",
+		// where the UID read failed too) falls through to a plain, non-anchored
+		// create — that mint stamps no anchor, so it can't orphan one.
+		if clusterID != "" {
+			ilog.Logf("in-cluster client discovery failed on a reachable cluster (failing closed): %v", err)
+			return nil, false, &exitError{code: 1, err: fmt.Errorf(
+				"couldn't check whether a tracebloc client is already running on this cluster (%w) — "+
+					"provisioning now could mint a duplicate that never deploys and locks the cluster to it. "+
+					"Re-run (if this was transient); if it persists, ensure your kubeconfig/context can list "+
+					"deployments and secrets across namespaces. Diagnose with `tracebloc cluster doctor`", err)}
+		}
+		ilog.Logf("in-cluster client discovery skipped — cluster unreachable (non-fatal): %v", err)
 		return nil, false, nil
 	}
 	if live == nil {
-		return nil, false, nil // fresh cluster — nothing installed to adopt
+		return nil, false, nil // reachable, nothing installed to adopt — a genuine fresh cluster
 	}
 	ilog.Logf("live in-cluster client: id=%s namespace=%s", live.ClientID, live.Namespace)
 

internal/cli/client_test.go

@@ -288,6 +288,67 @@ func TestClientCreate_R7_CrossAccountRefuse(t *testing.T) {
 	}
 }
 
+// TestClientCreate_R7_DiscoveryErrorReachableFailsClosed: the cluster is REACHABLE
+// (its kube-system UID read cleanly, clusterID != "") but in-cluster client discovery
+// ERRORS (RBAC/transient List failure). We can't tell whether a client is already
+// running, so minting would risk a duplicate that never deploys and strands the
+// cluster anchor (the phantom-1060 class). Must fail closed — no mint, no backfill.
+func TestClientCreate_R7_DiscoveryErrorReachableFailsClosed(t *testing.T) {
+	withClientBackend(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/edge-device/":
+			_, _ = w.Write([]byte(`[]`)) // account list (fetched before adopt) is allowed
+		default:
+			t.Errorf("unexpected %s %s — must fail closed before any mint/backfill", r.Method, r.URL.Path)
+		}
+	})
+	stubClusterID(t, "uid-9", nil) // cluster reachable
+	stubInClusterClient(t, nil, errors.New("forbidden: cannot list deployments"))
+
+	err := runClientCreate(context.Background(), ui.New(&bytes.Buffer{}), nil,
+		clientCreateOpts{name: "box", location: "DE", yes: true})
+	if err == nil || !strings.Contains(err.Error(), "couldn't check whether a tracebloc client is already running") {
+		t.Errorf("want fail-closed error, got %v", err)
+	}
+}
+
+// TestClientCreate_DiscoveryErrorUnreachableMintsNonAnchored: when the cluster is
+// genuinely UNREACHABLE (the UID read failed too → clusterID == ""), a discovery
+// error is not proof a client is running, and a non-anchored mint stamps no anchor
+// so it can't orphan one. Provisioning must still proceed (the deliberate no-cluster
+// fallback), minting with an empty cluster_id. Guards against over-tightening the
+// fail-closed gate into the legitimate headless path.
+func TestClientCreate_DiscoveryErrorUnreachableMintsNonAnchored(t *testing.T) {
+	var body api.CreateClientRequest
+	postCalled := false
+	withClientBackend(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/edge-device/":
+			_, _ = w.Write([]byte(`[]`))
+		case r.Method == http.MethodPost && r.URL.Path == "/edge-device/":
+			postCalled = true
+			_ = json.NewDecoder(r.Body).Decode(&body)
+			w.WriteHeader(http.StatusCreated)
+			_, _ = w.Write([]byte(`{"id":8,"first_name":"box","username":"u-8","namespace":"box","location":"DE"}`))
+		default:
+			t.Errorf("unexpected %s %s", r.Method, r.URL.Path)
+		}
+	})
+	stubClusterID(t, "", errors.New("no cluster reachable"))
+	stubInClusterClient(t, nil, errors.New("no cluster reachable"))
+
+	if err := runClientCreate(context.Background(), ui.New(&bytes.Buffer{}), nil,
+		clientCreateOpts{name: "box", location: "DE", yes: true}); err != nil {
+		t.Fatalf("unreachable-cluster provisioning must still mint (non-anchored): %v", err)
+	}
+	if !postCalled {
+		t.Fatal("expected a non-anchored mint when the cluster is unreachable")
+	}
+	if body.ClusterID != "" {
+		t.Errorf("cluster_id = %q, want empty (non-anchored mint)", body.ClusterID)
+	}
+}
+
 func TestClientCreate_RequiresLogin(t *testing.T) {
 	t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // no config → not signed in
 	err := runClientCreate(context.Background(), ui.New(&bytes.Buffer{}), nil, clientCreateOpts{name: "x", location: "DE", yes: true})

internal/cluster/discover.go

@@ -297,15 +297,25 @@ const clientChartSelector = "app.kubernetes.io/name=client,app.kubernetes.io/man
 // carries the same CLIENT_ID under the same labels.
 //
 // This anchors R7 adopt-backfill: a live client whose backend cluster_id is null
-// must be adopted (and its anchor backfilled), never re-minted. Best-effort — it
-// returns (nil, nil) when nothing is installed or the cluster can't be read
-// (unreachable / restricted RBAC), so callers fall back to a plain create.
+// must be adopted (and its anchor backfilled), never re-minted.
+//
+// Return contract (deliberately three-valued, so callers can tell "empty" from
+// "couldn't tell" — collapsing the two is what let `client create` mint a
+// duplicate over a live client and orphan it, the phantom-1060 class):
+//   - (client, nil) — a live client was found;
+//   - (nil, nil)    — the cluster is READABLE and genuinely has no client release;
+//   - (nil, err)    — a read/RBAC error meant we could NOT determine either way.
+//
+// Callers must fail closed on the error case, never treat it as "nothing installed".
 func DiscoverInClusterClientID(ctx context.Context, cs kubernetes.Interface) (*InClusterClient, error) {
 	deps, err := cs.AppsV1().Deployments(metav1.NamespaceAll).List(ctx, metav1.ListOptions{
 		LabelSelector: clientChartSelector,
 	})
 	if err != nil {
-		return nil, nil // best-effort: treat an unreadable cluster as "nothing installed"
+		// A reachable-but-unreadable cluster must NOT be reported as "nothing
+		// installed" — that ambiguity is exactly what let a duplicate be minted
+		// over a live client. Surface it so the caller fails closed.
+		return nil, fmt.Errorf("listing client deployments to check for an existing client: %w", err)
 	}
 	ns := ""
 	for _, d := range deps.Items {
@@ -315,20 +325,23 @@ func DiscoverInClusterClientID(ctx context.Context, cs kubernetes.Interface) (*I
 		}
 	}
 	if ns == "" {
-		return nil, nil // no client release on this cluster
+		return nil, nil // readable, no client release installed — a genuine fresh cluster
 	}
 	secrets, err := cs.CoreV1().Secrets(ns).List(ctx, metav1.ListOptions{
 		LabelSelector: clientChartSelector,
 	})
 	if err != nil {
-		return nil, nil
+		return nil, fmt.Errorf("reading the existing client's identity in namespace %q: %w", ns, err)
 	}
 	for _, s := range secrets.Items {
 		if v, ok := s.Data["CLIENT_ID"]; ok && len(v) > 0 {
 			return &InClusterClient{ClientID: string(v), Namespace: ns}, nil
 		}
 	}
-	return nil, nil
+	// A client release IS installed here (its jobs-manager Deployment exists) but its
+	// CLIENT_ID secret wasn't readable — we know a client is present, so this is not a
+	// fresh cluster. Fail closed rather than let the caller mint over it.
+	return nil, fmt.Errorf("a tracebloc client is installed in namespace %q but its CLIENT_ID could not be read", ns)
 }
 
 // pickJobsManagerService probes for the chart's jobs-manager

internal/cluster/discover_test.go

@@ -9,7 +9,9 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
+	k8stesting "k8s.io/client-go/testing"
 )
 
 // jobsManagerDeployment builds the minimal Deployment the chart
@@ -135,10 +137,42 @@ func TestDiscoverInClusterClientID_NoRelease(t *testing.T) {
 }
 
 func TestDiscoverInClusterClientID_ReleaseButNoSecret(t *testing.T) {
+	// A release IS installed (jobs-manager present) but its CLIENT_ID secret is
+	// absent/unreadable: we KNOW a client is here, so this must NOT read as "nothing
+	// installed". It returns an error so the caller fails closed rather than mint a
+	// duplicate over the live client (phantom-1060 class).
 	cs := fake.NewClientset(jobsManagerDeployment("tracebloc", "tracebloc", "client-1.3.5", "1.3.5", "d"))
 	got, err := DiscoverInClusterClientID(context.Background(), cs)
-	if err != nil || got != nil {
-		t.Errorf("release but no secret: want (nil,nil), got (%+v,%v)", got, err)
+	if err == nil || got != nil {
+		t.Errorf("release but unreadable CLIENT_ID: want (nil, error), got (%+v, %v)", got, err)
+	}
+}
+
+func TestDiscoverInClusterClientID_DeploymentsListError_FailsClosed(t *testing.T) {
+	// A reachable-but-unreadable cluster (RBAC/transient List failure) must NOT be
+	// reported as (nil,nil) "nothing installed" — that ambiguity is what let a
+	// duplicate be minted over a live client. Surface an error so the caller fails
+	// closed. Regression guard for the phantom-1060 root cause.
+	cs := fake.NewClientset()
+	cs.PrependReactor("list", "deployments", func(k8stesting.Action) (bool, runtime.Object, error) {
+		return true, nil, errors.New("forbidden: cannot list deployments")
+	})
+	got, err := DiscoverInClusterClientID(context.Background(), cs)
+	if err == nil || got != nil {
+		t.Errorf("deployments list error: want (nil, error), got (%+v, %v)", got, err)
+	}
+}
+
+func TestDiscoverInClusterClientID_SecretsListError_FailsClosed(t *testing.T) {
+	// A release is present but the secret read fails — still "couldn't determine",
+	// so return an error (fail closed), never (nil,nil).
+	cs := fake.NewClientset(jobsManagerDeployment("tracebloc", "tracebloc", "client-1.3.5", "1.3.5", "d"))
+	cs.PrependReactor("list", "secrets", func(k8stesting.Action) (bool, runtime.Object, error) {
+		return true, nil, errors.New("forbidden: cannot list secrets")
+	})
+	got, err := DiscoverInClusterClientID(context.Background(), cs)
+	if err == nil || got != nil {
+		t.Errorf("secrets list error: want (nil, error), got (%+v, %v)", got, err)
 	}
 }