From 1f87a77c8879b116374ffa434f591ebfe67c4472 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 11 Mar 2026 22:52:53 +0800 Subject: [PATCH 1/3] fix(main): address medium-severity security findings - Bound HTTP response reads to 1 MB via io.LimitReader to prevent unbounded memory allocation - Replace configInitialized bool with sync.Once to eliminate potential data race - Warn when client secret is passed via CLI flag as it may be visible in process listings Co-Authored-By: Claude Opus 4.6 --- main.go | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/main.go b/main.go index f8c3595..9aa68a4 100644 --- a/main.go +++ b/main.go @@ -14,6 +14,7 @@ import ( "os/signal" "strconv" "strings" + "sync" "syscall" "time" @@ -28,18 +29,18 @@ import ( ) var ( - serverURL string - clientID string - clientSecret string - redirectURI string - callbackPort int - scope string - tokenFile string - tokenStoreMode string - tokenStore credstore.Store[credstore.Token] - configInitialized bool - retryClient *retry.Client - configWarnings []string + serverURL string + clientID string + clientSecret string + redirectURI string + callbackPort int + scope string + tokenFile string + tokenStoreMode string + tokenStore credstore.Store[credstore.Token] + configOnce sync.Once + retryClient *retry.Client + configWarnings []string flagServerURL *string flagClientID *string @@ -55,6 +56,7 @@ const ( tokenExchangeTimeout = 10 * time.Second tokenVerificationTimeout = 10 * time.Second refreshTokenTimeout = 10 * time.Second + maxResponseSize = 1 << 20 // 1 MB ) func init() { @@ -96,16 +98,21 @@ func init() { // initConfig parses flags and initializes all configuration. func initConfig() { - if configInitialized { - return - } - configInitialized = true + configOnce.Do(doInitConfig) +} +func doInitConfig() { flag.Parse() serverURL = getConfig(*flagServerURL, "SERVER_URL", "http://localhost:8080") clientID = getConfig(*flagClientID, "CLIENT_ID", "") clientSecret = getConfig(*flagClientSecret, "CLIENT_SECRET", "") + if *flagClientSecret != "" { + configWarnings = append(configWarnings, + "Client secret passed via command-line flag. "+ + "This may be visible in process listings. "+ + "Consider using CLIENT_SECRET env var or .env file instead.") + } scope = getConfig(*flagScope, "SCOPE", "read write") tokenFile = getConfig(*flagTokenFile, "TOKEN_FILE", ".authgate-tokens.json") @@ -341,7 +348,7 @@ func exchangeCode(ctx context.Context, code, codeVerifier string) (*tui.TokenSto } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) if err != nil { return nil, fmt.Errorf("failed to read response: %w", err) } @@ -405,7 +412,7 @@ func refreshAccessToken(ctx context.Context, refreshToken string) (*tui.TokenSto } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) if err != nil { return nil, fmt.Errorf("failed to read response: %w", err) } @@ -472,7 +479,7 @@ func verifyToken(ctx context.Context, accessToken string) (string, error) { } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) if err != nil { return "", fmt.Errorf("failed to read response: %w", err) } @@ -531,7 +538,7 @@ func makeAPICallWithAutoRefresh(ctx context.Context, storage *tui.TokenStorage) defer resp.Body.Close() } - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) if err != nil { return fmt.Errorf("failed to read response: %w", err) } From 55ac4f0b33c820f2e15bd13afa2684eb78a30859 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 11 Mar 2026 22:59:54 +0800 Subject: [PATCH 2/3] ci(workflows): remove non-existent make generate step The lint job referenced `make generate` which has no corresponding Makefile target, causing CI to fail. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/testing.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 1908fbd..1c7bbaf 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -32,10 +32,6 @@ jobs: with: go-version: ${{ matrix.go }} - - name: Run Generate - run: | - make generate - - name: Setup golangci-lint uses: golangci/golangci-lint-action@v9 with: From cc5f47a5f8943f4d81ac56c222a481b601fb5b6e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 11 Mar 2026 23:04:44 +0800 Subject: [PATCH 3/3] fix(main): return explicit error on oversized responses Address Copilot review feedback: - Fix comment from "1 MB" to "1 MiB" (1<<20 is a mebibyte) - Read maxResponseSize+1 bytes and return a clear "response too large" error instead of silently truncating (which caused confusing JSON parse errors) - Extract readResponseBody helper to deduplicate the pattern - Add tests for readResponseBody (within limit, at limit, exceeds limit) Co-Authored-By: Claude Opus 4.6 --- main.go | 29 ++++++++++++++++++++++++----- main_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index 9aa68a4..3585854 100644 --- a/main.go +++ b/main.go @@ -56,7 +56,7 @@ const ( tokenExchangeTimeout = 10 * time.Second tokenVerificationTimeout = 10 * time.Second refreshTokenTimeout = 10 * time.Second - maxResponseSize = 1 << 20 // 1 MB + maxResponseSize = 1 << 20 // 1 MiB ) func init() { @@ -269,6 +269,25 @@ type tokenResponse struct { Scope string `json:"scope"` } +// errResponseTooLarge is returned when a server response exceeds maxResponseSize. +var errResponseTooLarge = fmt.Errorf( + "response body exceeds maximum allowed size of %d bytes", + maxResponseSize, +) + +// readResponseBody reads up to maxResponseSize bytes from r and returns an +// explicit error when the response is too large (rather than silently truncating). +func readResponseBody(r io.Reader) ([]byte, error) { + body, err := io.ReadAll(io.LimitReader(r, maxResponseSize+1)) + if err != nil { + return nil, err + } + if int64(len(body)) > maxResponseSize { + return nil, errResponseTooLarge + } + return body, nil +} + // parseOAuthError attempts to extract a structured OAuth error from a non-200 // response body. Falls back to including the raw body in the error message. func parseOAuthError(statusCode int, body []byte, action string) error { @@ -348,7 +367,7 @@ func exchangeCode(ctx context.Context, code, codeVerifier string) (*tui.TokenSto } defer resp.Body.Close() - body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) + body, err := readResponseBody(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response: %w", err) } @@ -412,7 +431,7 @@ func refreshAccessToken(ctx context.Context, refreshToken string) (*tui.TokenSto } defer resp.Body.Close() - body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) + body, err := readResponseBody(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response: %w", err) } @@ -479,7 +498,7 @@ func verifyToken(ctx context.Context, accessToken string) (string, error) { } defer resp.Body.Close() - body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) + body, err := readResponseBody(resp.Body) if err != nil { return "", fmt.Errorf("failed to read response: %w", err) } @@ -538,7 +557,7 @@ func makeAPICallWithAutoRefresh(ctx context.Context, storage *tui.TokenStorage) defer resp.Body.Close() } - body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseSize)) + body, err := readResponseBody(resp.Body) if err != nil { return fmt.Errorf("failed to read response: %w", err) } diff --git a/main_test.go b/main_test.go index dc799af..5479c3d 100644 --- a/main_test.go +++ b/main_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "errors" "path/filepath" "testing" "time" @@ -276,6 +278,38 @@ func TestInitTokenStore_Invalid(t *testing.T) { } } +func TestReadResponseBody(t *testing.T) { + t.Run("within limit", func(t *testing.T) { + data := bytes.Repeat([]byte("a"), 100) + body, err := readResponseBody(bytes.NewReader(data)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(body) != 100 { + t.Errorf("expected 100 bytes, got %d", len(body)) + } + }) + + t.Run("exactly at limit", func(t *testing.T) { + data := bytes.Repeat([]byte("a"), maxResponseSize) + body, err := readResponseBody(bytes.NewReader(data)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(body) != maxResponseSize { + t.Errorf("expected %d bytes, got %d", maxResponseSize, len(body)) + } + }) + + t.Run("exceeds limit", func(t *testing.T) { + data := bytes.Repeat([]byte("a"), maxResponseSize+1) + _, err := readResponseBody(bytes.NewReader(data)) + if !errors.Is(err, errResponseTooLarge) { + t.Errorf("expected errResponseTooLarge, got: %v", err) + } + }) +} + // containsSubstring is a helper to avoid importing strings in tests. func containsSubstring(s, sub string) bool { return len(s) >= len(sub) && findSubstring(s, sub)