diff --git a/internal/cli/client.go b/internal/cli/client.go index 481ddf7f..c4d06a3f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "net/http" + "os" + "path/filepath" "strconv" "strings" @@ -39,7 +41,7 @@ in your account. Requires sign-in first (` + "`tracebloc login`" + `).`, } func newClientCreateCmd() *cobra.Command { - var name, location, kubeconfigPath, contextOverride string + var name, location, kubeconfigPath, contextOverride, credentialFile string var yes bool cmd := &cobra.Command{ Use: "create", @@ -47,7 +49,7 @@ func newClientCreateCmd() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { return runClientCreate(cmd.Context(), printerFor(cmd), clientPrompter(), - clientCreateOpts{name: name, location: location, kubeconfigPath: kubeconfigPath, contextOverride: contextOverride, yes: yes}) + clientCreateOpts{name: name, location: location, kubeconfigPath: kubeconfigPath, contextOverride: contextOverride, credentialFile: credentialFile, yes: yes}) }, } cmd.Flags().StringVar(&name, "name", "", @@ -58,14 +60,16 @@ func newClientCreateCmd() *cobra.Command { "path to the kubeconfig for the target cluster (default: $KUBECONFIG, then ~/.kube/config) — read to anchor the client to this cluster") cmd.Flags().StringVar(&contextOverride, "context", "", "kubeconfig context for the target cluster (default: current-context)") + cmd.Flags().StringVar(&credentialFile, "credential-file", "", + "write the machine credential to this path (mode 0600, sourceable env) instead of printing it — for the installer to feed the chart (never shown on the terminal)") cmd.Flags().BoolVar(&yes, "yes", false, "skip the confirmation prompt") return cmd } // clientCreateOpts bundles the `client create` inputs (flags + resolved prompts). type clientCreateOpts struct { - name, location, kubeconfigPath, contextOverride string - yes bool + name, location, kubeconfigPath, contextOverride, credentialFile string + yes bool } func newClientListCmd() *cobra.Command { @@ -225,6 +229,20 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // installer's orchestration — #838 — not done here.) p.Successf("This cluster is already registered as client %q (namespace %s) — adopted it.", pc.Name, pc.Namespace) p.Hintf("No new credential issued; the existing one stands. This machine is set to enroll as client %d.", pc.ID) + if opts.credentialFile != "" { + // No password to hand over on adopt (it's write-only on the backend and + // the existing one stands). Emit id + namespace + an ADOPTED marker so the + // installer reconciles the existing release rather than expecting a fresh + // credential (#838). + if werr := writeClientCredential(opts.credentialFile, []string{ + "TRACEBLOC_CLIENT_ID=" + strconv.Itoa(pc.ID), + "TB_NAMESPACE=" + pc.Namespace, + "TRACEBLOC_CLIENT_ADOPTED=1", + }); werr != nil { + return &exitError{code: 1, err: werr} + } + p.Hintf("Wrote client id + namespace to %s (no new credential — the existing one stands).", opts.credentialFile) + } // Mirror the mint path: a config-save failure shouldn't bury the result — // hint how to set the pointer by hand and still exit clean. if serr := cfg.Save(); serr != nil { @@ -232,19 +250,80 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien } return nil } - // Mint: print the credential FIRST — it's the only copy (the backend stores - // only the hash), so a later config-save failure must never cost it. + // Mint. With --credential-file the secret goes to a 0600 file (never the + // terminal — RFC §9 "secure by invisibility") for the installer to source; + // otherwise it's printed (the interim, until the installer drives this). p.Successf("Provisioned client %q (namespace %s).", pc.Name, pc.Namespace) - p.Section("Machine credential — needed by the installer to connect this client") - p.Field("client id", strconv.Itoa(pc.ID)) - p.Field("username", pc.Username) - p.Field("password", password) + if opts.credentialFile != "" { + if werr := writeClientCredential(opts.credentialFile, []string{ + "TRACEBLOC_CLIENT_ID=" + strconv.Itoa(pc.ID), + "TRACEBLOC_CLIENT_PASSWORD=" + password, + "TB_NAMESPACE=" + pc.Namespace, + }); werr != nil { + // The credential is the only copy — a write failure must be fatal, not a + // silent drop (the installer would have nothing to connect with). + return &exitError{code: 1, err: werr} + } + p.Hintf("Credential written to %s (mode 0600, not shown). This machine is set to enroll as client %d.", opts.credentialFile, pc.ID) + } else { + // Print the credential FIRST — it's the only copy (the backend stores only + // the hash), so a later config-save failure must never cost it. + p.Section("Machine credential — needed by the installer to connect this client") + p.Field("client id", strconv.Itoa(pc.ID)) + p.Field("username", pc.Username) + p.Field("password", password) + } if serr := cfg.Save(); serr != nil { p.Hintf("Couldn't save the active-client pointer (%v) — run `tracebloc client use %d` to set it.", serr, pc.ID) } return nil } +// writeClientCredential writes the machine credential to path (mode 0600) as a +// shell-sourceable env file — the installer (#838) sources it to feed the chart, +// so the secret lands in a 0600 file, never the terminal (RFC §9 never-show). The +// values are constrained charsets (numeric id, hex password, DNS-1123 slug), so +// no shell-escaping is needed. +// +// Written via a 0600 temp file + atomic rename rather than os.WriteFile: WriteFile +// only applies its perm bits when it *creates* the file, so a pre-existing target +// (a stale file, or one an attacker pre-creates world-readable) would keep its old +// mode and leak the secret — the 0600 guarantee must hold unconditionally. The temp +// also avoids following a symlink at the target and never leaves a half-written +// credential behind. +func writeClientCredential(path string, lines []string) error { + dir := filepath.Dir(path) + if dir != "" && dir != "." { + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("creating credential-file directory: %w", err) + } + } + body := "# tracebloc client credential — written by `tracebloc client create`.\n" + + "# Mode 0600; sourced by the installer. Do not commit or share.\n" + + strings.Join(lines, "\n") + "\n" + // CreateTemp makes the file 0600 by construction, in the target dir so the + // rename stays on one filesystem. + f, err := os.CreateTemp(dir, ".cred-*") + if err != nil { + return fmt.Errorf("writing credential file %s: %w", path, err) + } + tmp := f.Name() + if _, err := f.WriteString(body); err != nil { + _ = f.Close() + _ = os.Remove(tmp) + return fmt.Errorf("writing credential file %s: %w", path, err) + } + if err := f.Close(); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("writing credential file %s: %w", path, err) + } + if err := os.Rename(tmp, path); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("writing credential file %s: %w", path, err) + } + return nil +} + // askAnAdmin renders the "you can't provision — here's who can" path (a 403 from // the backend means no CLIENT_WRITE; backend#836 Q4). func askAnAdmin(ctx context.Context, p *ui.Printer, client *api.Client) error { diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index 5d77cb7b..bd79021d 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -8,6 +8,8 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -325,6 +327,155 @@ func TestClientCreate_AdoptIdempotent(t *testing.T) { } } +func TestClientCreate_CredentialFileMint(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet: + _, _ = w.Write([]byte(`[]`)) + case r.Method == http.MethodPost: + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","username":"u-5","namespace":"my-ns","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + credPath := filepath.Join(t.TempDir(), "cred.env") + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "c", location: "DE", yes: true, credentialFile: credPath}); err != nil { + t.Fatalf("create: %v", err) + } + // never-show: the secret must NOT hit the terminal. + if strings.Contains(out.String(), "Machine credential") || strings.Contains(out.String(), "password") { + t.Errorf("credential must not be printed when --credential-file is set, got:\n%s", out.String()) + } + // the file is 0600 and carries the sourceable credential. + info, err := os.Stat(credPath) + if err != nil { + t.Fatalf("credential file not written: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("credential file mode = %o, want 600", perm) + } + kv := parseEnvFile(t, credPath) + if kv["TRACEBLOC_CLIENT_ID"] != "5" || kv["TB_NAMESPACE"] != "my-ns" || kv["TRACEBLOC_CLIENT_PASSWORD"] == "" { + t.Errorf("credential file = %v (want id=5, ns=my-ns, non-empty password)", kv) + } + // never-show, the real invariant: the minted password VALUE must not appear + // in stdout under any label (the string checks above are just a proxy). + if strings.Contains(out.String(), kv["TRACEBLOC_CLIENT_PASSWORD"]) { + t.Errorf("minted password leaked to the terminal:\n%s", out.String()) + } +} + +// TestClientCreate_CredentialFilePreexistingPerms locks in the 0600 guarantee +// when the target already exists with looser perms — os.WriteFile would have +// kept the stale mode and leaked the secret group/other-readable. +func TestClientCreate_CredentialFilePreexistingPerms(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet: + _, _ = w.Write([]byte(`[]`)) + case r.Method == http.MethodPost: + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","username":"u-5","namespace":"my-ns","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + credPath := filepath.Join(t.TempDir(), "cred.env") + // A stale, world-readable file already sits at the target path. + if err := os.WriteFile(credPath, []byte("stale\n"), 0o644); err != nil { + t.Fatal(err) + } + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "c", location: "DE", yes: true, credentialFile: credPath}); err != nil { + t.Fatalf("create: %v", err) + } + info, err := os.Stat(credPath) + if err != nil { + t.Fatalf("credential file not written: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("credential file mode = %o over a pre-existing 0644 target, want 600", perm) + } +} + +// TestClientCreate_CredentialFileWriteFailFatal asserts a credential-file write +// failure is fatal — the minted password is the only copy, so a failed write must +// surface an error, never a silent drop. The target's parent is a regular file, so +// the directory create (hence the write) fails deterministically. +func TestClientCreate_CredentialFileWriteFailFatal(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet: + _, _ = w.Write([]byte(`[]`)) + case r.Method == http.MethodPost: + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":5,"first_name":"c","username":"u-5","namespace":"my-ns","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + notADir := filepath.Join(t.TempDir(), "iam-a-file") + if err := os.WriteFile(notADir, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + credPath := filepath.Join(notADir, "cred.env") // parent is a file → write fails + var out bytes.Buffer + err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "c", location: "DE", yes: true, credentialFile: credPath}) + if err == nil { + t.Fatal("expected a fatal error when the credential file can't be written, got nil") + } +} + +func TestClientCreate_CredentialFileAdopt(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet: + _, _ = w.Write([]byte(`[]`)) + case r.Method == http.MethodPost: + w.WriteHeader(http.StatusOK) // adopt + _, _ = w.Write([]byte(`{"id":8,"first_name":"existing","username":"u-8","namespace":"ex-ns","location":"DE","cluster_id":"uid-1"}`)) + } + }) + stubClusterID(t, "uid-1", nil) + credPath := filepath.Join(t.TempDir(), "cred.env") + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "c", location: "DE", yes: true, credentialFile: credPath}); err != nil { + t.Fatalf("adopt: %v", err) + } + kv := parseEnvFile(t, credPath) + // adopt emits id + namespace + the ADOPTED marker, but NO password (the + // existing one stands; it's write-only on the backend). + if kv["TRACEBLOC_CLIENT_ID"] != "8" || kv["TB_NAMESPACE"] != "ex-ns" || kv["TRACEBLOC_CLIENT_ADOPTED"] != "1" { + t.Errorf("adopt credential file = %v (want id=8, ns=ex-ns, ADOPTED=1)", kv) + } + if _, hasPw := kv["TRACEBLOC_CLIENT_PASSWORD"]; hasPw { + t.Errorf("adopt must not write a password (none issued), got:\n%v", kv) + } +} + +// parseEnvFile reads a KEY=value env file (skipping # comments) into a map. +func parseEnvFile(t *testing.T, path string) map[string]string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read credential file: %v", err) + } + kv := map[string]string{} + for _, line := range strings.Split(string(raw), "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + if k, v, ok := strings.Cut(line, "="); ok { + kv[k] = v + } + } + return kv +} + func TestClientCreate_ClusterConflict(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { switch {