fix: reject truncated and non-success API responses

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Client: reject responses larger than 1 MiB without decoding truncated data, and report unhandled HTTP redirects as API errors instead of successful results.
 - Homebrew: install through a Formula (`brew install openclaw/tap/goplaces`) instead of the retired Cask; quarantine is preserved.
 
 ## 0.4.11 - 2026-09-13

docs/client-reference.md

@@ -16,3 +16,5 @@ These request details are easy to miss when moving between the CLI and library:
 Field masks are defined with each request implementation so calls ask Google only for the fields represented by that workflow.
 
 Redirects may stay within the original origin (scheme, hostname, and port); redirects to another origin are rejected to keep the API key scoped to the configured endpoint. Configure an endpoint override directly when using a different host. A custom `HTTPClient.CheckRedirect` can still stop redirects. API keys echoed in upstream or transport diagnostics are redacted; error causes remain available through `errors.Is` and `errors.As`.
+
+Only final 2xx responses are decoded as results. Other HTTP statuses return an `APIError`, including redirects stopped with `http.ErrUseLastResponse` or returned without a `Location` header. Responses are limited to 1 MiB; larger bodies return an explicit size error instead of decoding a truncated payload. Oversized HTTP error responses retain their status in `APIError` and replace the body with a size diagnostic. The CLI exits with code 1 for these response failures.

internal/places/client.go

@@ -16,6 +16,11 @@ import (
 // DefaultBaseURL is the default endpoint for the Places API (New).
 const DefaultBaseURL = "https://places.googleapis.com/v1"
 
+const (
+	maxResponseBytes        = 1 << 20
+	responseTooLargeMessage = "response exceeds 1 MiB limit"
+)
+
 // Client wraps access to the Google Places API.
 type Client struct {
 	apiKey            string
@@ -111,16 +116,24 @@ func (c *Client) doRequest(
 		_ = response.Body.Close()
 	}()
 
-	// Hard-cap payload size to avoid runaway error bodies.
-	payload, err := io.ReadAll(io.LimitReader(response.Body, 1<<20))
+	// Read one extra byte to distinguish an exact-size response from truncation.
+	payload, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBytes+1))
 	if err != nil {
 		return nil, c.requestError("read response", err)
 	}
 
-	if response.StatusCode >= http.StatusBadRequest {
-		apiErr := &APIError{StatusCode: response.StatusCode, Body: c.redactAPIKey(strings.TrimSpace(string(payload)))}
+	tooLarge := len(payload) > maxResponseBytes
+	if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices {
+		body := responseTooLargeMessage
+		if !tooLarge {
+			body = c.redactAPIKey(strings.TrimSpace(string(payload)))
+		}
+		apiErr := &APIError{StatusCode: response.StatusCode, Body: body}
 		return nil, apiErr
 	}
+	if tooLarge {
+		return nil, errors.New("goplaces: " + responseTooLargeMessage)
+	}
 
 	if len(payload) == 0 {
 		return nil, errors.New("goplaces: empty response")

internal/places/client_response_test.go

@@ -0,0 +1,93 @@
+package places
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestClientResponseSizeBoundary(t *testing.T) {
+	const limit = 1 << 20
+	for _, tt := range []struct {
+		name    string
+		size    int
+		wantErr bool
+	}{
+		{"below limit", limit - 1, false},
+		{"at limit", limit, false},
+		{"over limit", limit + 1, true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			prefix := `{"id":"fixture"}`
+			body := prefix + strings.Repeat(" ", tt.size-len(prefix))
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				// Flush headers so the check also covers unknown content lengths.
+				w.(http.Flusher).Flush()
+				_, _ = fmt.Fprint(w, body)
+			}))
+			defer server.Close()
+			client := NewClient(Options{APIKey: "test-key", BaseURL: server.URL})
+			place, err := client.Details(t.Context(), "fixture")
+			if tt.wantErr {
+				if err == nil || !strings.Contains(err.Error(), "response exceeds") || place.PlaceID != "" {
+					t.Fatalf("expected size error without a result, got place=%+v err=%v", place, err)
+				}
+				return
+			}
+			if err != nil || place.PlaceID != "fixture" {
+				t.Fatalf("place=%+v err=%v", place, err)
+			}
+		})
+	}
+}
+
+func TestClientRejectsTrailingDataBeyondResponseLimit(t *testing.T) {
+	body := `{"id":"fixture"}`
+	body += strings.Repeat(" ", (1<<20)-len(body)) + `{"id":"trailing"}`
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = fmt.Fprint(w, body)
+	}))
+	defer server.Close()
+	client := NewClient(Options{APIKey: "test-key", BaseURL: server.URL})
+	if place, err := client.Details(t.Context(), "fixture"); err == nil {
+		t.Fatalf("accepted a truncated response: %+v", place)
+	}
+}
+
+func TestClientRejectsNonSuccessStatus(t *testing.T) {
+	for _, statusCode := range []int{http.StatusMultipleChoices, http.StatusFound, http.StatusNotModified} {
+		t.Run(http.StatusText(statusCode), func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				w.WriteHeader(statusCode)
+				_, _ = fmt.Fprint(w, `{"id":"unexpected"}`)
+			}))
+			defer server.Close()
+			client := NewClient(Options{APIKey: "test-key", BaseURL: server.URL})
+			place, err := client.Details(t.Context(), "fixture")
+			var apiErr *APIError
+			if !errors.As(err, &apiErr) || apiErr.StatusCode != statusCode || place.PlaceID != "" {
+				t.Fatalf("expected APIError for %d without a result, got place=%+v err=%v", statusCode, place, err)
+			}
+		})
+	}
+}
+
+func TestClientOversizedAPIError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusBadGateway)
+		_, _ = fmt.Fprint(w, strings.Repeat("x", 1<<20)+securityTestKey)
+	}))
+	defer server.Close()
+	client := NewClient(Options{APIKey: securityTestKey, BaseURL: server.URL})
+	_, err := client.Details(t.Context(), "fixture")
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusBadGateway {
+		t.Fatalf("expected APIError with upstream status, got %v", err)
+	}
+	if !strings.Contains(apiErr.Body, "response exceeds") || len(apiErr.Body) > 100 {
+		t.Fatal("expected a concise size diagnostic instead of a truncated upstream body")
+	}
+}

internal/places/client_security_test.go

@@ -76,7 +76,8 @@ func TestClientPreservesCustomRedirectPolicy(t *testing.T) {
 	client := NewClient(Options{APIKey: securityTestKey, BaseURL: server.URL, HTTPClient: httpClient})
 	for range 2 {
 		place, err := client.Details(t.Context(), "fixture")
-		if err != nil || place.PlaceID != "stopped" {
+		var apiErr *APIError
+		if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusFound || place.PlaceID != "" {
 			t.Fatalf("place=%+v err=%v", place, err)
 		}
 	}