internal/ai/llm/errors.go

@@ -26,17 +26,28 @@ var (
 )
 
 // RateLimitedError is a throttled request. It matches ErrRateLimited.
+// Adapters build it with NewRateLimitedError so its text never holds their
+// credential.
 type RateLimitedError struct {
 	// RetryAfter is how long the vendor asked callers to wait; 0 means it
 	// did not say.
 	RetryAfter time.Duration
+	secret     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.
+func NewRateLimitedError(retryAfter time.Duration, secret string) *RateLimitedError {
+	return &RateLimitedError{RetryAfter: retryAfter, secret: secret}
 }
 
 func (e *RateLimitedError) Error() string {
 	if e.RetryAfter == 0 {
-		return ErrRateLimited.Error()
+		return Redact(ErrRateLimited.Error(), e.secret)
 	}
-	return fmt.Sprintf("%s: retry after %v", ErrRateLimited, e.RetryAfter)
+	return Redact(fmt.Sprintf("%s: retry after %v", ErrRateLimited, e.RetryAfter), e.secret)
 }
 
 // Is reports whether target is ErrRateLimited.
@@ -73,6 +84,7 @@ func Redact(s, secret string) string {
 type ProviderError struct {
 	status int
 	body   string
+	secret string
 }
 
 // NewProviderError returns the error for an unexpected HTTP status. It
@@ -80,7 +92,8 @@ 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.
+// truncation mark cannot complete it. Error removes it from the complete
+// text too, even where the fixed text happens to spell it.
 func NewProviderError(status int, body []byte, secret string) *ProviderError {
 	s := strings.TrimSpace(Redact(string(body), secret))
 	if len(s) > MaxBodyExcerpt {
@@ -90,7 +103,7 @@ func NewProviderError(status int, body []byte, secret string) *ProviderError {
 		}
 		s = Redact(s[:cut]+"…", secret)
 	}
-	return &ProviderError{status: status, body: s}
+	return &ProviderError{status: status, body: s, secret: secret}
 }
 
 // Status returns the HTTP status code.
@@ -102,9 +115,9 @@ func (e *ProviderError) Body() string { return e.body }
 
 func (e *ProviderError) Error() string {
 	if e.body == "" {
-		return fmt.Sprintf("%s: HTTP %d", ErrProvider, e.status)
+		return Redact(fmt.Sprintf("%s: HTTP %d", ErrProvider, e.status), e.secret)
 	}
-	return fmt.Sprintf("%s: HTTP %d: %s", ErrProvider, e.status, e.body)
+	return Redact(fmt.Sprintf("%s: HTTP %d: %s", ErrProvider, e.status, e.body), e.secret)
 }
 
 // Is reports whether target is ErrProvider.

internal/ai/llm/errors_test.go

@@ -159,3 +159,28 @@ func TestProviderErrorTruncationMarkCannotCompleteSecret(t *testing.T) {
 		t.Fatalf("secret leaked: %q", pe.Body())
 	}
 }
+
+func TestProviderErrorWithFixedWordSecretNeverLeaksSecret(t *testing.T) {
+	const secret = "HTTP"
+	pe := NewProviderError(500, []byte(`{"error":"HTTP down"}`), secret)
+	if strings.Contains(pe.Error(), secret) || strings.Contains(pe.Body(), secret) {
+		t.Fatalf("secret leaked: Error %q, Body %q", pe.Error(), pe.Body())
+	}
+	if got := NewProviderError(500, nil, secret).Error(); got != "llm: provider error: [REDACTED] 500" {
+		t.Fatalf("Error() without body = %q", got)
+	}
+}
+
+func TestRateLimitedErrorWithFixedWordSecretNeverLeaksSecret(t *testing.T) {
+	for _, secret := range []string{"rate", "retry after"} {
+		for _, wait := range []time.Duration{0, 30 * time.Second} {
+			rl := NewRateLimitedError(wait, secret)
+			if strings.Contains(rl.Error(), secret) {
+				t.Fatalf("secret %q leaked: %q", secret, rl.Error())
+			}
+			if rl.RetryAfter != wait || !errors.Is(rl, ErrRateLimited) {
+				t.Fatalf("NewRateLimitedError(%v) = %+v", wait, rl)
+			}
+		}
+	}
+}

internal/ai/llm/openai/openai.go

@@ -446,7 +446,7 @@ func (p *Provider) statusError(resp *http.Response, body []byte) error {
 	case http.StatusUnauthorized, http.StatusForbidden:
 		return fmt.Errorf("HTTP %d: %w", resp.StatusCode, llm.ErrAuth)
 	case http.StatusTooManyRequests:
-		return &llm.RateLimitedError{RetryAfter: retryAfter(resp.Header.Get("Retry-After"), time.Now())}
+		return llm.NewRateLimitedError(retryAfter(resp.Header.Get("Retry-After"), time.Now()), p.apiKey)
 	case http.StatusRequestTimeout:
 		return fmt.Errorf("HTTP %d: %w", resp.StatusCode, llm.ErrTimeout)
 	default:
@@ -518,8 +518,10 @@ func (p *Provider) transportError(err error) error {
 // API key. Its text is the layer's text with the key redacted, and Unwrap
 // returns the layer's causes passed through redact, so no error reached by
 // unwrapping has text holding the key. Is consults the original layer and
-// its chain, so sentinels still match. As does too, but refuses a match
-// whose text holds the key, so it never hands back an unredacted error.
+// its chain, so sentinels still match. As does too. The llm typed errors
+// remove the key from their own complete text, fixed words included, so
+// they always match; As refuses only a match from outside llm whose text
+// holds the key, so it never hands back an unredacted error.
 type redactedError struct {
 	text string
 	err  error

internal/ai/llm/openai/openai_test.go

@@ -1042,3 +1042,80 @@ func TestGenerateMalformedJSONWithShortKeyNeverLeaksKey(t *testing.T) {
 		}
 	}
 }
+
+// assertLayersNoExactKey fails if the text of err or of any error reached
+// from it by errors.Unwrap holds key, matched case-sensitively.
+func assertLayersNoExactKey(t *testing.T, err error, key string) {
+	t.Helper()
+	for layer := err; layer != nil; layer = errors.Unwrap(layer) {
+		if strings.Contains(layer.Error(), key) {
+			t.Fatalf("layer leaks %q: %q", key, layer.Error())
+		}
+	}
+}
+
+// TestProviderErrorWithFixedWordKeyKeepsTypedMatch uses the fixed word
+// "HTTP" of ProviderError's text as the API key: the text must drop it and
+// errors.As must still reach the ProviderError.
+func TestProviderErrorWithFixedWordKeyKeepsTypedMatch(t *testing.T) {
+	const key = "HTTP"
+	p := keyServerWithKey(t, key, 500, `{"error":"down"}`)
+	_, err := p.Generate(context.Background(), schemaRequest())
+	var pe *llm.ProviderError
+	if !errors.Is(err, llm.ErrProvider) || !errors.As(err, &pe) || pe.Status() != 500 {
+		t.Fatalf("err = %v, want a 500 ProviderError", err)
+	}
+	if strings.Contains(pe.Error(), key) || strings.Contains(pe.Body(), key) {
+		t.Fatalf("ProviderError leaks the key: Error %q, Body %q", pe.Error(), pe.Body())
+	}
+	assertLayersNoExactKey(t, err, key)
+	assertChainNoExactKey(t, err, key)
+}
+
+// TestRateLimitedErrorWithFixedWordKeyKeepsTypedMatch uses the fixed word
+// "rate" of RateLimitedError's text as the API key.
+func TestRateLimitedErrorWithFixedWordKeyKeepsTypedMatch(t *testing.T) {
+	const key = "rate"
+	srv := newVendorServer(t, reply{status: 429, header: map[string]string{"Retry-After": "17"}})
+	p, err := New(Config{BaseURL: srv.URL + "/v1", APIKey: key, Model: "test-model", StructuredOutput: PromptOnly})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = p.Generate(context.Background(), schemaRequest())
+	var rl *llm.RateLimitedError
+	if !errors.Is(err, llm.ErrRateLimited) || !errors.As(err, &rl) || rl.RetryAfter != 17*time.Second {
+		t.Fatalf("err = %v, want a RateLimitedError with RetryAfter 17s", err)
+	}
+	if strings.Contains(rl.Error(), key) {
+		t.Fatalf("RateLimitedError leaks the key: %q", rl.Error())
+	}
+	assertLayersNoExactKey(t, err, key)
+	assertChainNoExactKey(t, err, key)
+}
+
+// TestGenerateWithAdapterNameKeyNeverLeaksKey uses the fixed prefix
+// "openai" of the adapter's errors as the API key.
+func TestGenerateWithAdapterNameKeyNeverLeaksKey(t *testing.T) {
+	const key = "openai"
+	t.Run("401", func(t *testing.T) {
+		p := keyServerWithKey(t, key, 401, `{"error":{"message":"bad key"}}`)
+		_, err := p.Generate(context.Background(), schemaRequest())
+		if !errors.Is(err, llm.ErrAuth) {
+			t.Fatalf("err = %v, want ErrAuth", err)
+		}
+		assertLayersNoExactKey(t, err, key)
+		assertChainNoExactKey(t, err, key)
+	})
+	t.Run("no model", func(t *testing.T) {
+		p, err := New(Config{BaseURL: "http://127.0.0.1:1/v1", APIKey: key, StructuredOutput: PromptOnly})
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, err = p.Generate(context.Background(), schemaRequest())
+		if err == nil {
+			t.Fatal("Generate succeeded without a model")
+		}
+		assertLayersNoExactKey(t, err, key)
+		assertChainNoExactKey(t, err, key)
+	})
+}