From e200e4b969b18ef0025134abcef60797ec203836 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Wed, 24 Jun 2026 15:42:42 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(cli):=20client=20create=20--credential?= =?UTF-8?q?-file=20=E2=80=94=20write=20the=20machine=20credential=20for=20?= =?UTF-8?q?the=20installer=20(#84)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `tracebloc client create --credential-file PATH`: instead of printing the minted credential, write it to PATH (mode 0600) as a sourceable env file the installer reorder (#838) consumes — the secret never hits the terminal (RFC §9 "secure by invisibility" / never-show, deferred here from cli#102). - Mint (201): writes TRACEBLOC_CLIENT_ID + TRACEBLOC_CLIENT_PASSWORD + TB_NAMESPACE (0600) and suppresses the stdout credential print. Write failure is fatal (the credential is the only copy). - Adopt (200): writes TRACEBLOC_CLIENT_ID + TB_NAMESPACE + TRACEBLOC_CLIENT_ADOPTED=1 (no password — the existing one stands, write-only on the backend); the installer reconciles the existing release rather than expecting a fresh credential. - Without the flag: behaviour unchanged (the interim credential print). Unblocks the #838 installer reorder (login -> create -> feed the chart). Tests cover mint (0600 + sourceable + never-printed) and adopt (id+ns+marker, no password). go build/vet/test ./... green. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/client.go | 74 +++++++++++++++++++++++++++----- internal/cli/client_test.go | 85 +++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 10 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 481ddf7f..966820ff 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,55 @@ 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. +func writeClientCredential(path string, lines []string) error { + if dir := filepath.Dir(path); 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" + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + 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..c894dbd0 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,89 @@ 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) + } +} + +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 { From c989bd22b1438a85965511e88879981b67999a1b Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 24 Jun 2026 18:57:05 +0500 Subject: [PATCH 2/2] fix(cli): force 0600 on credential file via temp+rename (#84) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit writeClientCredential used os.WriteFile(path, ..., 0o600), but WriteFile only applies its perm bits when it *creates* the file — over a pre-existing target it truncates and writes WITHOUT changing the mode. So a stale file, or one an attacker pre-creates world-readable, at --credential-file would receive the minted password (the only copy) at its old, possibly 0644 mode — silently breaking the flag's own 0600/never-show contract (RFC §9). Verified: a 0644 target stays 0644 after the write. Write to a 0600 temp file in the target dir and atomically rename over the path instead. CreateTemp is 0600 by construction, so the guarantee holds unconditionally; rename is atomic (no half-written credential) and the final write never follows a symlink planted at the target. Tests: - preexisting-perms: a 0644 target ends up 0600 (locks in the fix). - write-fail-fatal: an unwritable target surfaces an error, never a silent drop (the credential is the only copy). - mint never-show: also assert the password VALUE is absent from stdout, not just the literal "password"/"Machine credential" strings. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 29 ++++++++++++++-- internal/cli/client_test.go | 66 +++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 966820ff..c4d06a3f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -284,8 +284,16 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // 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 { - if dir := filepath.Dir(path); dir != "" && dir != "." { + dir := filepath.Dir(path) + if dir != "" && dir != "." { if err := os.MkdirAll(dir, 0o700); err != nil { return fmt.Errorf("creating credential-file directory: %w", err) } @@ -293,7 +301,24 @@ func writeClientCredential(path string, lines []string) error { 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" - if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + // 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 diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index c894dbd0..bd79021d 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -360,6 +360,72 @@ func TestClientCreate_CredentialFileMint(t *testing.T) { 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) {