From 754d75ea079aa569d8e7dd80469a1e8697fd6857 Mon Sep 17 00:00:00 2001 From: Ryota Ikezawa Date: Thu, 29 Jan 2026 20:27:43 +0900 Subject: [PATCH] fix(security): redact webhook bearer token in watch status output The hook_token was printed in plaintext via `gmail watch status`, which could leak the bearer token through terminal scrollback, CI logs, or screen sharing. Token is now masked by default (first 4 chars + length) in both text and JSON output. A new `--show-secrets` flag on the status subcommand reveals the full value when explicitly requested. Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 1 + internal/cmd/gmail_watch_cmds.go | 89 +++++++++----- internal/cmd/gmail_watch_cmds_errors_test.go | 2 +- internal/cmd/gmail_watch_helpers_test.go | 4 +- internal/cmd/gmail_watch_redact_test.go | 123 +++++++++++++++++++ 5 files changed, 182 insertions(+), 37 deletions(-) create mode 100644 internal/cmd/gmail_watch_redact_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c8ee14f..bbfaef3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ ### Fixed - Build: refresh the dependency stack to Go 1.26.1, current Go indirects, GitHub Actions v6/v7 pins, and current Cloudflare worker dependencies. +- Security: redact stored Gmail watch webhook bearer tokens in `gmail watch status` text and JSON output unless `--show-secrets` is set. (#136) — thanks @paveg. - Calendar: respond patches only attendees to avoid custom reminders validation errors. (#265) — thanks @sebasrodriguez. - Contacts: fix grouped parameter types in CRUD helpers to restore builds on newer Go toolchains. (#355) — thanks @laihenyi. - Timezone: embed the IANA timezone database so Windows builds can resolve calendar timezones correctly. (#388) — thanks @visionik. diff --git a/internal/cmd/gmail_watch_cmds.go b/internal/cmd/gmail_watch_cmds.go index 1d12f7a..4f6df9c 100644 --- a/internal/cmd/gmail_watch_cmds.go +++ b/internal/cmd/gmail_watch_cmds.go @@ -105,10 +105,12 @@ func (c *GmailWatchStartCmd) Run(ctx context.Context, kctx *kong.Context, flags return err } - return writeWatchState(ctx, state) + return writeWatchState(ctx, state, false) } -type GmailWatchStatusCmd struct{} +type GmailWatchStatusCmd struct { + ShowSecrets bool `help:"Show secret values (e.g. hook token) in plaintext"` +} func (c *GmailWatchStatusCmd) Run(ctx context.Context, flags *RootFlags) error { account, err := requireAccount(flags) @@ -119,7 +121,7 @@ func (c *GmailWatchStatusCmd) Run(ctx context.Context, flags *RootFlags) error { if err != nil { return err } - return writeWatchState(ctx, store.Get()) + return writeWatchState(ctx, store.Get(), c.ShowSecrets) } type GmailWatchRenewCmd struct { @@ -178,7 +180,7 @@ func (c *GmailWatchRenewCmd) Run(ctx context.Context, flags *RootFlags) error { return err } - return writeWatchState(ctx, updated) + return writeWatchState(ctx, updated, false) } type GmailWatchStopCmd struct{} @@ -386,56 +388,75 @@ func (c *GmailWatchServeCmd) Run(ctx context.Context, kctx *kong.Context, flags return listenAndServe(httpServer) } -func writeWatchState(ctx context.Context, state gmailWatchState) error { +func writeWatchState(ctx context.Context, state gmailWatchState, showSecrets bool) error { + displayState := state + if !showSecrets { + displayState = redactWatchStateSecrets(state) + } if outfmt.IsJSON(ctx) { - return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"watch": state}) + return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"watch": displayState}) } u := ui.FromContext(ctx) - u.Out().Printf("account\t%s", state.Account) - u.Out().Printf("topic\t%s", state.Topic) - if len(state.Labels) > 0 { - u.Out().Printf("labels\t%s", strings.Join(state.Labels, ",")) + u.Out().Printf("account\t%s", displayState.Account) + u.Out().Printf("topic\t%s", displayState.Topic) + if len(displayState.Labels) > 0 { + u.Out().Printf("labels\t%s", strings.Join(displayState.Labels, ",")) } - u.Out().Printf("history_id\t%s", state.HistoryID) - if state.ExpirationMs > 0 { - u.Out().Printf("expiration\t%s", formatUnixMillis(state.ExpirationMs)) + u.Out().Printf("history_id\t%s", displayState.HistoryID) + if displayState.ExpirationMs > 0 { + u.Out().Printf("expiration\t%s", formatUnixMillis(displayState.ExpirationMs)) } - if state.ProviderExpirationMs > 0 { - u.Out().Printf("provider_expiration\t%s", formatUnixMillis(state.ProviderExpirationMs)) + if displayState.ProviderExpirationMs > 0 { + u.Out().Printf("provider_expiration\t%s", formatUnixMillis(displayState.ProviderExpirationMs)) } - if state.RenewAfterMs > 0 { - u.Out().Printf("renew_after\t%s", formatUnixMillis(state.RenewAfterMs)) + if displayState.RenewAfterMs > 0 { + u.Out().Printf("renew_after\t%s", formatUnixMillis(displayState.RenewAfterMs)) } - if state.UpdatedAtMs > 0 { - u.Out().Printf("updated_at\t%s", formatUnixMillis(state.UpdatedAtMs)) + if displayState.UpdatedAtMs > 0 { + u.Out().Printf("updated_at\t%s", formatUnixMillis(displayState.UpdatedAtMs)) } - if state.Hook != nil { - u.Out().Printf("hook_url\t%s", state.Hook.URL) - if state.Hook.IncludeBody { + if displayState.Hook != nil { + u.Out().Printf("hook_url\t%s", displayState.Hook.URL) + if displayState.Hook.IncludeBody { u.Out().Printf("hook_include_body\ttrue") } - if state.Hook.MaxBytes > 0 { - u.Out().Printf("hook_max_bytes\t%d", state.Hook.MaxBytes) + if displayState.Hook.MaxBytes > 0 { + u.Out().Printf("hook_max_bytes\t%d", displayState.Hook.MaxBytes) } - if state.Hook.Token != "" { - u.Out().Printf("hook_token\t%s", state.Hook.Token) + if displayState.Hook.Token != "" { + u.Out().Printf("hook_token\t%s", displayState.Hook.Token) } } - if state.LastDeliveryStatus != "" { - u.Out().Printf("last_delivery_status\t%s", state.LastDeliveryStatus) + if displayState.LastDeliveryStatus != "" { + u.Out().Printf("last_delivery_status\t%s", displayState.LastDeliveryStatus) } - if state.LastDeliveryAtMs > 0 { - u.Out().Printf("last_delivery_at\t%s", formatUnixMillis(state.LastDeliveryAtMs)) + if displayState.LastDeliveryAtMs > 0 { + u.Out().Printf("last_delivery_at\t%s", formatUnixMillis(displayState.LastDeliveryAtMs)) } - if state.LastDeliveryStatusNote != "" { - u.Out().Printf("last_delivery_note\t%s", state.LastDeliveryStatusNote) + if displayState.LastDeliveryStatusNote != "" { + u.Out().Printf("last_delivery_note\t%s", displayState.LastDeliveryStatusNote) } - if state.LastPushMessageID != "" { - u.Out().Printf("last_push_message_id\t%s", state.LastPushMessageID) + if displayState.LastPushMessageID != "" { + u.Out().Printf("last_push_message_id\t%s", displayState.LastPushMessageID) } return nil } +func redactWatchStateSecrets(state gmailWatchState) gmailWatchState { + if state.Hook == nil || state.Hook.Token == "" { + return state + } + redacted := state + hook := *state.Hook + if len(hook.Token) > 4 { + hook.Token = hook.Token[:4] + "...(" + strconv.Itoa(len(hook.Token)) + " chars)" + } else { + hook.Token = "[REDACTED]" + } + redacted.Hook = &hook + return redacted +} + func buildWatchState(account, topic string, labels []string, resp *gmail.WatchResponse, ttl time.Duration, hook *gmailWatchHook) (gmailWatchState, error) { if resp == nil { return gmailWatchState{}, errors.New("watch response missing") diff --git a/internal/cmd/gmail_watch_cmds_errors_test.go b/internal/cmd/gmail_watch_cmds_errors_test.go index 1082464..b39577c 100644 --- a/internal/cmd/gmail_watch_cmds_errors_test.go +++ b/internal/cmd/gmail_watch_cmds_errors_test.go @@ -484,7 +484,7 @@ func TestWriteWatchState_LastPushMessageID(t *testing.T) { LastDeliveryStatusNote: "note", LastPushMessageID: "msg1", } - if err := writeWatchState(ctx, state); err != nil { + if err := writeWatchState(ctx, state, false); err != nil { t.Fatalf("writeWatchState: %v", err) } if !strings.Contains(out.String(), "last_push_message_id") { diff --git a/internal/cmd/gmail_watch_helpers_test.go b/internal/cmd/gmail_watch_helpers_test.go index 84a32b5..fa02217 100644 --- a/internal/cmd/gmail_watch_helpers_test.go +++ b/internal/cmd/gmail_watch_helpers_test.go @@ -40,7 +40,7 @@ func TestWriteWatchState_TextAndJSON(t *testing.T) { t.Fatalf("ui.New: %v", uiErr) } ctx := ui.WithUI(context.Background(), u) - if err := writeWatchState(ctx, state); err != nil { + if err := writeWatchState(ctx, state, false); err != nil { t.Fatalf("writeWatchState: %v", err) } }) @@ -58,7 +58,7 @@ func TestWriteWatchState_TextAndJSON(t *testing.T) { } ctx := ui.WithUI(context.Background(), u) ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true}) - if err := writeWatchState(ctx, state); err != nil { + if err := writeWatchState(ctx, state, false); err != nil { t.Fatalf("writeWatchState json: %v", err) } }) diff --git a/internal/cmd/gmail_watch_redact_test.go b/internal/cmd/gmail_watch_redact_test.go new file mode 100644 index 0000000..8936e5b --- /dev/null +++ b/internal/cmd/gmail_watch_redact_test.go @@ -0,0 +1,123 @@ +package cmd + +import ( + "context" + "encoding/json" + "io" + "os" + "strings" + "testing" + + "github.com/steipete/gogcli/internal/outfmt" + "github.com/steipete/gogcli/internal/ui" +) + +func TestWriteWatchState_TokenRedaction(t *testing.T) { + makeState := func(token string) gmailWatchState { + return gmailWatchState{ + Account: "a@b.com", + Topic: "projects/p/topics/t", + HistoryID: "1", + Hook: &gmailWatchHook{ + URL: "http://example.com/hook", + Token: token, + }, + } + } + + run := func(t *testing.T, state gmailWatchState, showSecrets bool) string { + t.Helper() + return captureStdout(t, func() { + u, err := ui.New(ui.Options{Stdout: os.Stdout, Stderr: io.Discard, Color: "never"}) + if err != nil { + t.Fatalf("ui.New: %v", err) + } + ctx := ui.WithUI(context.Background(), u) + if err := writeWatchState(ctx, state, showSecrets); err != nil { + t.Fatalf("writeWatchState: %v", err) + } + }) + } + + t.Run("long token is redacted by default", func(t *testing.T) { + out := run(t, makeState("supersecrettoken123"), false) + if strings.Contains(out, "supersecrettoken123") { + t.Fatal("token should be redacted but was shown in full") + } + if !strings.Contains(out, "supe...(19 chars)") { + t.Fatalf("expected masked token, got: %s", out) + } + }) + + t.Run("short token is fully redacted", func(t *testing.T) { + out := run(t, makeState("ab"), false) + if strings.Contains(out, "hook_token\tab") { + t.Fatal("short token should be fully redacted") + } + if !strings.Contains(out, "[REDACTED]") { + t.Fatalf("expected [REDACTED], got: %s", out) + } + }) + + t.Run("4-char token is fully redacted", func(t *testing.T) { + out := run(t, makeState("abcd"), false) + if !strings.Contains(out, "[REDACTED]") { + t.Fatalf("expected [REDACTED] for 4-char token, got: %s", out) + } + }) + + t.Run("show-secrets reveals full token", func(t *testing.T) { + out := run(t, makeState("supersecrettoken123"), true) + if !strings.Contains(out, "hook_token\tsupersecrettoken123") { + t.Fatalf("expected full token with --show-secrets, got: %s", out) + } + }) + + t.Run("empty token not shown", func(t *testing.T) { + out := run(t, makeState(""), false) + if strings.Contains(out, "hook_token") { + t.Fatal("empty token should not appear in output") + } + }) + + t.Run("json output redacts token by default", func(t *testing.T) { + out := captureStdout(t, func() { + u, err := ui.New(ui.Options{Stdout: os.Stdout, Stderr: io.Discard, Color: "never"}) + if err != nil { + t.Fatalf("ui.New: %v", err) + } + ctx := ui.WithUI(context.Background(), u) + ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true}) + if err := writeWatchState(ctx, makeState("supersecrettoken123"), false); err != nil { + t.Fatalf("writeWatchState json: %v", err) + } + }) + if strings.Contains(out, "supersecrettoken123") { + t.Fatal("JSON output should not contain plaintext token") + } + var parsed map[string]json.RawMessage + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("json parse: %v", err) + } + if !strings.Contains(out, `"[REDACTED]"`) { + t.Fatalf("expected [REDACTED] in JSON, got: %s", out) + } + }) + + t.Run("json output shows token with show-secrets", func(t *testing.T) { + out := captureStdout(t, func() { + u, err := ui.New(ui.Options{Stdout: os.Stdout, Stderr: io.Discard, Color: "never"}) + if err != nil { + t.Fatalf("ui.New: %v", err) + } + ctx := ui.WithUI(context.Background(), u) + ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true}) + if err := writeWatchState(ctx, makeState("supersecrettoken123"), true); err != nil { + t.Fatalf("writeWatchState json: %v", err) + } + }) + if !strings.Contains(out, "supersecrettoken123") { + t.Fatalf("JSON with --show-secrets should contain token, got: %s", out) + } + }) +}