From 7162d6d577f37289847ac74a9d187e722c941620 Mon Sep 17 00:00:00 2001 From: Diego Braga Date: Wed, 11 Nov 2026 23:51:11 +0100 Subject: [PATCH] fix(security): pass the passthrough token via a per-call kubeconfig, never argv With TOKEN_PASSTHROUGH=true the caller's bearer token was appended to the kubectl command line as `--token `. The joined argument list is the Operation of the ToolError returned when a command fails, so the token was copied into the tool result sent to the MCP client and into span status and exception events. It was also visible in the container's process list, and in the cache key that cached commands log at info level and put on spans. The CommandBuilder no longer puts a token on argv. For a call that carries one, Execute writes a minimal kubeconfig into a fresh 0700 temporary directory and passes it with --kubeconfig; the directory is removed when the call returns (success, failure, cancellation or deadline). The kubeconfig is derived from the configured one (explicit path, $KUBECONFIG or ~/.kube/config, else the in-cluster environment): the selected context's cluster entry is copied unchanged and the context namespace is kept. Its only user entry is `tokenFile:` pointing at a 0600 file next to it, so the token is neither on argv nor inside the kubeconfig. No credential or impersonation setting of the base user is carried over. Before, `--token` only overrode the token: a base user with a client certificate still sent it, and the API server could authenticate the call as that identity instead of the caller. The builder's flags are now placed before the first "--", so options such as --kubeconfig take effect for `kubectl exec ... -- cmd` and `kubectl run ... -- cmd` instead of being passed to the remote command. The cache key of a token-carrying command holds a per-process HMAC fingerprint of the token, which keeps results per identity without exposing the token. Defense in depth, a new internal/redact package: argument lists are redacted (values of --token, --kube-token, --password, --client-key, inline forms, and any key=value whose key matches token, password or secret) before they are logged or used as an error Operation; ToolError.Error, ToMCPResult, the tool span status and recorded exceptions, telemetry.RecordError and the shell executor's failure log redact the same patterns, plus bearer credentials, in free text. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016RAxvkp7wRXwXDx4Lx8STi Signed-off-by: Diego Braga --- internal/cmd/cmd.go | 5 +- internal/commands/builder.go | 74 ++- internal/commands/token_kubeconfig.go | 222 +++++++++ internal/commands/token_kubeconfig_test.go | 501 +++++++++++++++++++++ internal/errors/tool_errors.go | 12 +- internal/logger/logger.go | 20 +- internal/mcp/mcp.go | 10 +- internal/mcp/mcp_test.go | 52 +++ internal/redact/redact.go | 120 +++++ internal/redact/redact_test.go | 89 ++++ internal/telemetry/middleware.go | 9 +- pkg/k8s/k8s_test.go | 121 +++-- pkg/k8s/token_passthrough_test.go | 197 ++++++++ 13 files changed, 1321 insertions(+), 111 deletions(-) create mode 100644 internal/commands/token_kubeconfig.go create mode 100644 internal/commands/token_kubeconfig_test.go create mode 100644 internal/redact/redact.go create mode 100644 internal/redact/redact_test.go create mode 100644 pkg/k8s/token_passthrough_test.go diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 0fa2741..f9ce7d4 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -6,6 +6,7 @@ import ( "time" "github.com/kagent-dev/tools/internal/logger" + "github.com/kagent-dev/tools/internal/redact" ) // ShellExecutor defines the interface for executing shell commands @@ -36,8 +37,8 @@ func (e *DefaultShellExecutor) Exec(ctx context.Context, command string, args .. log.Error("command execution failed", "command", command, "args", redactedArgs, - "error", err, - "output", string(output), + "error", redact.String(err.Error()), + "output", redact.String(string(output)), "duration", duration.Seconds(), ) } else { diff --git a/internal/commands/builder.go b/internal/commands/builder.go index c9baed8..9651e25 100644 --- a/internal/commands/builder.go +++ b/internal/commands/builder.go @@ -10,6 +10,7 @@ import ( "github.com/kagent-dev/tools/internal/cmd" "github.com/kagent-dev/tools/internal/errors" "github.com/kagent-dev/tools/internal/logger" + "github.com/kagent-dev/tools/internal/redact" "github.com/kagent-dev/tools/internal/security" "github.com/kagent-dev/tools/internal/telemetry" "go.opentelemetry.io/otel/attribute" @@ -121,7 +122,11 @@ func (cb *CommandBuilder) WithKubeconfig(kubeconfig string) *CommandBuilder { return cb } -// WithToken sets the authentication token for kubectl commands +// WithToken sets a bearer token to authenticate the command with. The token is +// never put on the command line: Execute writes it to a per-call kubeconfig +// (see newTokenKubeconfig) that replaces the configured one for that call and +// is removed when the call returns. The command must accept --kubeconfig +// (kubectl, helm, istioctl and cilium do). func (cb *CommandBuilder) WithToken(token string) *CommandBuilder { if token != "" { cb.token = token @@ -227,12 +232,30 @@ func (cb *CommandBuilder) WithCacheKey(key string) *CommandBuilder { return cb } -// Build constructs the final command arguments +// Build constructs the final command arguments. A token set with WithToken is +// not part of them; Execute supplies it through a per-call kubeconfig. func (cb *CommandBuilder) Build() (string, []string, error) { + return cb.command, cb.buildArgs(cb.kubeconfig), nil +} + +// buildArgs constructs the command arguments with the given kubeconfig path +// (none when empty). The builder's flags are placed before the first "--" +// argument: everything after it is passed to a remote or child command (for +// example `kubectl exec pod -- cmd`), so a flag there would neither take +// effect nor stay inside this process. +func (cb *CommandBuilder) buildArgs(kubeconfig string) []string { + head, tail := cb.args, []string(nil) + for i, a := range cb.args { + if a == "--" { + head, tail = cb.args[:i], cb.args[i:] + break + } + } + args := make([]string, 0, len(cb.args)+20) // Add main arguments - args = append(args, cb.args...) + args = append(args, head...) // Add namespace if specified if cb.namespace != "" { @@ -245,13 +268,8 @@ func (cb *CommandBuilder) Build() (string, []string, error) { } // Add kubeconfig if specified - if cb.kubeconfig != "" { - args = append(args, "--kubeconfig", cb.kubeconfig) - } - - // Add token if specified - if cb.token != "" { - args = append(args, "--token", cb.token) + if kubeconfig != "" { + args = append(args, "--kubeconfig", kubeconfig) } // Add output format @@ -299,7 +317,7 @@ func (cb *CommandBuilder) Build() (string, []string, error) { args = append(args, "--validate=false") } - return cb.command, args, nil + return append(args, tail...) } // Execute runs the command @@ -370,6 +388,12 @@ func (cb *CommandBuilder) executeWithCache(ctx context.Context, command string, if cacheKey == "" { cacheKey = cache.CacheKey(append([]string{command}, args...)...) } + if cb.token != "" { + // Results are per identity, also for a custom key. The key is logged + // and put on spans, so it carries a keyed fingerprint of the token, + // not the token. + cacheKey = cache.CacheKey(cacheKey, "token-fingerprint="+tokenFingerprint(cb.token)) + } log.Info("executing cached command", "command", command, @@ -422,22 +446,40 @@ func (cb *CommandBuilder) executeWithCache(ctx context.Context, command string, return result, nil } -// executeCommand executes the actual command +// executeCommand executes the actual command. With a token, args (built for +// the configured kubeconfig) are rebuilt to point at a per-call kubeconfig that +// carries the token; it is removed when executeCommand returns, whether the +// command succeeded, failed or was cancelled. func (cb *CommandBuilder) executeCommand(ctx context.Context, command string, args []string) (string, error) { + // Errors describe the command as built for the configured kubeconfig; the + // per-call kubeconfig's path is meaningless once it has been removed. + displayArgs := args + if cb.token != "" { + kc, err := newTokenKubeconfig(cb.kubeconfig, cb.context, cb.token) + if err != nil { + return "", errors.NewCommandError(command, err) + } + defer kc.cleanup() + args = cb.buildArgs(kc.path) + } + executor := cmd.GetShellExecutor(ctx) output, err := executor.Exec(ctx, command, args...) if err != nil { + // The operation is shown to the MCP client and recorded on spans, so + // any credential in the arguments is redacted. + operation := strings.Join(redact.Args(displayArgs), " ") // Create appropriate error based on command type var toolError *errors.ToolError switch command { case "kubectl": - toolError = errors.NewKubernetesError(strings.Join(args, " "), err) + toolError = errors.NewKubernetesError(operation, err) case "helm": - toolError = errors.NewHelmError(strings.Join(args, " "), err) + toolError = errors.NewHelmError(operation, err) case "istioctl": - toolError = errors.NewIstioError(strings.Join(args, " "), err) + toolError = errors.NewIstioError(operation, err) case "cilium": - toolError = errors.NewCiliumError(strings.Join(args, " "), err) + toolError = errors.NewCiliumError(operation, err) default: toolError = errors.NewCommandError(command, err) } diff --git a/internal/commands/token_kubeconfig.go b/internal/commands/token_kubeconfig.go new file mode 100644 index 0000000..c6da353 --- /dev/null +++ b/internal/commands/token_kubeconfig.go @@ -0,0 +1,222 @@ +package commands + +import ( + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "encoding/hex" + "fmt" + "net" + "os" + "path/filepath" + "strings" + + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +const ( + // tokenUserName names the only user entry in a per-call kubeconfig. + tokenUserName = "kagent-token-passthrough" + // inClusterName names the cluster and context derived from the in-cluster + // environment when no kubeconfig is available. + inClusterName = "in-cluster" +) + +// In-cluster service account files, the same ones kubectl and client-go read. +// Variables so tests can point them elsewhere. +var ( + inClusterCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + inClusterNamespaceFile = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" +) + +// tokenKubeconfig is a kubeconfig written for a single command execution. It +// carries the caller's bearer token by reference (tokenFile) so the token is +// never on the child process's command line, and never inside the kubeconfig +// itself (a CLI that quotes its kubeconfig, or `kubectl config view --raw`, +// shows only the token file's path). +type tokenKubeconfig struct { + dir string + path string +} + +// newTokenKubeconfig writes a minimal kubeconfig to a fresh private temporary +// directory (mode 0700, files 0600). The cluster entry is copied unchanged from +// the base configuration's selected context (server, CA, TLS settings, +// proxy-url, ...). The user entry carries only the token file: no client +// certificate or key, exec plugin, auth provider, basic auth or impersonation +// is copied from the base user. +// +// The base configuration is resolved the way kubectl resolves it: the explicit +// kubeconfig path when set, otherwise $KUBECONFIG or ~/.kube/config, and when +// none of those yields any configuration, the in-cluster environment +// (KUBERNETES_SERVICE_HOST/PORT and the service account CA and namespace). +// contextName selects a context of the base kubeconfig; empty means its +// current context. +// +// The caller must call cleanup, which removes the directory and both files. +// Error messages never contain the token. +func newTokenKubeconfig(baseKubeconfig, contextName, token string) (*tokenKubeconfig, error) { + cfg, err := tokenKubeconfigConfig(baseKubeconfig, contextName) + if err != nil { + return nil, err + } + + dir, err := os.MkdirTemp("", "kagent-kubeconfig-") + if err != nil { + return nil, fmt.Errorf("token passthrough: create temporary directory: %w", err) + } + kc := &tokenKubeconfig{dir: dir} + ok := false + defer func() { + if !ok { + kc.cleanup() + } + }() + // MkdirTemp creates the directory 0700; make that explicit so a permissive + // umask or platform default can never widen it. + if err := os.Chmod(dir, 0o700); err != nil { + return nil, fmt.Errorf("token passthrough: restrict temporary directory: %w", err) + } + + tokenPath, err := writePrivateFile(dir, "token-*", []byte(token)) + if err != nil { + return nil, err + } + cfg.AuthInfos[tokenUserName] = &clientcmdapi.AuthInfo{TokenFile: tokenPath} + + data, err := clientcmd.Write(*cfg) + if err != nil { + return nil, fmt.Errorf("token passthrough: encode kubeconfig: %w", err) + } + kc.path, err = writePrivateFile(dir, "kubeconfig-*", data) + if err != nil { + return nil, err + } + + ok = true + return kc, nil +} + +// cleanup removes the kubeconfig and the token file. It is safe to call more +// than once and on a nil receiver. +func (kc *tokenKubeconfig) cleanup() { + if kc == nil || kc.dir == "" { + return + } + _ = os.RemoveAll(kc.dir) +} + +// tokenKubeconfigConfig builds the per-call kubeconfig without the user's +// credential, which newTokenKubeconfig adds once the token file exists. +func tokenKubeconfigConfig(baseKubeconfig, contextName string) (*clientcmdapi.Config, error) { + rules := clientcmd.NewDefaultClientConfigLoadingRules() + rules.ExplicitPath = baseKubeconfig + rules.MigrationRules = nil // read only: never move files in the home directory + base, err := rules.Load() + if err != nil { + return nil, fmt.Errorf("token passthrough: load kubeconfig: %w", err) + } + + if isEmptyConfig(base) { + if contextName != "" { + return nil, fmt.Errorf("token passthrough: context %q not found: no kubeconfig is configured", contextName) + } + return inClusterConfig() + } + + if contextName == "" { + contextName = base.CurrentContext + } + if contextName == "" { + return nil, fmt.Errorf("token passthrough: kubeconfig has no current context") + } + ctx, ok := base.Contexts[contextName] + if !ok || ctx == nil { + return nil, fmt.Errorf("token passthrough: context %q not found in kubeconfig", contextName) + } + cluster, ok := base.Clusters[ctx.Cluster] + if !ok || cluster == nil { + return nil, fmt.Errorf("token passthrough: cluster %q of context %q not found in kubeconfig", ctx.Cluster, contextName) + } + + cfg := clientcmdapi.NewConfig() + // The whole cluster entry is kept: server, certificate-authority(-data), + // insecure-skip-tls-verify, tls-server-name, proxy-url, disable-compression + // and extensions. The loader has already made file paths absolute. + cfg.Clusters[ctx.Cluster] = cluster.DeepCopy() + cfg.Contexts[contextName] = &clientcmdapi.Context{ + Cluster: ctx.Cluster, + AuthInfo: tokenUserName, + Namespace: ctx.Namespace, + } + cfg.CurrentContext = contextName + return cfg, nil +} + +// inClusterConfig derives the cluster from the in-cluster environment, as +// kubectl does when no kubeconfig is available. +func inClusterConfig() (*clientcmdapi.Config, error) { + host, port := os.Getenv("KUBERNETES_SERVICE_HOST"), os.Getenv("KUBERNETES_SERVICE_PORT") + if host == "" || port == "" { + return nil, fmt.Errorf("token passthrough: no kubeconfig is configured and not running in a cluster") + } + + cluster := &clientcmdapi.Cluster{Server: "https://" + net.JoinHostPort(host, port)} + if _, err := os.Stat(inClusterCAFile); err == nil { + cluster.CertificateAuthority = inClusterCAFile + } + ctx := &clientcmdapi.Context{Cluster: inClusterName, AuthInfo: tokenUserName} + if ns, err := os.ReadFile(inClusterNamespaceFile); err == nil { + ctx.Namespace = strings.TrimSpace(string(ns)) + } + + cfg := clientcmdapi.NewConfig() + cfg.Clusters[inClusterName] = cluster + cfg.Contexts[inClusterName] = ctx + cfg.CurrentContext = inClusterName + return cfg, nil +} + +func isEmptyConfig(cfg *clientcmdapi.Config) bool { + return cfg == nil || (len(cfg.Clusters) == 0 && len(cfg.Contexts) == 0 && len(cfg.AuthInfos) == 0) +} + +// writePrivateFile writes data to a new file in dir with mode 0600 and returns +// its path. +func writePrivateFile(dir, pattern string, data []byte) (string, error) { + f, err := os.CreateTemp(dir, pattern) + if err != nil { + return "", fmt.Errorf("token passthrough: create %s: %w", pattern, err) + } + path := f.Name() + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + return "", fmt.Errorf("token passthrough: restrict %s: %w", filepath.Base(path), err) + } + if _, err := f.Write(data); err != nil { + _ = f.Close() + return "", fmt.Errorf("token passthrough: write %s: %w", filepath.Base(path), err) + } + if err := f.Close(); err != nil { + return "", fmt.Errorf("token passthrough: close %s: %w", filepath.Base(path), err) + } + return path, nil +} + +// fingerprintKey keys tokenFingerprint. It is random per process, so a +// fingerprint in a log or a trace cannot be used to confirm a guessed token. +var fingerprintKey = func() []byte { + k := make([]byte, 32) + if _, err := rand.Read(k); err != nil { + panic("commands: read random fingerprint key: " + err.Error()) + } + return k +}() + +// tokenFingerprint identifies a token in cache keys without revealing it. +func tokenFingerprint(token string) string { + m := hmac.New(sha256.New, fingerprintKey) + m.Write([]byte(token)) + return hex.EncodeToString(m.Sum(nil)) +} diff --git a/internal/commands/token_kubeconfig_test.go b/internal/commands/token_kubeconfig_test.go new file mode 100644 index 0000000..899d0b7 --- /dev/null +++ b/internal/commands/token_kubeconfig_test.go @@ -0,0 +1,501 @@ +package commands + +import ( + "context" + "encoding/base64" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/kagent-dev/tools/internal/cache" + "github.com/kagent-dev/tools/internal/cmd" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +const testToken = "eyJhbGciOiJSUzI1NiJ9.passthrough-test-token.signature" + +var testCAData = []byte("-----BEGIN CERTIFICATE-----\nnot-a-real-ca\n-----END CERTIFICATE-----\n") + +// writeBaseKubeconfig writes a kubeconfig whose user carries every credential +// type and impersonation, so tests can check none of it is carried over. +func writeBaseKubeconfig(t *testing.T) string { + t.Helper() + dir := t.TempDir() + caFile := filepath.Join(dir, "ca.crt") + require.NoError(t, os.WriteFile(caFile, testCAData, 0o600)) + cfg := `apiVersion: v1 +kind: Config +clusters: +- name: prod + cluster: + server: https://prod.example.test:6443 + certificate-authority-data: ` + base64.StdEncoding.EncodeToString(testCAData) + ` + tls-server-name: api.prod.example.test + proxy-url: http://proxy.example.test:3128 + disable-compression: true +- name: staging + cluster: + server: https://staging.example.test:6443 + certificate-authority: ca.crt +contexts: +- name: prod-ctx + context: + cluster: prod + user: admin + namespace: apps +- name: staging-ctx + context: + cluster: staging + user: admin +current-context: prod-ctx +users: +- name: admin + user: + client-certificate-data: ` + base64.StdEncoding.EncodeToString([]byte("client-cert")) + ` + client-key-data: ` + base64.StdEncoding.EncodeToString([]byte("client-key")) + ` + token: server-identity-token + username: admin + password: admin-password + as: system:admin + as-uid: "1234" + as-groups: ["system:masters"] + as-user-extra: + scopes: ["all"] + exec: + apiVersion: client.authentication.k8s.io/v1 + command: get-credentials + interactiveMode: Never +` + path := filepath.Join(dir, "config") + require.NoError(t, os.WriteFile(path, []byte(cfg), 0o600)) + return path +} + +// observedKubeconfig is what a command saw in its --kubeconfig while it ran. +type observedKubeconfig struct { + path string + dirMode os.FileMode + fileMode os.FileMode + tokenMode os.FileMode + token string + config *clientcmdapi.Config +} + +// recordingExecutor captures argv and the per-call kubeconfig during Exec. +type recordingExecutor struct { + t *testing.T + mu sync.Mutex + calls [][]string + seen []observedKubeconfig + output string + err error + // block, when set, makes Exec wait for the context to be done. + block bool +} + +func (e *recordingExecutor) Exec(ctx context.Context, _ string, args ...string) ([]byte, error) { + e.mu.Lock() + e.calls = append(e.calls, append([]string(nil), args...)) + for i := 0; i+1 < len(args); i++ { + if args[i] == "--kubeconfig" { + e.seen = append(e.seen, observe(e.t, args[i+1])) + } + } + e.mu.Unlock() + if e.block { + <-ctx.Done() + return nil, ctx.Err() + } + return []byte(e.output), e.err +} + +func observe(t *testing.T, path string) observedKubeconfig { + o := observedKubeconfig{path: path} + fi, err := os.Stat(path) + require.NoError(t, err) + o.fileMode = fi.Mode().Perm() + di, err := os.Stat(filepath.Dir(path)) + require.NoError(t, err) + o.dirMode = di.Mode().Perm() + o.config, err = clientcmd.LoadFromFile(path) + require.NoError(t, err) + if user := o.config.AuthInfos[tokenUserName]; user != nil && user.TokenFile != "" { + ti, err := os.Stat(user.TokenFile) + require.NoError(t, err) + o.tokenMode = ti.Mode().Perm() + b, err := os.ReadFile(user.TokenFile) + require.NoError(t, err) + o.token = string(b) + } + return o +} + +func (e *recordingExecutor) lastSeen(t *testing.T) observedKubeconfig { + e.mu.Lock() + defer e.mu.Unlock() + require.NotEmpty(t, e.seen, "command must be given a --kubeconfig") + return e.seen[len(e.seen)-1] +} + +func assertNoTokenOnArgv(t *testing.T, e *recordingExecutor) { + t.Helper() + e.mu.Lock() + defer e.mu.Unlock() + require.NotEmpty(t, e.calls) + for _, args := range e.calls { + for _, a := range args { + assert.NotContains(t, a, testToken) + assert.NotEqual(t, "--token", a) + } + } +} + +func assertRemoved(t *testing.T, o observedKubeconfig) { + t.Helper() + _, err := os.Stat(filepath.Dir(o.path)) + assert.True(t, os.IsNotExist(err), "per-call kubeconfig directory must be removed, stat err: %v", err) +} + +func TestExecuteWithTokenKeepsTokenOffArgv(t *testing.T) { + base := writeBaseKubeconfig(t) + exec := &recordingExecutor{t: t, output: "ok"} + ctx := cmd.WithShellExecutor(context.Background(), exec) + + out, err := KubectlBuilder().WithArgs("get", "pods").WithNamespace("default"). + WithKubeconfig(base).WithToken(testToken).Execute(ctx) + require.NoError(t, err) + assert.Equal(t, "ok", out) + + assertNoTokenOnArgv(t, exec) + args := exec.calls[0] + assert.Equal(t, []string{"get", "pods", "--namespace", "default", "--kubeconfig"}, args[:5]) + assert.NotEqual(t, base, args[5], "the configured kubeconfig is replaced for the call") + assert.Len(t, args, 6) + + o := exec.lastSeen(t) + assert.Equal(t, testToken, o.token) + assert.Equal(t, os.FileMode(0o600), o.fileMode) + assert.Equal(t, os.FileMode(0o600), o.tokenMode) + assert.Equal(t, os.FileMode(0o700), o.dirMode) + assertRemoved(t, o) +} + +func TestBuildNeverEmitsToken(t *testing.T) { + _, args, err := KubectlBuilder().WithArgs("get", "pods").WithToken(testToken).Build() + require.NoError(t, err) + assert.Equal(t, []string{"get", "pods"}, args) +} + +func TestTokenKubeconfigUserCarriesOnlyTheToken(t *testing.T) { + base := writeBaseKubeconfig(t) + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithToken(testToken).Execute(ctx) + require.NoError(t, err) + + cfg := exec.lastSeen(t).config + require.Len(t, cfg.AuthInfos, 1, "exactly one user") + user := cfg.AuthInfos[tokenUserName] + require.NotNil(t, user) + + // Exactly one credential: the token, by reference. + assert.NotEmpty(t, user.TokenFile) + assert.Empty(t, user.Token, "token is referenced through tokenFile, not inlined") + assert.Empty(t, user.ClientCertificate) + assert.Empty(t, user.ClientCertificateData) + assert.Empty(t, user.ClientKey) + assert.Empty(t, user.ClientKeyData) + assert.Empty(t, user.Username) + assert.Empty(t, user.Password) + assert.Nil(t, user.Exec) + assert.Nil(t, user.AuthProvider) + // No impersonation. + assert.Empty(t, user.Impersonate) + assert.Empty(t, user.ImpersonateUID) + assert.Empty(t, user.ImpersonateGroups) + assert.Empty(t, user.ImpersonateUserExtra) + // Nothing else either: the whole entry equals a token-file-only user. + assert.Equal(t, clientcmdapi.AuthInfo{TokenFile: user.TokenFile, Extensions: user.Extensions, LocationOfOrigin: user.LocationOfOrigin}, *user) + assert.Empty(t, user.Extensions) + + // The file on disk holds no credential from the base kubeconfig. + raw, err := os.ReadFile(base) + require.NoError(t, err) + require.Contains(t, string(raw), "server-identity-token") + derived, err := clientcmd.Write(*cfg) + require.NoError(t, err) + for _, s := range []string{"server-identity-token", "admin-password", "get-credentials", "system:admin", "system:masters", base64.StdEncoding.EncodeToString([]byte("client-key"))} { + assert.NotContains(t, string(derived), s) + } +} + +func TestTokenKubeconfigKeepsClusterAndContext(t *testing.T) { + base := writeBaseKubeconfig(t) + + t.Run("current context", func(t *testing.T) { + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithToken(testToken).Execute(ctx) + require.NoError(t, err) + + cfg := exec.lastSeen(t).config + assert.Equal(t, "prod-ctx", cfg.CurrentContext) + require.Len(t, cfg.Contexts, 1) + c := cfg.Contexts["prod-ctx"] + require.NotNil(t, c) + assert.Equal(t, "prod", c.Cluster) + assert.Equal(t, tokenUserName, c.AuthInfo) + assert.Equal(t, "apps", c.Namespace, "context namespace is kept") + require.Len(t, cfg.Clusters, 1) + cl := cfg.Clusters["prod"] + require.NotNil(t, cl) + assert.Equal(t, "https://prod.example.test:6443", cl.Server) + assert.Equal(t, testCAData, cl.CertificateAuthorityData) + assert.Equal(t, "api.prod.example.test", cl.TLSServerName) + assert.Equal(t, "http://proxy.example.test:3128", cl.ProxyURL) + assert.True(t, cl.DisableCompression) + }) + + t.Run("explicit context and relative CA path", func(t *testing.T) { + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithContext("staging-ctx"). + WithToken(testToken).Execute(ctx) + require.NoError(t, err) + + args := exec.calls[0] + assert.Contains(t, strings.Join(args, " "), "--context staging-ctx") + cfg := exec.lastSeen(t).config + assert.Equal(t, "staging-ctx", cfg.CurrentContext) + cl := cfg.Clusters["staging"] + require.NotNil(t, cl) + assert.Equal(t, "https://staging.example.test:6443", cl.Server) + assert.Equal(t, filepath.Join(filepath.Dir(base), "ca.crt"), cl.CertificateAuthority, + "relative CA path is resolved against the base kubeconfig") + }) + + t.Run("unknown context fails without running the command", func(t *testing.T) { + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithContext("missing"). + WithToken(testToken).Execute(ctx) + require.Error(t, err) + assert.NotContains(t, err.Error(), testToken) + assert.Empty(t, exec.calls) + }) +} + +func TestTokenKubeconfigInCluster(t *testing.T) { + dir := t.TempDir() + t.Setenv("KUBECONFIG", filepath.Join(dir, "does-not-exist")) + t.Setenv("HOME", dir) + t.Setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1") + t.Setenv("KUBERNETES_SERVICE_PORT", "443") + ca := filepath.Join(dir, "ca.crt") + ns := filepath.Join(dir, "namespace") + require.NoError(t, os.WriteFile(ca, testCAData, 0o600)) + require.NoError(t, os.WriteFile(ns, []byte("kagent\n"), 0o600)) + prevCA, prevNS := inClusterCAFile, inClusterNamespaceFile + inClusterCAFile, inClusterNamespaceFile = ca, ns + t.Cleanup(func() { inClusterCAFile, inClusterNamespaceFile = prevCA, prevNS }) + + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithToken(testToken).Execute(ctx) + require.NoError(t, err) + + o := exec.lastSeen(t) + cfg := o.config + assert.Equal(t, inClusterName, cfg.CurrentContext) + assert.Equal(t, "https://10.96.0.1:443", cfg.Clusters[inClusterName].Server) + assert.Equal(t, ca, cfg.Clusters[inClusterName].CertificateAuthority) + assert.Equal(t, "kagent", cfg.Contexts[inClusterName].Namespace) + assert.Equal(t, testToken, o.token) + assertNoTokenOnArgv(t, exec) + assertRemoved(t, o) + + t.Run("no kubeconfig and not in a cluster", func(t *testing.T) { + t.Setenv("KUBERNETES_SERVICE_HOST", "") + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "pods").WithToken(testToken).Execute(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "not running in a cluster") + assert.NotContains(t, err.Error(), testToken) + assert.Empty(t, exec.calls) + }) +} + +func TestTokenKubeconfigRemovedOnErrorAndCancel(t *testing.T) { + base := writeBaseKubeconfig(t) + + t.Run("command fails", func(t *testing.T) { + exec := &recordingExecutor{t: t, output: "Error from server (Forbidden)", err: assert.AnError} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("get", "secrets").WithKubeconfig(base).WithToken(testToken).Execute(ctx) + require.Error(t, err) + assertRemoved(t, exec.lastSeen(t)) + }) + + t.Run("context cancelled while the command runs", func(t *testing.T) { + exec := &recordingExecutor{t: t, block: true} + ctx, cancel := context.WithCancel(cmd.WithShellExecutor(context.Background(), exec)) + done := make(chan error, 1) + go func() { + _, err := KubectlBuilder().WithArgs("get", "pods", "--watch").WithKubeconfig(base).WithToken(testToken).Execute(ctx) + done <- err + }() + require.Eventually(t, func() bool { + exec.mu.Lock() + defer exec.mu.Unlock() + return len(exec.seen) == 1 + }, 5*time.Second, 10*time.Millisecond) + o := exec.lastSeen(t) + _, err := os.Stat(o.path) + require.NoError(t, err, "kubeconfig exists while the command runs") + cancel() + select { + case err := <-done: + require.Error(t, err) + case <-time.After(5 * time.Second): + t.Fatal("Execute did not return after cancel") + } + assertRemoved(t, o) + }) + + t.Run("context deadline exceeded", func(t *testing.T) { + exec := &recordingExecutor{t: t, block: true} + ctx, cancel := context.WithTimeout(cmd.WithShellExecutor(context.Background(), exec), 50*time.Millisecond) + defer cancel() + _, err := KubectlBuilder().WithArgs("wait", "--for=condition=ready", "pod/x").WithKubeconfig(base). + WithToken(testToken).Execute(ctx) + require.Error(t, err) + assertRemoved(t, exec.lastSeen(t)) + }) +} + +// TestFailedCommandDoesNotLeakToken covers the reported leak: the error a +// failing call returns (shown to the MCP client) and the spans it records must +// not contain the token. +func TestFailedCommandDoesNotLeakToken(t *testing.T) { + exp := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exp)) + prev := otel.GetTracerProvider() + otel.SetTracerProvider(tp) + t.Cleanup(func() { + otel.SetTracerProvider(prev) + _ = tp.Shutdown(context.Background()) + }) + + base := writeBaseKubeconfig(t) + for _, command := range []string{"kubectl", "helm", "istioctl", "cilium"} { + t.Run(command, func(t *testing.T) { + exp.Reset() + exec := &recordingExecutor{t: t, err: assert.AnError} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := NewCommandBuilder(command).WithArgs("get", "secrets").WithKubeconfig(base). + WithToken(testToken).Execute(ctx) + require.Error(t, err) + assert.NotContains(t, err.Error(), testToken) + assert.Contains(t, err.Error(), "get secrets --kubeconfig "+base+" failed", + "the error names the configured kubeconfig, not the removed per-call one") + assertNoTokenOnArgv(t, exec) + + spans := exp.GetSpans() + require.NotEmpty(t, spans) + for _, s := range spans { + assert.NotContains(t, s.Status.Description, testToken) + for _, a := range s.Attributes { + assert.NotContains(t, a.Value.Emit(), testToken, "span %s attribute %s", s.Name, a.Key) + } + for _, ev := range s.Events { + for _, a := range ev.Attributes { + assert.NotContains(t, a.Value.Emit(), testToken, "span %s event %s", s.Name, ev.Name) + } + } + } + }) + } +} + +// TestFailedCommandRedactsCredentialArgs covers the defense in depth: a +// credential a caller puts in the arguments is redacted from the error. +func TestFailedCommandRedactsCredentialArgs(t *testing.T) { + exec := &recordingExecutor{t: t, err: assert.AnError} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := HelmBuilder().WithArgs("install", "x", "chart", "--kube-token", testToken, "--set", "db.password="+testToken).Execute(ctx) + require.Error(t, err) + assert.NotContains(t, err.Error(), testToken) + assert.Contains(t, err.Error(), "--kube-token ") +} + +func TestBuilderFlagsGoBeforeDoubleDash(t *testing.T) { + base := writeBaseKubeconfig(t) + exec := &recordingExecutor{t: t} + ctx := cmd.WithShellExecutor(context.Background(), exec) + _, err := KubectlBuilder().WithArgs("exec", "mypod", "--", "ls", "-la").WithNamespace("default"). + WithKubeconfig(base).WithToken(testToken).Execute(ctx) + require.NoError(t, err) + + args := exec.calls[0] + dd := -1 + for i, a := range args { + if a == "--" { + dd = i + break + } + } + require.NotEqual(t, -1, dd) + assert.Equal(t, []string{"--", "ls", "-la"}, args[dd:], "the remote command is unchanged") + assert.Contains(t, args[:dd], "--kubeconfig") + assert.Contains(t, args[:dd], "--namespace") +} + +func TestCachedExecutionIsPerToken(t *testing.T) { + cache.InitCaches() + cache.InvalidateKubernetesCache() + t.Cleanup(cache.InvalidateKubernetesCache) + + base := writeBaseKubeconfig(t) + exec := &recordingExecutor{t: t, output: "pods"} + ctx := cmd.WithShellExecutor(context.Background(), exec) + run := func(token string) { + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithToken(token). + WithCache(true).Execute(ctx) + require.NoError(t, err) + } + run(testToken) + run(testToken) + run("another-callers-token") + exec.mu.Lock() + calls := len(exec.calls) + exec.mu.Unlock() + assert.Equal(t, 2, calls, "same token hits the cache, a different token does not") + assertNoTokenOnArgv(t, exec) + + // A custom cache key is per token too. + _, err := KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithToken("third-token"). + WithCache(true).WithCacheKey("pods").Execute(ctx) + require.NoError(t, err) + _, err = KubectlBuilder().WithArgs("get", "pods").WithKubeconfig(base).WithToken("fourth-token"). + WithCache(true).WithCacheKey("pods").Execute(ctx) + require.NoError(t, err) + exec.mu.Lock() + calls = len(exec.calls) + exec.mu.Unlock() + assert.Equal(t, 4, calls, "a custom cache key does not share results across tokens") + + key := tokenFingerprint(testToken) + assert.NotContains(t, key, testToken) + assert.Len(t, key, 64) + assert.NotEqual(t, key, tokenFingerprint("another-callers-token")) +} diff --git a/internal/errors/tool_errors.go b/internal/errors/tool_errors.go index 95fd684..75b2941 100644 --- a/internal/errors/tool_errors.go +++ b/internal/errors/tool_errors.go @@ -6,6 +6,7 @@ import ( "time" mcp "github.com/kagent-dev/tools/internal/mcp" + "github.com/kagent-dev/tools/internal/redact" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -23,12 +24,15 @@ type ToolError struct { Context map[string]string `json:"context,omitempty"` } -// Error implements the error interface +// Error implements the error interface. The message is redacted +// (redact.String) because callers return it to MCP clients and record it on +// spans; a credential in the operation or cause must not leave the process. func (e *ToolError) Error() string { - return fmt.Sprintf("[%s] %s failed: %v", e.Component, e.Operation, e.Cause) + return redact.String(fmt.Sprintf("[%s] %s failed: %v", e.Component, e.Operation, e.Cause)) } -// ToMCPResult converts the error to an MCP result with rich context +// ToMCPResult converts the error to an MCP result with rich context. Like +// Error, the text is redacted before it is returned. func (e *ToolError) ToMCPResult() *sdkmcp.CallToolResult { var message strings.Builder @@ -68,7 +72,7 @@ func (e *ToolError) ToMCPResult() *sdkmcp.CallToolResult { } } - return mcp.NewToolResultError(message.String()) + return mcp.NewToolResultError(redact.String(message.String())) } // NewToolError creates a new structured tool error diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 908a658..42f0013 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -5,6 +5,7 @@ import ( "log/slog" "os" + "github.com/kagent-dev/tools/internal/redact" "go.opentelemetry.io/otel/trace" ) @@ -67,21 +68,10 @@ func WithContext(ctx context.Context) *slog.Logger { return logger } -// RedactArgsForLog returns a copy of args with sensitive values redacted for logging. -// Any value immediately following "--token" is replaced with "" so tokens are not logged. +// RedactArgsForLog returns a copy of args with credential values redacted for +// logging. See redact.Args for the flags and assignments that are covered. func RedactArgsForLog(args []string) []string { - if len(args) == 0 { - return nil - } - out := make([]string, len(args)) - copy(out, args) - for i := 0; i < len(out)-1; i++ { - if out[i] == "--token" { - out[i+1] = "" - i++ // skip the redacted value - } - } - return out + return redact.Args(args) } func LogExecCommand(ctx context.Context, logger *slog.Logger, command string, args []string, caller string) { @@ -98,7 +88,7 @@ func LogExecCommandResult(ctx context.Context, logger *slog.Logger, command stri logger.Error("command execution failed", "command", command, "args", redacted, - "error", err.Error(), + "error", redact.String(err.Error()), "duration_seconds", duration, "caller", caller, ) diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index 7acb361..eae1398 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -20,6 +20,7 @@ import ( "github.com/google/jsonschema-go/jsonschema" "github.com/kagent-dev/tools/internal/metrics" + "github.com/kagent-dev/tools/internal/redact" sdk "github.com/modelcontextprotocol/go-sdk/mcp" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -183,11 +184,14 @@ func ToolMiddleware() sdk.Middleware { // and the traces disagree and the span stays neither Ok nor // Error. switch { + // Error text is redacted before it is put on the span: it can + // quote a command line, and the trace backend must never + // receive a credential. case err != nil: - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) + span.RecordError(redact.Error(err)) + span.SetStatus(codes.Error, redact.String(err.Error())) case toolErrMessage != "": - span.SetStatus(codes.Error, toolErrMessage) + span.SetStatus(codes.Error, redact.String(toolErrMessage)) default: span.SetStatus(codes.Error, "tool returned IsError") } diff --git a/internal/mcp/mcp_test.go b/internal/mcp/mcp_test.go index 6bdb883..d7b2539 100644 --- a/internal/mcp/mcp_test.go +++ b/internal/mcp/mcp_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strings" "testing" "github.com/google/jsonschema-go/jsonschema" @@ -272,3 +273,54 @@ func TestToolMiddleware_MarksSpanOkOnSuccess(t *testing.T) { t.Errorf("span status: expected Ok, got %v", got) } } + +// TestToolMiddleware_RedactsCredentialsOnSpan guards the trace backend: a tool +// error that quotes a credential (a command line with --token, a bearer +// header) must not put it in the span status or the recorded exception. +func TestToolMiddleware_RedactsCredentialsOnSpan(t *testing.T) { + const secret = "eyJhbGciOiJSUzI1NiJ9.span-secret.sig" + exp := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exp)) + prev := otel.GetTracerProvider() + otel.SetTracerProvider(tp) + t.Cleanup(func() { + otel.SetTracerProvider(prev) + _ = tp.Shutdown(context.Background()) + }) + + cases := map[string]sdk.MethodHandler{ + "tool-level error": func(_ context.Context, _ string, _ sdk.Request) (sdk.Result, error) { + return NewToolResultError("[Kubernetes] get pods --token " + secret + " failed: exit status 1"), nil + }, + "go error": func(_ context.Context, _ string, _ sdk.Request) (sdk.Result, error) { + return nil, fmt.Errorf("upstream said: Authorization: Bearer %s rejected", secret) + }, + } + for name, next := range cases { + t.Run(name, func(t *testing.T) { + exp.Reset() + _, _ = invokeMiddleware("leaky_tool", "test", next) + spans := exp.GetSpans() + if len(spans) != 1 { + t.Fatalf("expected exactly 1 span, got %d", len(spans)) + } + s := spans[0] + if s.Status.Code != codes.Error { + t.Errorf("span status: expected Error, got %v", s.Status.Code) + } + if strings.Contains(s.Status.Description, secret) { + t.Errorf("span status leaks the credential: %q", s.Status.Description) + } + if !strings.Contains(s.Status.Description, "") { + t.Errorf("span status should show the redaction, got %q", s.Status.Description) + } + for _, ev := range s.Events { + for _, a := range ev.Attributes { + if strings.Contains(a.Value.Emit(), secret) { + t.Errorf("span event %s attribute %s leaks the credential", ev.Name, a.Key) + } + } + } + }) + } +} diff --git a/internal/redact/redact.go b/internal/redact/redact.go new file mode 100644 index 0000000..10c3f6a --- /dev/null +++ b/internal/redact/redact.go @@ -0,0 +1,120 @@ +// Package redact removes credentials from command arguments and free text +// before they reach logs, span attributes, span status or tool results. +// +// It is a safety net, not the primary control: credentials should never be put +// on a command line in the first place (see internal/commands, which passes +// bearer tokens to CLIs through a per-call kubeconfig instead of --token). The +// functions here make sure that, if a credential is ever added to an argument +// list or an error message, the text that leaves the process does not carry it. +package redact + +import ( + "regexp" + "strings" +) + +// Placeholder replaces every redacted value. +const Placeholder = "" + +// sensitiveFlags take a credential (or a path to one) as their value, either as +// the next argument ("--token X") or inline ("--token=X"). +var sensitiveFlags = []string{ + "--token", + "--kube-token", + "--password", + "--client-key", +} + +// assignmentPattern matches key=value assignments whose key names a credential, +// such as "--set db.password=x" or "aws_secret=x". The value starts after "=". +var assignmentPattern = regexp.MustCompile(`(?i)(token|password|secret)=`) + +// stringFlagPattern matches a sensitive flag and its value in free text, +// separated by "=" or whitespace. +var stringFlagPattern = regexp.MustCompile(`(--(?:token|kube-token|password|client-key))(=|\s+)\S+`) + +// stringAssignmentPattern matches a credential assignment in free text. +var stringAssignmentPattern = regexp.MustCompile(`(?i)((?:token|password|secret)=)\S+`) + +// bearerPattern matches an HTTP bearer credential in free text. +var bearerPattern = regexp.MustCompile(`(?i)(bearer\s+)[A-Za-z0-9._~+/=-]+`) + +// Args returns a copy of args with credential values replaced by Placeholder. +// The value following --token, --kube-token, --password and --client-key is +// redacted, as is the inline form (--token=X), and the value of any argument +// containing an assignment whose key matches token, password or secret +// (case-insensitive). A nil or empty slice yields nil. +func Args(args []string) []string { + if len(args) == 0 { + return nil + } + out := make([]string, len(args)) + copy(out, args) + for i := 0; i < len(out); i++ { + arg := out[i] + if isSensitiveFlag(arg) { + if i+1 < len(out) { + out[i+1] = Placeholder + i++ // the value has been handled + } + continue + } + if flag, ok := sensitiveInlineFlag(arg); ok { + out[i] = flag + "=" + Placeholder + continue + } + if loc := assignmentPattern.FindStringIndex(arg); loc != nil { + out[i] = arg[:loc[1]] + Placeholder + } + } + return out +} + +// String returns s with credentials replaced by Placeholder. It covers the same +// flags and assignments as Args, written as text (for example a command line +// joined with spaces, or an error message that quotes one), and bearer +// credentials ("Bearer X"). +func String(s string) string { + if s == "" { + return s + } + s = stringFlagPattern.ReplaceAllString(s, "${1}${2}"+Placeholder) + s = stringAssignmentPattern.ReplaceAllString(s, "${1}"+Placeholder) + s = bearerPattern.ReplaceAllString(s, "${1}"+Placeholder) + return s +} + +// Error wraps err so that its message is redacted with String. It returns nil +// for a nil error. The original error stays reachable through errors.Unwrap. +func Error(err error) error { + if err == nil { + return nil + } + return &redactedError{msg: String(err.Error()), cause: err} +} + +type redactedError struct { + msg string + cause error +} + +func (e *redactedError) Error() string { return e.msg } +func (e *redactedError) Unwrap() error { return e.cause } + +func isSensitiveFlag(arg string) bool { + for _, f := range sensitiveFlags { + if arg == f { + return true + } + } + return false +} + +func sensitiveInlineFlag(arg string) (string, bool) { + for _, f := range sensitiveFlags { + if strings.HasPrefix(arg, f+"=") { + return f, true + } + } + return "", false +} diff --git a/internal/redact/redact_test.go b/internal/redact/redact_test.go new file mode 100644 index 0000000..3ea4ca4 --- /dev/null +++ b/internal/redact/redact_test.go @@ -0,0 +1,89 @@ +package redact + +import ( + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +const secret = "s3cr3t-value" + +func TestArgs(t *testing.T) { + tests := []struct { + name string + in []string + want []string + }{ + {"token", []string{"get", "pods", "--token", secret}, []string{"get", "pods", "--token", Placeholder}}, + {"token inline", []string{"get", "--token=" + secret}, []string{"get", "--token=" + Placeholder}}, + {"kube-token", []string{"list", "--kube-token", secret}, []string{"list", "--kube-token", Placeholder}}, + {"kube-token inline", []string{"list", "--kube-token=" + secret}, []string{"list", "--kube-token=" + Placeholder}}, + {"password", []string{"login", "--password", secret, "-n", "x"}, []string{"login", "--password", Placeholder, "-n", "x"}}, + {"password inline", []string{"login", "--password=" + secret}, []string{"login", "--password=" + Placeholder}}, + {"client-key", []string{"get", "--client-key", secret}, []string{"get", "--client-key", Placeholder}}, + {"client-key inline", []string{"get", "--client-key=" + secret}, []string{"get", "--client-key=" + Placeholder}}, + {"token assignment", []string{"--set", "auth.token=" + secret}, []string{"--set", "auth.token=" + Placeholder}}, + {"password assignment upper case", []string{"--set", "DB_PASSWORD=" + secret}, []string{"--set", "DB_PASSWORD=" + Placeholder}}, + {"secret assignment in list", []string{"--set", "a=1,aws.secret=" + secret + ",b=2"}, []string{"--set", "a=1,aws.secret=" + Placeholder}}, + {"flag at end without value", []string{"get", "--token"}, []string{"get", "--token"}}, + {"unrelated args unchanged", []string{"get", "pods", "-n", "default", "app=web"}, []string{"get", "pods", "-n", "default", "app=web"}}, + {"value after flag is not inspected again", []string{"--token", "--password", "x"}, []string{"--token", Placeholder, "x"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + in := append([]string(nil), tt.in...) + got := Args(tt.in) + assert.Equal(t, tt.want, got) + assert.Equal(t, in, tt.in, "input must not be modified") + for _, a := range got { + assert.NotContains(t, a, secret) + } + }) + } + assert.Nil(t, Args(nil)) + assert.Nil(t, Args([]string{})) +} + +func TestString(t *testing.T) { + inputs := []string{ + "get pods --token " + secret + " -n default", + "get pods --token=" + secret, + "list --kube-token " + secret, + "list --kube-token=" + secret, + "login --password " + secret, + "login --password=" + secret, + "get --client-key " + secret, + "get --client-key=" + secret, + "install x --set auth.Token=" + secret, + "PASSWORD=" + secret, + "aws_secret=" + secret, + "Authorization: Bearer " + secret, + "[Kubernetes] get pods --token\t" + secret + " failed: exit status 1", + } + for _, in := range inputs { + t.Run(in[:min(len(in), 30)], func(t *testing.T) { + got := String(in) + assert.NotContains(t, got, secret) + assert.Contains(t, got, Placeholder) + }) + } + + t.Run("text without credentials unchanged", func(t *testing.T) { + in := "[Kubernetes] get pods -n default failed: exit status 1" + assert.Equal(t, in, String(in)) + }) + t.Run("idempotent", func(t *testing.T) { + in := "get --token=" + secret + assert.Equal(t, String(in), String(String(in))) + }) +} + +func TestError(t *testing.T) { + assert.NoError(t, Error(nil)) + base := errors.New("kubectl get --token " + secret + " failed") + err := Error(base) + assert.False(t, strings.Contains(err.Error(), secret)) + assert.ErrorIs(t, err, base) +} diff --git a/internal/telemetry/middleware.go b/internal/telemetry/middleware.go index a00a727..bed0b66 100644 --- a/internal/telemetry/middleware.go +++ b/internal/telemetry/middleware.go @@ -4,6 +4,7 @@ import ( "context" "net/http" + "github.com/kagent-dev/tools/internal/redact" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -95,9 +96,13 @@ func StartSpan(ctx context.Context, operationName string, attrs ...attribute.Key return ctx, span } +// RecordError records err on span and marks the span failed. The recorded +// exception message and the status description are redacted (redact.String), +// so a credential that ends up in an error message does not reach the trace +// backend. func RecordError(span trace.Span, err error, message string) { - span.RecordError(err) - span.SetStatus(codes.Error, message) + span.RecordError(redact.Error(err)) + span.SetStatus(codes.Error, redact.String(message)) } func RecordSuccess(span trace.Span, message string) { diff --git a/pkg/k8s/k8s_test.go b/pkg/k8s/k8s_test.go index 57d8566..6d6eab6 100644 --- a/pkg/k8s/k8s_test.go +++ b/pkg/k8s/k8s_test.go @@ -1137,8 +1137,8 @@ func TestBearerTokenPassthrough(t *testing.T) { t.Run("get resources with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `NAME READY STATUS RESTARTS AGE` - mock.AddCommandString("kubectl", []string{"get", "pods", "-o", "wide", "--token", "test-token-123"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"get", "pods", "-o", "wide"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("test-token-123") @@ -1147,19 +1147,18 @@ func TestBearerTokenPassthrough(t *testing.T) { assert.NotNil(t, result) assert.False(t, result.IsError) - // Verify the command was executed with the token + // The token reaches kubectl only through the per-call kubeconfig callLog := mock.GetCallLog() require.Len(t, callLog, 1) assert.Equal(t, "kubectl", callLog[0].Command) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "test-token-123") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "test-token-123") }) t.Run("scale deployment with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment scaled` - mock.AddCommandString("kubectl", []string{"scale", "deployment", "test-deployment", "--replicas", "5", "-n", "default", "--token", "my-auth-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"scale", "deployment", "test-deployment", "--replicas", "5", "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("my-auth-token") @@ -1168,19 +1167,18 @@ func TestBearerTokenPassthrough(t *testing.T) { assert.NotNil(t, result) assert.False(t, result.IsError) - // Verify the command was executed with the token + // The token reaches kubectl only through the per-call kubeconfig callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "my-auth-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "my-auth-token") }) t.Run("get pod logs with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `log line 1 log line 2` - mock.AddCommandString("kubectl", []string{"logs", "test-pod", "-n", "default", "--tail", "50", "--token", "logs-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"logs", "test-pod", "-n", "default", "--tail", "50"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("logs-token") @@ -1191,15 +1189,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "logs-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "logs-token") }) t.Run("delete resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment deleted` - mock.AddCommandString("kubectl", []string{"delete", "deployment", "test-deployment", "-n", "default", "--token", "delete-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"delete", "deployment", "test-deployment", "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("delete-token") @@ -1210,15 +1207,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "delete-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "delete-token") }) t.Run("patch resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment patched` - mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default", "--token", "patch-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"patch", "deployment", "test-deployment", "--type=strategic", "-p", `{"spec":{"replicas":5}}`, "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("patch-token") @@ -1229,15 +1225,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "patch-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "patch-token") }) t.Run("describe resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `Name: test-deployment` - mock.AddCommandString("kubectl", []string{"describe", "deployment", "test-deployment", "-n", "default", "--token", "describe-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"describe", "deployment", "test-deployment", "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("describe-token") @@ -1248,15 +1243,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "describe-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "describe-token") }) t.Run("rollout with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/myapp restarted` - mock.AddCommandString("kubectl", []string{"rollout", "restart", "deployment/myapp", "-n", "default", "--token", "rollout-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"rollout", "restart", "deployment/myapp", "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("rollout-token") @@ -1267,15 +1261,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "rollout-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "rollout-token") }) t.Run("get events with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `{"items": []}` - mock.AddCommandString("kubectl", []string{"get", "events", "-o", "json", "--all-namespaces", "--token", "events-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"get", "events", "-o", "json", "--all-namespaces"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("events-token") @@ -1286,15 +1279,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "events-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "events-token") }) t.Run("exec command with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `total 8` - mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls -la", "--token", "exec-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"exec", "mypod", "-n", "default", "--", "ls -la"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("exec-token") @@ -1305,15 +1297,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "exec-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "exec-token") }) t.Run("annotate resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment annotated` - mock.AddCommandString("kubectl", []string{"annotate", "deployment", "test-deployment", "key1=value1", "--token", "annotate-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"annotate", "deployment", "test-deployment", "key1=value1"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("annotate-token") @@ -1324,15 +1315,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "annotate-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "annotate-token") }) t.Run("label resource with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment labeled` - mock.AddCommandString("kubectl", []string{"label", "deployment", "test-deployment", "env=prod", "--token", "label-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"label", "deployment", "test-deployment", "env=prod"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("label-token") @@ -1343,15 +1333,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "label-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "label-token") }) t.Run("api resources with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `NAME SHORTNAMES APIVERSION NAMESPACED KIND` - mock.AddCommandString("kubectl", []string{"api-resources", "--token", "api-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"api-resources"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("api-token") @@ -1362,15 +1351,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "api-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "api-token") }) t.Run("cluster configuration with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `{"current-context": "default"}` - mock.AddCommandString("kubectl", []string{"config", "view", "-o", "json", "--token", "config-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"config", "view", "-o", "json"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("config-token") @@ -1381,15 +1369,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "config-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "config-token") }) t.Run("remove annotation with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment annotated` - mock.AddCommandString("kubectl", []string{"annotate", "deployment", "test-deployment", "key1-", "--token", "remove-anno-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"annotate", "deployment", "test-deployment", "key1-"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("remove-anno-token") @@ -1400,15 +1387,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "remove-anno-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "remove-anno-token") }) t.Run("remove label with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment labeled` - mock.AddCommandString("kubectl", []string{"label", "deployment", "test-deployment", "env-", "--token", "remove-label-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"label", "deployment", "test-deployment", "env-"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("remove-label-token") @@ -1419,15 +1405,14 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "remove-label-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "remove-label-token") }) t.Run("create resource from URL with bearer token", func(t *testing.T) { mock := cmd.NewMockShellExecutor() expectedOutput := `deployment.apps/test-deployment created` - mock.AddCommandString("kubectl", []string{"create", "-f", "https://example.com/manifest.yaml", "-n", "default", "--token", "url-token"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + mock.AddCommandString("kubectl", []string{"create", "-f", "https://example.com/manifest.yaml", "-n", "default"}, expectedOutput, nil) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("url-token") @@ -1438,8 +1423,7 @@ log line 2` callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "url-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "url-token") }) t.Run("apply manifest with bearer token", func(t *testing.T) { @@ -1451,7 +1435,7 @@ metadata: expectedOutput := `pod/test-pod created` // Use partial matcher since temp file name is dynamic mock.AddPartialMatcherString("kubectl", []string{"apply", "-f"}, expectedOutput, nil) - ctx := cmd.WithShellExecutor(ctx, mock) + ctx, seen := withPassthroughKubeconfig(t, ctx, mock) k8sTool := newTestK8sToolWithPassthrough(true) req := requestWithBearerToken("apply-token") @@ -1462,8 +1446,7 @@ metadata: callLog := mock.GetCallLog() require.Len(t, callLog, 1) - assert.Contains(t, callLog[0].Args, "--token") - assert.Contains(t, callLog[0].Args, "apply-token") + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, "apply-token") }) t.Run("returns error when passthrough true and authorization header missing", func(t *testing.T) { diff --git a/pkg/k8s/token_passthrough_test.go b/pkg/k8s/token_passthrough_test.go new file mode 100644 index 0000000..4db8bfc --- /dev/null +++ b/pkg/k8s/token_passthrough_test.go @@ -0,0 +1,197 @@ +package k8s + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/kagent-dev/tools/internal/cmd" + mcp "github.com/kagent-dev/tools/internal/mcp" + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + "k8s.io/client-go/tools/clientcmd" +) + +// baseKubeconfig is the configured kubeconfig the per-call kubeconfig is +// derived from in these tests. +const baseKubeconfig = `apiVersion: v1 +kind: Config +clusters: +- name: test-cluster + cluster: + server: https://127.0.0.1:6443 + insecure-skip-tls-verify: true +contexts: +- name: test-context + context: + cluster: test-cluster + user: server-identity +current-context: test-context +users: +- name: server-identity + user: + token: server-identity-token-must-not-be-used +` + +// passthroughKubeconfig records what a kubectl call saw in its --kubeconfig +// while it ran: the file is removed again before the call returns. +type passthroughKubeconfig struct { + mu sync.Mutex + path string + fileMode os.FileMode + dirMode os.FileMode + tokenMode os.FileMode + token string + inline string // a token written into the kubeconfig itself (must stay empty) + server string +} + +// kubeconfigCapturingExecutor reads the per-call kubeconfig during Exec, then +// forwards the call to the mock without the --kubeconfig pair, so the mock's +// expectations are the arguments a caller asked for. +type kubeconfigCapturingExecutor struct { + t *testing.T + next cmd.ShellExecutor + seen *passthroughKubeconfig +} + +func (e *kubeconfigCapturingExecutor) Exec(ctx context.Context, command string, args ...string) ([]byte, error) { + var rest []string + for i := 0; i < len(args); i++ { + if args[i] == "--kubeconfig" && i+1 < len(args) { + e.capture(args[i+1]) + i++ + continue + } + rest = append(rest, args[i]) + } + return e.next.Exec(ctx, command, rest...) +} + +func (e *kubeconfigCapturingExecutor) capture(path string) { + e.seen.mu.Lock() + defer e.seen.mu.Unlock() + e.seen.path = path + fi, err := os.Stat(path) + require.NoError(e.t, err, "per-call kubeconfig must exist while the command runs") + e.seen.fileMode = fi.Mode().Perm() + di, err := os.Stat(filepath.Dir(path)) + require.NoError(e.t, err) + e.seen.dirMode = di.Mode().Perm() + + cfg, err := clientcmd.LoadFromFile(path) + require.NoError(e.t, err) + ctx := cfg.Contexts[cfg.CurrentContext] + require.NotNil(e.t, ctx) + e.seen.server = cfg.Clusters[ctx.Cluster].Server + user := cfg.AuthInfos[ctx.AuthInfo] + require.NotNil(e.t, user) + e.seen.inline = user.Token + ti, err := os.Stat(user.TokenFile) + require.NoError(e.t, err) + e.seen.tokenMode = ti.Mode().Perm() + b, err := os.ReadFile(user.TokenFile) + require.NoError(e.t, err) + e.seen.token = string(b) +} + +// withPassthroughKubeconfig configures a base kubeconfig and installs an +// executor that records the per-call kubeconfig before delegating to mock. +func withPassthroughKubeconfig(t *testing.T, ctx context.Context, mock cmd.ShellExecutor) (context.Context, *passthroughKubeconfig) { + t.Helper() + path := filepath.Join(t.TempDir(), "config") + require.NoError(t, os.WriteFile(path, []byte(baseKubeconfig), 0o600)) + t.Setenv("KUBECONFIG", path) + seen := &passthroughKubeconfig{} + return cmd.WithShellExecutor(ctx, &kubeconfigCapturingExecutor{t: t, next: mock, seen: seen}), seen +} + +// assertTokenOnlyInKubeconfig checks that token reached kubectl only through +// the per-call kubeconfig, never on argv, and that the files are gone. +func assertTokenOnlyInKubeconfig(t *testing.T, args []string, seen *passthroughKubeconfig, token string) { + t.Helper() + for _, a := range args { + assert.NotContains(t, a, token, "token must not be on argv") + assert.NotEqual(t, "--token", a) + } + seen.mu.Lock() + defer seen.mu.Unlock() + require.NotEmpty(t, seen.path, "kubectl must be given a per-call --kubeconfig") + assert.Equal(t, token, seen.token, "token file must hold the caller's token") + assert.Empty(t, seen.inline, "kubeconfig must reference the token, not contain it") + assert.Equal(t, "https://127.0.0.1:6443", seen.server, "cluster must come from the base kubeconfig") + assert.Equal(t, os.FileMode(0o600), seen.fileMode) + assert.Equal(t, os.FileMode(0o600), seen.tokenMode) + assert.Equal(t, os.FileMode(0o700), seen.dirMode) + _, err := os.Stat(filepath.Dir(seen.path)) + assert.True(t, os.IsNotExist(err), "per-call kubeconfig directory must be removed after the call") +} + +// TestBearerTokenNotInFailedResult covers the reported leak: a failing kubectl +// call's tool result must not contain the caller's token. +func TestBearerTokenNotInFailedResult(t *testing.T) { + const token = "eyJhbGciOiJSUzI1NiJ9.leak-check-token.sig" + mock := cmd.NewMockShellExecutor() + mock.AddPartialMatcherString("kubectl", []string{"get", "secrets"}, `Error from server (Forbidden): secrets is forbidden`, assert.AnError) + ctx, seen := withPassthroughKubeconfig(t, context.Background(), mock) + + exp := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exp)) + prev := otel.GetTracerProvider() + otel.SetTracerProvider(tp) + t.Cleanup(func() { + otel.SetTracerProvider(prev) + _ = tp.Shutdown(context.Background()) + }) + + // Run the handler behind the same middleware the server installs, so the + // tool span (status from the result text) is covered too. + k8sTool := newTestK8sToolWithPassthrough(true) + req := requestWithBearerToken(token) + req.Params = &sdkmcp.CallToolParamsRaw{Name: "k8s_get_resources"} + h := mcp.ToolMiddleware()(func(ctx context.Context, _ string, r sdkmcp.Request) (sdkmcp.Result, error) { + res, _, err := k8sTool.handleKubectlGetEnhanced(ctx, r.(*sdkmcp.CallToolRequest), getResourcesInput{ResourceType: "secrets"}) + return res, err + }) + res, err := h(ctx, "tools/call", req) + require.NoError(t, err) + result, ok := res.(*sdkmcp.CallToolResult) + require.True(t, ok) + require.NotNil(t, result) + assert.True(t, result.IsError) + text := getResultText(result) + assert.NotContains(t, text, token) + assert.Contains(t, text, "get secrets") + for _, c := range result.Content { + if tc, ok := c.(*sdkmcp.TextContent); ok { + assert.False(t, strings.Contains(tc.Text, token)) + } + } + callLog := mock.GetCallLog() + require.Len(t, callLog, 1) + assertTokenOnlyInKubeconfig(t, callLog[0].Args, seen, token) + + spans := exp.GetSpans() + names := map[string]bool{} + for _, s := range spans { + names[s.Name] = true + assert.NotContains(t, s.Status.Description, token, "span %s status", s.Name) + for _, a := range s.Attributes { + assert.NotContains(t, a.Value.Emit(), token, "span %s attribute %s", s.Name, a.Key) + } + for _, ev := range s.Events { + for _, a := range ev.Attributes { + assert.NotContains(t, a.Value.Emit(), token, "span %s event %s", s.Name, ev.Name) + } + } + } + assert.True(t, names["mcp.tool.k8s_get_resources"], "tool span recorded, got %v", names) + assert.True(t, names["commands.execute"], "command span recorded, got %v", names) +}