From 080ea5e67d6ce5743b8bec1b2837598458f65096 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:04:11 +0000 Subject: [PATCH] fix(daemon): mask the API token in the startup config log The daemon logs its whole config at Debug on stdout at startup with slog.Any("config", cfg), which printed Token (and the tokens of debug Endpoints and the exclude patterns) in plaintext to the systemd journal, ~/.shelltime/logs/shelltime-daemon.log, or wherever stdout was redirected. ShellTimeConfig now implements slog.LogValuer, so any slog call that logs it writes a group of the fields worth seeing instead: tokens masked (first and last 4 characters), proxy URL redacted, exclude patterns only counted (they may contain the very strings they keep out of history), debug endpoints without their tokens, and nested configs by value rather than as pointer addresses. New fields stay out of the log until added there. RedactProxyURL returned a bare "user:pass@host:port" (accepted by ParseProxyURL) and URLs that fail to parse (a password with an unescaped '/' or '%') unchanged. It now reads a scheme-less URL as http, like ParseProxyURL, and otherwise hides everything before the last '@'. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VqGMwLkjnydsiEvDrRcMt8 --- model/config_log.go | 76 +++++++++++++++++++++++++++++++++++++++ model/config_log_test.go | 71 ++++++++++++++++++++++++++++++++++++ model/http_client.go | 26 ++++++++++++-- model/http_client_test.go | 5 +++ 4 files changed, 175 insertions(+), 3 deletions(-) create mode 100644 model/config_log.go create mode 100644 model/config_log_test.go diff --git a/model/config_log.go b/model/config_log.go new file mode 100644 index 0000000..d23b3a5 --- /dev/null +++ b/model/config_log.go @@ -0,0 +1,76 @@ +package model + +import "log/slog" + +// LogValue implements slog.LogValuer, so a config passed to slog (for example +// slog.Any("config", cfg)) is logged without its secrets: tokens are masked, +// proxy credentials redacted and exclude patterns, which may hold the very +// strings they keep out of history, only counted. Fields added to +// ShellTimeConfig are not logged until they are added here. +func (c ShellTimeConfig) LogValue() slog.Value { + endpoints := make([]string, 0, len(c.Endpoints)) + for _, e := range c.Endpoints { + endpoints = append(endpoints, e.APIEndpoint) + } + + attrs := []slog.Attr{ + slog.String("token", maskToken(c.Token)), + slog.String("apiEndpoint", c.APIEndpoint), + slog.String("webEndpoint", c.WebEndpoint), + slog.Int("flushCount", c.FlushCount), + slog.Int("gcTime", c.GCTime), + boolPtrAttr("dataMasking", c.DataMasking), + boolPtrAttr("enableMetrics", c.EnableMetrics), + boolPtrAttr("encrypted", c.Encrypted), + slog.Any("endpoints", endpoints), + slog.Int("excludePatterns", len(c.Exclude)), + slog.String("socketPath", c.SocketPath), + } + if c.AI != nil { + attrs = append(attrs, slog.Group("ai", + boolPtrAttr("showTips", c.AI.ShowTips), + boolPtrAttr("shareContext", c.AI.ShareContext), + slog.Bool("agentView", c.AI.Agent.View), + slog.Bool("agentEdit", c.AI.Agent.Edit), + slog.Bool("agentDelete", c.AI.Agent.Delete), + )) + } + if c.AICodeOtel != nil { + attrs = append(attrs, slog.Group("aiCodeOtel", + boolPtrAttr("enabled", c.AICodeOtel.Enabled), + slog.Int("grpcPort", c.AICodeOtel.GRPCPort), + boolPtrAttr("debug", c.AICodeOtel.Debug), + )) + } + if c.CodeTracking != nil { + attrs = append(attrs, slog.Group("codeTracking", + boolPtrAttr("enabled", c.CodeTracking.Enabled), + slog.String("apiEndpoint", c.CodeTracking.APIEndpoint), + slog.String("token", maskToken(c.CodeTracking.Token)), + )) + } + if c.LogCleanup != nil { + attrs = append(attrs, slog.Group("logCleanup", + boolPtrAttr("enabled", c.LogCleanup.Enabled), + slog.Int64("thresholdMB", c.LogCleanup.ThresholdMB), + )) + } + if c.Storage != nil { + attrs = append(attrs, slog.Group("storage", slog.String("engine", c.Storage.Engine))) + } + if c.Proxy != nil { + attrs = append(attrs, slog.Group("proxy", + slog.String("url", RedactProxyURL(c.Proxy.URL)), + slog.Any("noProxy", c.Proxy.NoProxy), + )) + } + return slog.GroupValue(attrs...) +} + +// boolPtrAttr logs an optional bool by value; slog would print the pointer. +func boolPtrAttr(key string, b *bool) slog.Attr { + if b == nil { + return slog.Any(key, nil) + } + return slog.Bool(key, *b) +} diff --git a/model/config_log_test.go b/model/config_log_test.go new file mode 100644 index 0000000..6f490da --- /dev/null +++ b/model/config_log_test.go @@ -0,0 +1,71 @@ +package model + +import ( + "bytes" + "context" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShellTimeConfigLogValue_HidesSecrets(t *testing.T) { + const ( + token = "st_v1_0123456789abcdefSECRETtokenBODY9876" + endpointToken = "st_v1_endpointSECRETtoken0000000000000000" + codeToken = "st_v1_codetrackingSECRETtoken00000000000" + proxyPassword = "proxySECRETpass" + excludeSecret = "hunter2SECRET" + ) + enabled := true + cfg := ShellTimeConfig{ + Token: token, + APIEndpoint: "https://api.shelltime.xyz", + Endpoints: []Endpoint{{APIEndpoint: "https://debug.example.com", Token: endpointToken}}, + Exclude: []string{".*" + excludeSecret + ".*"}, + CodeTracking: &CodeTracking{Enabled: &enabled, Token: codeToken}, + AICodeOtel: &AICodeOtel{Enabled: &enabled, GRPCPort: 54027}, + Proxy: &ProxyConfig{URL: "socks5://user:" + proxyPassword + "@127.0.0.1:1080"}, + SocketPath: "/tmp/shelltime.sock", + } + + handlers := map[string]func(*bytes.Buffer) slog.Handler{ + "text": func(b *bytes.Buffer) slog.Handler { + return slog.NewTextHandler(b, &slog.HandlerOptions{Level: slog.LevelDebug}) + }, + "json": func(b *bytes.Buffer) slog.Handler { + return slog.NewJSONHandler(b, &slog.HandlerOptions{Level: slog.LevelDebug}) + }, + } + for name, newHandler := range handlers { + for form, value := range map[string]any{"value": cfg, "pointer": &cfg} { + t.Run(name+"/"+form, func(t *testing.T) { + var buf bytes.Buffer + // The same call the daemon makes at startup. + slog.New(newHandler(&buf)).DebugContext(context.Background(), "daemon.config", slog.Any("config", value)) + out := buf.String() + + for _, secret := range []string{token, endpointToken, codeToken, proxyPassword, excludeSecret} { + assert.NotContains(t, out, secret) + } + assert.Contains(t, out, "st_v") + assert.Contains(t, out, "9876") + assert.Contains(t, out, "https://api.shelltime.xyz") + assert.Contains(t, out, "https://debug.example.com") + assert.Contains(t, out, "socks5://user:xxxxx@127.0.0.1:1080") + assert.Contains(t, out, "54027") + assert.Contains(t, out, "/tmp/shelltime.sock") + }) + } + } +} + +func TestShellTimeConfigLogValue_EmptyConfig(t *testing.T) { + var buf bytes.Buffer + slog.New(slog.NewTextHandler(&buf, nil)).Info("daemon.config", slog.Any("config", ShellTimeConfig{})) + out := buf.String() + + assert.Contains(t, out, "config.token=\"\"") + assert.Contains(t, out, "config.dataMasking=") + assert.NotContains(t, out, "config.proxy") +} diff --git a/model/http_client.go b/model/http_client.go index eceb6ce..ec2d1a7 100644 --- a/model/http_client.go +++ b/model/http_client.go @@ -81,12 +81,32 @@ func ParseProxyURL(raw string) (*url.URL, error) { } // RedactProxyURL hides the password of a proxy URL for display and logging. +// Like ParseProxyURL it reads a URL without a scheme as http, so a bare +// "user:pass@host:port" is redacted too. When the URL doesn't parse into user +// info (a password with an unescaped '/', '%', '?' or '#'), everything between +// the scheme and the last '@' is hidden. func RedactProxyURL(raw string) string { - u, err := url.Parse(raw) - if err != nil || u.User == nil { + bare := !strings.Contains(raw, "://") + toParse := raw + if bare { + toParse = "http://" + raw + } + if u, err := url.Parse(toParse); err == nil && u.User != nil { + if bare { + return strings.TrimPrefix(u.Redacted(), "http://") + } + return u.Redacted() + } + + at := strings.LastIndex(raw, "@") + if at < 0 { return raw } - return u.Redacted() + start := 0 + if i := strings.Index(raw, "://"); i >= 0 && i < at { + start = i + len("://") + } + return raw[:start] + "xxxxx" + raw[at:] } // HTTPTransport returns the shared, proxy-aware base transport. diff --git a/model/http_client_test.go b/model/http_client_test.go index 21dbf93..e6273b9 100644 --- a/model/http_client_test.go +++ b/model/http_client_test.go @@ -67,6 +67,11 @@ func TestRedactProxyURL(t *testing.T) { assert.Equal(t, "socks5://u:xxxxx@h:1", RedactProxyURL("socks5://u:secret@h:1")) assert.Equal(t, "http://h:1", RedactProxyURL("http://h:1")) assert.Equal(t, "", RedactProxyURL("")) + assert.Equal(t, "h:1", RedactProxyURL("h:1")) + assert.Equal(t, "u:xxxxx@h:1", RedactProxyURL("u:secret@h:1")) + assert.Equal(t, "http://xxxxx@h:1", RedactProxyURL("http://u:sec/ret@h:1")) + assert.Equal(t, "http://xxxxx@h:1", RedactProxyURL("http://u:sec%zzret@h:1")) + assert.Equal(t, "xxxxx@h:1", RedactProxyURL("u:sec/ret@h:1")) } func TestConfigureProxy_NilOrEmptyKeepsEnvBehavior(t *testing.T) {