From fba766794905039ec4a3ea0334f892b4209062e5 Mon Sep 17 00:00:00 2001
From: dev_agent <dev_agent@cubecraftlabs.com>
Date: Sun, 4 Oct 2026 07:38:03 -0400
Subject: [PATCH] Keep only redacted text in typed errors, not the credential

ProviderError and RateLimitedError now build their redacted Error text
when constructed and do not store the secret, so formatting the value
with %#v or %+v cannot print it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/ai/llm/errors.go      | 44 ++++++++++++++++++++--------------
 internal/ai/llm/errors_test.go | 12 ++++++++++
 2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/internal/ai/llm/errors.go b/internal/ai/llm/errors.go
index 50183e6..07fddef 100644
--- a/internal/ai/llm/errors.go
+++ b/internal/ai/llm/errors.go
@@ -32,22 +32,30 @@ type RateLimitedError struct {
 	// RetryAfter is how long the vendor asked callers to wait; 0 means it
 	// did not say.
 	RetryAfter time.Duration
-	secret     string
+	// text is the redacted Error text set by NewRateLimitedError; "" means
+	// the error was built without a credential.
+	text string
 }
 
 // NewRateLimitedError returns the error for a throttled request. secret is
-// the credential sent with the request, or "" when the request sent none;
-// Error removes every occurrence of a non-empty secret, even where the
-// fixed text happens to spell it.
+// the credential sent with the request, or "" when the request sent none.
+// Every occurrence of a non-empty secret is removed from the error text
+// when it is built, even where the fixed text happens to spell it, and the
+// secret itself is not kept.
 func NewRateLimitedError(retryAfter time.Duration, secret string) *RateLimitedError {
-	return &RateLimitedError{RetryAfter: retryAfter, secret: secret}
+	e := &RateLimitedError{RetryAfter: retryAfter}
+	e.text = Redact(e.Error(), secret)
+	return e
 }
 
 func (e *RateLimitedError) Error() string {
-	if e.RetryAfter == 0 {
-		return Redact(ErrRateLimited.Error(), e.secret)
+	if e.text != "" {
+		return e.text
 	}
-	return Redact(fmt.Sprintf("%s: retry after %v", ErrRateLimited, e.RetryAfter), e.secret)
+	if e.RetryAfter == 0 {
+		return ErrRateLimited.Error()
+	}
+	return fmt.Sprintf("%s: retry after %v", ErrRateLimited, e.RetryAfter)
 }
 
 // Is reports whether target is ErrRateLimited.
@@ -84,7 +92,7 @@ func Redact(s, secret string) string {
 type ProviderError struct {
 	status int
 	body   string
-	secret string
+	text   string
 }
 
 // NewProviderError returns the error for an unexpected HTTP status. It
@@ -92,8 +100,9 @@ type ProviderError struct {
 // credential sent with the request, or "" when the request sent none; every
 // occurrence of a non-empty secret in body is removed by Redact before the
 // body is cut, so no part of it survives, and again after the cut, so the
-// truncation mark cannot complete it. Error removes it from the complete
-// text too, even where the fixed text happens to spell it.
+// truncation mark cannot complete it. It is removed from the complete error
+// text too, even where the fixed text happens to spell it, and the secret
+// itself is not kept.
 func NewProviderError(status int, body []byte, secret string) *ProviderError {
 	s := strings.TrimSpace(Redact(string(body), secret))
 	if len(s) > MaxBodyExcerpt {
@@ -103,7 +112,11 @@ func NewProviderError(status int, body []byte, secret string) *ProviderError {
 		}
 		s = Redact(s[:cut]+"…", secret)
 	}
-	return &ProviderError{status: status, body: s, secret: secret}
+	text := fmt.Sprintf("%s: HTTP %d", ErrProvider, status)
+	if s != "" {
+		text += ": " + s
+	}
+	return &ProviderError{status: status, body: s, text: Redact(text, secret)}
 }
 
 // Status returns the HTTP status code.
@@ -113,12 +126,7 @@ func (e *ProviderError) Status() int { return e.status }
 // when the vendor sent no body.
 func (e *ProviderError) Body() string { return e.body }
 
-func (e *ProviderError) Error() string {
-	if e.body == "" {
-		return Redact(fmt.Sprintf("%s: HTTP %d", ErrProvider, e.status), e.secret)
-	}
-	return Redact(fmt.Sprintf("%s: HTTP %d: %s", ErrProvider, e.status, e.body), e.secret)
-}
+func (e *ProviderError) Error() string { return e.text }
 
 // Is reports whether target is ErrProvider.
 func (e *ProviderError) Is(target error) bool { return target == ErrProvider }
diff --git a/internal/ai/llm/errors_test.go b/internal/ai/llm/errors_test.go
index b5784ad..5ac5fa7 100644
--- a/internal/ai/llm/errors_test.go
+++ b/internal/ai/llm/errors_test.go
@@ -184,3 +184,15 @@ func TestRateLimitedErrorWithFixedWordSecretNeverLeaksSecret(t *testing.T) {
 		}
 	}
 }
+
+func TestTypedErrorsDoNotKeepSecret(t *testing.T) {
+	const secret = "sk-live-0123456789abcdef"
+	for _, err := range []error{
+		NewProviderError(500, []byte("down "+secret), secret),
+		NewRateLimitedError(30*time.Second, secret),
+	} {
+		if got := fmt.Sprintf("%#v %+v", err, err); strings.Contains(got, secret) {
+			t.Fatalf("formatting %T leaks the secret: %q", err, got)
+		}
+	}
+}