[braghettos]

Summary

Security fix for TOKEN_PASSTHROUGH=true. The k8s tools passed the caller's bearer token to kubectl as --token <token> on the command line. When a command failed, the joined arguments became the Operation of the returned ToolError, so the token ended up in:

• the tool result returned to the MCP client (and from there in the agent's context, its model provider's requests and its session history);
• the tool span's status description and recorded exception;
• the container's process list while kubectl ran;
• the cache key of cached commands, which is logged at info level and set as a span attribute.

A second problem showed up during testing. --token only overrides the token in the configured kubeconfig. If that kubeconfig's user also has a client certificate, kubectl sends both, and the API server can authenticate the request as the certificate's identity instead of the caller. In that setup, passthrough did not limit what a caller could do.

Change

Token off argv (internal/commands). CommandBuilder.Build no longer emits a token. For a call with a token, Execute writes a per-call kubeconfig to a fresh 0700 temporary directory, passes it with --kubeconfig, and removes the directory when the call returns, whether it succeeded, failed, was cancelled or hit its deadline. The kubeconfig is derived from the configured one: the explicit path, else $KUBECONFIG or ~/.kube/config, else the in-cluster environment (KUBERNETES_SERVICE_HOST/PORT, service account CA and namespace).

• The cluster entry of the selected context is copied unchanged (server, CA data or file, TLS settings, proxy-url, and so on). The context namespace is kept.
• The only user entry is tokenFile:, pointing at a 0600 file in the same directory. Nothing else from the base user is copied: no client certificate or key, exec plugin, auth provider, basic auth, or impersonation (as, as-groups, as-uid, as-user-extra).
• tokenFile is used instead of an inline token: so the kubeconfig itself never contains the token. A CLI that prints its kubeconfig, including kubectl config view --raw, shows only the file's path.
• Error messages name the configured kubeconfig, not the temporary path, which no longer exists by the time the error is read.

Related fixes in the builder.

• The builder's flags (--namespace, --kubeconfig, ...) now go before the first --. Before, a kubectl exec <pod> -- <cmd> call (and the kubectl run ... -- sleep used by the connectivity check) got them after --: they were passed to the remote command and kubectl ignored them.
• The cache key of a token-carrying command now includes a per-process HMAC-SHA256 fingerprint of the token instead of the token. Cached results stay separate per caller.

Defense in depth (new internal/redact).

• redact.Args covers the value after --token, --kube-token, --password and --client-key, their --flag=value forms, and any argument containing a key=value whose key matches (?i)(token|password|secret). It is applied to logged arguments and to the Operation of command errors. logger.RedactArgsForLog now delegates to it.
• redact.String applies the same patterns, plus Bearer <credential>, to free text. It is applied in ToolError.Error, ToolError.ToMCPResult, the tool span status and recorded exception in ToolMiddleware, telemetry.RecordError, and the shell executor's failure log (error and output).

Behaviour notes

• Under passthrough, k8s_get_cluster_configuration (kubectl config view) now shows the per-call kubeconfig: one cluster and context, user kagent-token-passthrough with a tokenFile. Before, it showed the server's whole kubeconfig.
• Under passthrough, a call fails before kubectl runs if no configuration can be resolved, or if the selected context or its cluster does not exist. The error does not contain the token.
• The temporary files go to $TMPDIR (/tmp by default). The Helm chart already mounts an emptyDir there.
• Only the k8s provider forwards the token today. The builder handles any CLI that accepts --kubeconfig (kubectl, helm, istioctl, cilium).

Testing

• make lint: 0 issues. go vet ./... passes. go test -tags=test ./pkg/... ./internal/... ./cmd/... passes. -race passes on the changed packages.
• New unit tests:
  • no token on argv for passthrough calls (the existing TestBearerTokenPassthrough cases now assert this, through an executor that reads the per-call kubeconfig while the command runs);
  • the per-call kubeconfig and token file have mode 0600 in a 0700 directory, hold the token, and are gone after success, failure, cancellation and deadline;
  • the derived user carries exactly one credential (the token file) and no impersonation, even when the base user has client certificates, an exec plugin, basic auth and impersonation. The cluster fields are preserved;
  • context selection, relative CA path resolution, in-cluster derivation, and failure when nothing is configured;
  • a failing call's tool result, error string, span status, span attributes and span events contain no token. This is checked with an in-memory span exporter through ToolMiddleware and the k8s handler;
  • redaction of every listed flag and pattern;
  • the cache is per token;
  • builder flags are placed before --.
• Manual check against a kind cluster, using the built binary with TOKEN_PASSTHROUGH=true, a ServiceAccount token that may list pods in one namespace, and a kubectl wrapper on PATH that records argv:
  • the allowed call succeeds and the forbidden call fails;
  • no tool result, kubectl argv, process-list sample or server log line contains the token, and no temporary kubeconfig is left behind;
  • on a forbidden call, kubectl and helm print only the API server's Forbidden message. kubectl -v=9 masks the Authorization header;
  • run against the unpatched code, the same check reported the token on argv and in a failing call's tool result, and the "forbidden" call succeeded through the base kubeconfig's client certificate.

🤖 Generated with Claude Code

https://claude.ai/code/session_016RAxvkp7wRXwXDx4Lx8STi

[dimetron]

@braghettos thanks for both PRs! #103 and #107 both change internal/mcp/mcp.go and internal/telemetry/middleware.go (ToolMiddleware spans and the HTTP middleware), so whichever merges second needs a rebase, and the two need to be reviewed together anyway (for example span attributes and redaction).

Could you combine them into a single PR? You could stack #103 on top of #107 and close one of them, or squash both into one branch. Please keep the tests from both (cmd/tracing_test.go, token_kubeconfig_test.go, redact_test.go). Review found no blocking issues in either PR, so once they are combined and CI is green it should be ready to approve.

[dimetron]

LGTM. Reviewed for token leakage (argv, logs, errors, spans), temp-file permissions and cleanup, cache keying per token, and behaviour without a token. No issues found. #103 will need a rebase on internal/mcp/mcp.go and internal/telemetry/middleware.go after this merges.