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()