diff --git a/CHANGELOG.md b/CHANGELOG.md index ddef043ab..99e3d10fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Security: redact credentials embedded in Gmail watch hook URLs, including userinfo, path, query, and fragment components, unless `--show-secrets` is set. (#960) — thanks @bunlongheng. - Gmail: add guarded single-message RFC822/EML import from a file or stdin, with labels, internal-date, spam, calendar-processing, and parse-only dry-run controls. (#956) — thanks @holgergruenhagen. - Gmail: warn before a draft update replaces an existing rich-text body with plain text only, while keeping JSON stdout clean. (#955) — thanks @mcinteerj. - Dependencies: update the Google API and OpenTelemetry stacks, Go developer tools, pnpm, and email-tracking worker toolchain to their latest policy-eligible releases. diff --git a/internal/cmd/gmail_watch_cmds.go b/internal/cmd/gmail_watch_cmds.go index f420031a6..5e256bcb7 100644 --- a/internal/cmd/gmail_watch_cmds.go +++ b/internal/cmd/gmail_watch_cmds.go @@ -5,6 +5,7 @@ import ( "errors" "net" "net/http" + neturl "net/url" "strconv" "strings" "time" @@ -436,10 +437,13 @@ func (c *GmailWatchServeCmd) Run(ctx context.Context, kctx *kong.Context, flags func writeWatchState(ctx context.Context, state gmailWatchState, showSecrets bool) error { if outfmt.IsJSON(ctx) { - if !showSecrets && state.Hook != nil && state.Hook.Token != "" { + if !showSecrets && state.Hook != nil { redacted := state h := *state.Hook - h.Token = "[REDACTED]" + if h.Token != "" { + h.Token = "[REDACTED]" + } + h.URL = redactHookURL(h.URL) redacted.Hook = &h return outfmt.WriteJSON(ctx, stdoutWriter(ctx), map[string]any{"watch": redacted}) } @@ -465,7 +469,11 @@ func writeWatchState(ctx context.Context, state gmailWatchState, showSecrets boo u.Out().Linef("updated_at\t%s", formatUnixMillis(state.UpdatedAtMs)) } if state.Hook != nil { - u.Out().Linef("hook_url\t%s", state.Hook.URL) + hookURL := state.Hook.URL + if !showSecrets { + hookURL = redactHookURL(hookURL) + } + u.Out().Linef("hook_url\t%s", hookURL) if state.Hook.IncludeBody { u.Out().Linef("hook_include_body\ttrue") } @@ -510,6 +518,24 @@ func writeWatchState(ctx context.Context, state gmailWatchState, showSecrets boo return nil } +// redactHookURL keeps only the origin of a webhook URL. Webhook providers place +// credentials in userinfo, paths, queries, and fragments, so preserving any of +// those components would make the default status output unsafe to share. +func redactHookURL(raw string) string { + if strings.TrimSpace(raw) == "" { + return raw + } + parsed, err := neturl.Parse(raw) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return "[REDACTED]" + } + + if parsed.User == nil && parsed.Path == "" && parsed.RawQuery == "" && parsed.Fragment == "" { + return raw + } + return parsed.Scheme + "://" + parsed.Host + "/[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_helpers_test.go b/internal/cmd/gmail_watch_helpers_test.go index d85b6074b..a663ca29d 100644 --- a/internal/cmd/gmail_watch_helpers_test.go +++ b/internal/cmd/gmail_watch_helpers_test.go @@ -129,7 +129,7 @@ func TestWriteWatchState_TextAndJSON(t *testing.T) { if !strings.Contains(textOut, "account\ta@b.com") { t.Fatalf("expected account output") } - if !strings.Contains(textOut, "hook_url\thttp://example.com/hook") { + if !strings.Contains(textOut, "hook_url\thttp://example.com/[REDACTED]") { t.Fatalf("expected hook output") } @@ -150,8 +150,8 @@ func TestWriteWatchState_TextAndJSON(t *testing.T) { if err := json.Unmarshal([]byte(jsonOut), &parsed); err != nil { t.Fatalf("json parse: %v", err) } - if parsed.Watch.Hook == nil || parsed.Watch.Hook.URL == "" { - t.Fatalf("expected hook in json") + if parsed.Watch.Hook == nil || parsed.Watch.Hook.URL != "http://example.com/[REDACTED]" { + t.Fatalf("expected redacted hook in json") } } diff --git a/internal/cmd/gmail_watch_redact_test.go b/internal/cmd/gmail_watch_redact_test.go index 8936e5b2a..877b5daa3 100644 --- a/internal/cmd/gmail_watch_redact_test.go +++ b/internal/cmd/gmail_watch_redact_test.go @@ -121,3 +121,95 @@ func TestWriteWatchState_TokenRedaction(t *testing.T) { } }) } + +func TestWriteWatchState_HookURLCredentialRedaction(t *testing.T) { + password := "example-" + "password" + queryToken := "example-" + "query-token" + pathToken := "example-" + "path-token" + basicAuthURL := "https://alice:" + password + "@example.com/hook" + credentialURL := "https://alice:" + password + "@example.com/hooks/" + pathToken + "?token=" + queryToken + + makeState := func(hookURL string) gmailWatchState { + return gmailWatchState{ + Account: "a@b.com", + Topic: "projects/p/topics/t", + HistoryID: "1", + Hook: &gmailWatchHook{ + URL: hookURL, + }, + } + } + + run := func(t *testing.T, state gmailWatchState, showSecrets, jsonOut 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 jsonOut { + ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true}) + } + if err := writeWatchState(ctx, state, showSecrets); err != nil { + t.Fatalf("writeWatchState: %v", err) + } + }) + } + + t.Run("userinfo password redacted by default", func(t *testing.T) { + out := run(t, makeState(basicAuthURL), false, false) + if strings.Contains(out, password) { + t.Fatalf("basic-auth password leaked in hook URL: %s", out) + } + if !strings.Contains(out, "hook_url\thttps://example.com/[REDACTED]") { + t.Fatalf("expected recognizable redacted origin, got: %s", out) + } + }) + + t.Run("query token redacted by default", func(t *testing.T) { + out := run(t, makeState("https://example.com/hook?token="+queryToken), false, false) + if strings.Contains(out, queryToken) { + t.Fatalf("query token leaked in hook URL: %s", out) + } + }) + + t.Run("path credential redacted by default", func(t *testing.T) { + out := run(t, makeState("https://example.com/hooks/"+pathToken), false, false) + if strings.Contains(out, pathToken) || strings.Contains(out, "/hooks/") { + t.Fatalf("path credential leaked in hook URL: %s", out) + } + }) + + t.Run("origin-only url unchanged", func(t *testing.T) { + out := run(t, makeState("https://example.com"), false, false) + if !strings.Contains(out, "hook_url\thttps://example.com") { + t.Fatalf("origin-only hook URL should be shown unchanged, got: %s", out) + } + }) + + t.Run("malformed url fails closed", func(t *testing.T) { + out := run(t, makeState("opaque-secret-without-an-origin"), false, false) + if strings.Contains(out, "opaque-secret") || !strings.Contains(out, "hook_url\t[REDACTED]") { + t.Fatalf("malformed hook URL was not fully redacted: %s", out) + } + }) + + t.Run("show-secrets reveals full url", func(t *testing.T) { + out := run(t, makeState(credentialURL), true, false) + if !strings.Contains(out, password) || !strings.Contains(out, pathToken) || !strings.Contains(out, queryToken) { + t.Fatalf("--show-secrets should reveal full hook URL, got: %s", out) + } + }) + + t.Run("json output redacts url credentials by default", func(t *testing.T) { + out := run(t, makeState(credentialURL), false, true) + if strings.Contains(out, password) || strings.Contains(out, pathToken) || strings.Contains(out, queryToken) { + t.Fatalf("JSON output leaked hook URL credentials: %s", out) + } + var parsed map[string]json.RawMessage + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("json parse: %v", err) + } + }) +}