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) + } + } +}