From ab09e27eed01cf5ab901029c1578f7c496387746 Mon Sep 17 00:00:00 2001 From: Vasil Date: Sat, 24 Oct 2026 02:27:40 +0200 Subject: [PATCH] ktms CLI: a named context wins over the environment A context named with --context or KTMS_CONTEXT no longer mixes with KTMS_ADDR or KTMS_ADMIN_KEY from the shell. Private files are written through a fresh temporary file, keychain writes are read back since security -i exits 0 on failure, and KTMS_TLS must be a boolean. --- CHANGELOG.md | 2 ++ README.md | 3 ++- cmd/ktms/conn.go | 30 ++++++++++++++++++++++-------- cmd/ktms/contexts.go | 25 +++++++++++++++++++++---- cmd/ktms/main_test.go | 27 ++++++++++++++++++++++----- 5 files changed, 69 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc05ff8..4004786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ All notable changes to KTMS are listed here. The format follows `chat` for a test call showing the rule, model and vendor; `health`; `-o json|yaml`; exit codes by error class; completion for bash, zsh and fish. Released as archives, the `coyl/ktms-cli` image and a Homebrew formula (published once the tap token is set). + A context named with `--context` or `KTMS_CONTEXT` wins over `KTMS_ADDR`, `KTMS_ADMIN_KEY` and + the other variables. - `/healthz` reports the server's `version`. - Configuration as a file: `ktms config validate|plan|apply|export|schema` keeps models, rules, organizations and teams, client keys and their policies, vendor keys (from environment diff --git a/README.md b/README.md index ef3e6cc..4bc6def 100644 --- a/README.md +++ b/README.md @@ -912,7 +912,8 @@ ktms context use prod Contexts live in `~/.config/ktms/config.yaml` (`KTMS_CONFIG`). A key source is `env:VAR`, `file:PATH` or `keychain` (macOS Keychain, or the secret service through `secret-tool` on Linux); `store-key --file` keeps the key in a `0600` file instead. Flags win over `KTMS_ADDR`, `KTMS_TLS`, -`KTMS_HTTP`, `KTMS_ADMIN_KEY` and `KTMS_API_KEY`, which win over the context. +`KTMS_HTTP`, `KTMS_ADMIN_KEY` and `KTMS_API_KEY`, which win over the current context; a context named +with `--context` or `KTMS_CONTEXT` wins over them, so its keys never go elsewhere. ```bash ktms health # /healthz, /readyz and the server's version diff --git a/cmd/ktms/conn.go b/cmd/ktms/conn.go index 129d246..da98424 100644 --- a/cmd/ktms/conn.go +++ b/cmd/ktms/conn.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "os" + "strconv" "strings" "google.golang.org/grpc" @@ -32,6 +33,8 @@ type target struct { ctxName string warn io.Writer resolved bool + // KTMS_ADMIN_KEY and KTMS_API_KEY, unless a context was asked for by name. + envAdminKey, envClientKey string } // resolve fills addr, tls and http from the environment and the context, on @@ -63,15 +66,26 @@ func (t *target) resolve() error { if c == nil { c = &Context{} } - t.addr = firstNonEmpty(t.addr, os.Getenv("KTMS_ADDR"), c.Addr, "localhost:9090") + // A context asked for by name wins over the environment, so its keys never + // go to an address left in the shell, nor the shell's keys to its address. + env := os.Getenv + if asked != "" { + env = func(string) string { return "" } + } + t.envAdminKey, t.envClientKey = env("KTMS_ADMIN_KEY"), env("KTMS_API_KEY") + t.addr = firstNonEmpty(t.addr, env("KTMS_ADDR"), c.Addr, "localhost:9090") if !t.tlsSet { - if v := os.Getenv("KTMS_TLS"); v != "" { - t.tls = v == "true" || v == "1" + if v := env("KTMS_TLS"); v != "" { + b, err := strconv.ParseBool(v) + if err != nil { + return usageError{fmt.Errorf("KTMS_TLS=%q: want true or false", v)} + } + t.tls = b } else { t.tls = c.TLS } } - t.http = firstNonEmpty(t.http, os.Getenv("KTMS_HTTP"), c.HTTP) + t.http = firstNonEmpty(t.http, env("KTMS_HTTP"), c.HTTP) if t.http == "" { host, _, err := net.SplitHostPort(t.addr) if err != nil { @@ -96,8 +110,8 @@ func (t *target) adminKey() (string, error) { if t.keyFile != "" { return readKeyFile(t.keyFile) } - if k := os.Getenv("KTMS_ADMIN_KEY"); k != "" { - return k, nil + if t.envAdminKey != "" { + return t.envAdminKey, nil } if t.ctx != nil && !t.ctx.Key.empty() { k, err := t.ctx.Key.read(t.ctxName) @@ -117,8 +131,8 @@ func (t *target) clientKey(file string) (string, error) { if file != "" { return readKeyFile(file) } - if k := os.Getenv("KTMS_API_KEY"); k != "" { - return k, nil + if t.envClientKey != "" { + return t.envClientKey, nil } if t.ctx != nil && !t.ctx.ClientKey.empty() { k, err := t.ctx.ClientKey.read(t.ctxName + "/client") diff --git a/cmd/ktms/contexts.go b/cmd/ktms/contexts.go index 026de91..a30315b 100644 --- a/cmd/ktms/contexts.go +++ b/cmd/ktms/contexts.go @@ -164,13 +164,26 @@ func (c *Contexts) names() []string { return out } -// writePrivate writes a file only its owner can read, replacing it whole. +// writePrivate writes a file only its owner can read, replacing it whole. A +// fresh temporary file keeps a leftover or planted one from setting the mode. func writePrivate(path string, b []byte) error { - tmp := path + ".tmp" - if err := os.WriteFile(tmp, b, 0o600); err != nil { + f, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*") + if err != nil { + return err + } + defer os.Remove(f.Name()) + if err := f.Chmod(0o600); err != nil { + f.Close() + return err + } + if _, err := f.Write(b); err != nil { + f.Close() + return err + } + if err := f.Close(); err != nil { return err } - return os.Rename(tmp, path) + return os.Rename(f.Name(), path) } func expandHome(p string) string { @@ -227,6 +240,10 @@ func keychainSet(account, key string) error { if err := cmd.Run(); err != nil { return fmt.Errorf("keychain: %v %s", err, strings.TrimSpace(errb.String())) } + // security -i exits 0 even when the command in it failed, so read it back. + if got, err := keychainGet(account); err != nil || got != key { + return fmt.Errorf("keychain did not store the entry %s/%s %s", keychainService, account, strings.TrimSpace(errb.String())) + } return nil } diff --git a/cmd/ktms/main_test.go b/cmd/ktms/main_test.go index 60f2622..91d9e0b 100644 --- a/cmd/ktms/main_test.go +++ b/cmd/ktms/main_test.go @@ -127,9 +127,16 @@ func TestContexts(t *testing.T) { t.Fatalf("admin key = %q %v", k, err) } - // Flags beat the environment, which beats the context. + // Flags beat the environment, which beats the current context. t.Setenv("KTMS_ADDR", "env.example.com:9090") t.Setenv("KTMS_ADMIN_KEY", "ktms_env") + tg = target{} + if err := tg.resolve(); err != nil { + t.Fatal(err) + } + if k, _ := tg.adminKey(); tg.addr != "env.example.com:9090" || k != "ktms_env" { + t.Fatalf("env over the current context: %s %q", tg.addr, k) + } tg = target{context: "local", addr: "flag.example.com:9090"} if err := tg.resolve(); err != nil { t.Fatal(err) @@ -137,13 +144,23 @@ func TestContexts(t *testing.T) { if tg.addr != "flag.example.com:9090" || tg.tls { t.Fatalf("flags: %+v", tg) } - if k, _ := tg.adminKey(); k != "ktms_env" { - t.Fatalf("env key = %q", k) + if _, err := tg.adminKey(); err == nil || !strings.Contains(err.Error(), "LOCAL_KEY") { + t.Fatalf("the named context's key source is not used: %v", err) + } + t.Setenv("KTMS_ADDR", "") + + // A context asked for by name beats the shell's address and key. + t.Setenv("KTMS_ADMIN_KEY", "ktms_env") + tg = target{context: "prod"} + if k, _ := tg.adminKey(); k != "ktms_from_file" || tg.addr != "ktms.example.com:443" { + t.Fatalf("named context: addr %s key %q", tg.addr, k) } t.Setenv("KTMS_ADMIN_KEY", "") - if _, err := tg.adminKey(); err == nil || !strings.Contains(err.Error(), "LOCAL_KEY") { - t.Fatalf("missing env key: %v", err) + t.Setenv("KTMS_TLS", "yes please") + if code, _, _ := call("models", "list"); code != exitUsage { + t.Fatalf("bad KTMS_TLS = %d, want %d", code, exitUsage) } + t.Setenv("KTMS_TLS", "") if code, _, _ := call("context", "use", "nope"); code == 0 { t.Fatal("use of an unknown context succeeded")