From b6c463b50f2cf894d3ecd8f5e7b1f841c390149f Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 15 Aug 2026 13:54:29 -0700 Subject: [PATCH] fix(http): bound watch serve and OAuth callback read timeouts gmail watch serve and the local OAuth callback listener only set ReadHeaderTimeout. Copy the Drive changes serve pattern so both servers also set ReadTimeout, IdleTimeout, and MaxHeaderBytes. Signed-off-by: Sebastien Tardif --- CHANGELOG.md | 4 + internal/cmd/gmail_watch_cmds.go | 3 + internal/cmd/gmail_watch_serve_test.go | 43 ++++++++++ internal/cmd/gmail_watch_types.go | 1 + internal/googleauth/oauth_flow.go | 95 +++++++++++---------- internal/googleauth/oauth_flow_more_test.go | 19 +++++ 6 files changed, 122 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 740b9f388..25cbe6368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Security: bound `gmail watch serve` and the local OAuth callback HTTP servers with read, idle, and header-size limits so a slow or oversized request cannot stall the listener. + ## v0.37.0 - 2026-08-14 - Gmail: emit sanitized message headers and bodies once in `gmail get --json --sanitize-content`, while retaining the `message` envelope and `--results-only` unwrapping. (#986) — thanks @ronny-rentner. diff --git a/internal/cmd/gmail_watch_cmds.go b/internal/cmd/gmail_watch_cmds.go index 3bbf68d94..1727ff28f 100644 --- a/internal/cmd/gmail_watch_cmds.go +++ b/internal/cmd/gmail_watch_cmds.go @@ -430,7 +430,10 @@ func (c *GmailWatchServeCmd) Run(ctx context.Context, kctx *kong.Context, flags httpServer := &http.Server{ Addr: addr, Handler: server, + ReadTimeout: defaultGmailWatchReadTimeout, ReadHeaderTimeout: 5 * time.Second, + IdleTimeout: 30 * time.Second, + MaxHeaderBytes: 64 << 10, } return listenAndServe(httpServer) } diff --git a/internal/cmd/gmail_watch_serve_test.go b/internal/cmd/gmail_watch_serve_test.go index e372c6e52..731fca812 100644 --- a/internal/cmd/gmail_watch_serve_test.go +++ b/internal/cmd/gmail_watch_serve_test.go @@ -459,3 +459,46 @@ func TestGmailWatchServeCmd_PreservesClientOverrideForRequestContexts(t *testing t.Fatalf("newService: %v", callErr) } } + +func TestGmailWatchServeCmd_HTTPServerTimeouts(t *testing.T) { + origListen := listenAndServe + t.Cleanup(func() { listenAndServe = origListen }) + + setWatchTestConfigHome(t) + + store := newGmailWatchTestStore(t, "a@b.com") + updateErr := store.Update(func(s *gmailWatchState) error { + s.Account = "a@b.com" + return nil + }) + if updateErr != nil { + t.Fatalf("seed: %v", updateErr) + } + + flags := &RootFlags{Account: "a@b.com"} + var got *http.Server + listenAndServe = func(srv *http.Server) error { + got = srv + return nil + } + + ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), &gmail.Service{}) + if execErr := runKong(t, &GmailWatchServeCmd{}, []string{"--port", "9999", "--path", "/hook"}, ctx, flags); execErr != nil { + t.Fatalf("execute: %v", execErr) + } + if got == nil { + t.Fatal("expected server") + } + if got.ReadTimeout == 0 { + t.Fatal("ReadTimeout must be set") + } + if got.ReadHeaderTimeout == 0 { + t.Fatal("ReadHeaderTimeout must be set") + } + if got.IdleTimeout == 0 { + t.Fatal("IdleTimeout must be set") + } + if got.MaxHeaderBytes == 0 { + t.Fatal("MaxHeaderBytes must be set") + } +} diff --git a/internal/cmd/gmail_watch_types.go b/internal/cmd/gmail_watch_types.go index 3176f8e42..73214a412 100644 --- a/internal/cmd/gmail_watch_types.go +++ b/internal/cmd/gmail_watch_types.go @@ -16,6 +16,7 @@ const ( defaultHistoryFetchDelay = 3 * time.Second defaultPushBodyLimitBytes = 1024 * 1024 defaultHookRequestTimeoutSec = 10 + defaultGmailWatchReadTimeout = 10 * time.Second ) type gmailWatchServeConfig struct { diff --git a/internal/googleauth/oauth_flow.go b/internal/googleauth/oauth_flow.go index 986fc6e79..3333fb726 100644 --- a/internal/googleauth/oauth_flow.go +++ b/internal/googleauth/oauth_flow.go @@ -45,6 +45,8 @@ type ManualAuthURLResult struct { // visible before the local OAuth server shuts down. const postSuccessDisplaySeconds = 30 +const defaultOAuthCallbackReadTimeout = 10 * time.Second + // successTemplateData holds data passed to the success page template. type successTemplateData struct { Email string @@ -153,6 +155,16 @@ func manageCredentialsReader( } } +func newOAuthCallbackServer(handler http.Handler) *http.Server { + return &http.Server{ + Handler: handler, + ReadTimeout: defaultOAuthCallbackReadTimeout, + ReadHeaderTimeout: 5 * time.Second, + IdleTimeout: 30 * time.Second, + MaxHeaderBytes: 64 << 10, + } +} + func authorizeServer(ctx context.Context, opts AuthorizeOptions, creds config.ClientCredentials) (string, error) { state, err := randomStateFn() if err != nil { @@ -184,63 +196,60 @@ func authorizeServer(ctx context.Context, opts AuthorizeOptions, creds config.Cl codeCh := make(chan string, 1) errCh := make(chan error, 1) - srv := &http.Server{ - ReadHeaderTimeout: 5 * time.Second, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/oauth2/callback" { - http.NotFound(w, r) - return - } - q := r.URL.Query() - - w.Header().Set("Content-Type", "text/html; charset=utf-8") - - if q.Get("error") != "" { - select { - case errCh <- fmt.Errorf("%w: %s", errAuthorization, q.Get("error")): - default: - } + srv := newOAuthCallbackServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth2/callback" { + http.NotFound(w, r) + return + } + q := r.URL.Query() - w.WriteHeader(http.StatusOK) - renderCancelledPage(w) + w.Header().Set("Content-Type", "text/html; charset=utf-8") - return + if q.Get("error") != "" { + select { + case errCh <- fmt.Errorf("%w: %s", errAuthorization, q.Get("error")): + default: } - if q.Get("state") != state { - select { - case errCh <- errStateMismatch: - default: - } + w.WriteHeader(http.StatusOK) + renderCancelledPage(w) - w.WriteHeader(http.StatusBadRequest) - renderErrorPage(w, "State mismatch - possible CSRF attack. Please try again.") + return + } - return + if q.Get("state") != state { + select { + case errCh <- errStateMismatch: + default: } - code := q.Get("code") - if code == "" { - select { - case errCh <- errMissingCode: - default: - } - - w.WriteHeader(http.StatusBadRequest) - renderErrorPage(w, "Missing authorization code. Please try again.") + w.WriteHeader(http.StatusBadRequest) + renderErrorPage(w, "State mismatch - possible CSRF attack. Please try again.") - return - } + return + } + code := q.Get("code") + if code == "" { select { - case codeCh <- code: + case errCh <- errMissingCode: default: } - w.WriteHeader(http.StatusOK) - renderSuccessPage(w) - }), - } + w.WriteHeader(http.StatusBadRequest) + renderErrorPage(w, "Missing authorization code. Please try again.") + + return + } + + select { + case codeCh <- code: + default: + } + + w.WriteHeader(http.StatusOK) + renderSuccessPage(w) + })) go func() { <-ctx.Done() diff --git a/internal/googleauth/oauth_flow_more_test.go b/internal/googleauth/oauth_flow_more_test.go index 15e65aa04..a847ff908 100644 --- a/internal/googleauth/oauth_flow_more_test.go +++ b/internal/googleauth/oauth_flow_more_test.go @@ -3,6 +3,7 @@ package googleauth import ( "context" "net" + "net/http" "net/url" "strings" "testing" @@ -10,6 +11,24 @@ import ( "golang.org/x/oauth2" ) +func TestNewOAuthCallbackServer_ReadTimeout(t *testing.T) { + t.Parallel() + + srv := newOAuthCallbackServer(http.NotFoundHandler()) + if srv.ReadTimeout == 0 { + t.Fatal("ReadTimeout must be set") + } + if srv.ReadHeaderTimeout == 0 { + t.Fatal("ReadHeaderTimeout must be set") + } + if srv.IdleTimeout == 0 { + t.Fatal("IdleTimeout must be set") + } + if srv.MaxHeaderBytes == 0 { + t.Fatal("MaxHeaderBytes must be set") + } +} + func TestAuthURLParams(t *testing.T) { t.Parallel()