Never quote request or reply text in errors and redact the key as a backstop

Request.Validate no longer quotes the role or schema name, and the bad-output error no longer quotes finish_reason: any of that text may equal the API key. New, Generate, ListModels and Verify now pass every error through redact, which removes the key (and so "Bearer "+key) from the error text, wrapped errors included, while Unwrap keeps the chain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 07:24:37 -04:00
parent f333440256
commit 2afd6a9338
3 changed files with 162 additions and 8 deletions
@@ -67,14 +67,15 @@ type Request struct {
 }

 // Validate reports whether r is well formed. Adapters call it before
-// contacting the vendor.
+// contacting the vendor. Its errors never quote the request's text, such as
+// a role or schema name: any of it may equal the API key.
 func (r Request) Validate() error {
 	if len(r.Messages) == 0 {
 		return errors.New("llm: request has no messages")
 	}
 	for i, m := range r.Messages {
 		if m.Role != RoleUser && m.Role != RoleAssistant {
-			return fmt.Errorf("llm: message %d has unknown role %q", i, m.Role)
+			return fmt.Errorf("llm: message %d has an unknown role", i)
 		}
 	}
 	if r.Temperature != nil && *r.Temperature < 0 {
@@ -92,7 +93,7 @@ func (r Request) Validate() error {
 		}
 		var obj map[string]json.RawMessage
 		if err := json.Unmarshal(r.Schema.Schema, &obj); err != nil || obj == nil {
-			return fmt.Errorf("llm: schema %q is not a JSON object", r.Schema.Name)
+			return errors.New("llm: schema is not a JSON object")
 		}
 	}
 	return nil
@@ -3,7 +3,9 @@
 // POST {base}/chat/completions and GET {base}/models over plain net/http.
 //
 // The adapter never logs, and no error it returns contains the API key or
-// the Authorization header value.
+// the Authorization header value: errors do not quote caller- or
+// vendor-supplied text, and every exported method passes its error through
+// redact as a backstop.
 package openai

 import (
@@ -76,6 +78,14 @@ var _ llm.Provider = (*Provider)(nil)

 // New validates cfg and returns a Provider.
 func New(cfg Config) (*Provider, error) {
+	p, err := newFromConfig(cfg)
+	if err != nil {
+		return nil, redact(err, cfg.APIKey)
+	}
+	return p, nil
+}
+
+func newFromConfig(cfg Config) (*Provider, error) {
 	// Base URL errors never echo the URL: a rejected one may carry a
 	// credential.
 	u, err := url.Parse(cfg.BaseURL)
@@ -219,6 +229,11 @@ const schemaInstruction = "Reply with only a JSON object, and no other text, tha

 // Generate sends req to {base}/chat/completions.
 func (p *Provider) Generate(ctx context.Context, req llm.Request) (llm.Response, error) {
+	resp, err := p.generate(ctx, req)
+	return resp, redact(err, p.apiKey)
+}
+
+func (p *Provider) generate(ctx context.Context, req llm.Request) (llm.Response, error) {
 	if err := req.Validate(); err != nil {
 		return llm.Response{}, fmt.Errorf("openai: generate: %w", err)
 	}
@@ -298,7 +313,7 @@ func (p *Provider) Generate(ctx context.Context, req llm.Request) (llm.Response,
 		resp.JSON = json.RawMessage(text)
 	}
 	if resp.JSON == nil {
-		return resp, fmt.Errorf("openai: generate: reply (finish reason %q) holds no JSON object: %w", resp.FinishReason, llm.ErrBadOutput)
+		return resp, fmt.Errorf("openai: generate: reply holds no JSON object: %w", llm.ErrBadOutput)
 	}
 	return resp, nil
 }
@@ -343,6 +358,11 @@ type modelsResponse struct {

 // ListModels returns the models {base}/models lists.
 func (p *Provider) ListModels(ctx context.Context) ([]llm.Model, error) {
+	models, err := p.listModels(ctx)
+	return models, redact(err, p.apiKey)
+}
+
+func (p *Provider) listModels(ctx context.Context) ([]llm.Model, error) {
 	raw, _, err := p.do(ctx, http.MethodGet, "/models", nil)
 	if err != nil {
 		return nil, fmt.Errorf("openai: list models: %w", err)
@@ -370,8 +390,8 @@ func (p *Provider) ListModels(ctx context.Context) ([]llm.Model, error) {

 // Verify checks the credentials with an authenticated ListModels call.
 func (p *Provider) Verify(ctx context.Context) error {
-	_, err := p.ListModels(ctx)
-	return err
+	_, err := p.listModels(ctx)
+	return redact(err, p.apiKey)
 }

 // do sends one request and returns the body and status of a 2xx response.
@@ -493,10 +513,44 @@ func (p *Provider) transportError(err error) error {
 	return &transportFailure{detail: redactKey(err.Error(), p.apiKey), kind: kind}
 }

-// redactKey replaces every occurrence of a non-empty key in s.
+// redactKey replaces every occurrence of a non-empty key in s. Replacing
+// the key also replaces the "Bearer "+key header value that contains it.
 func redactKey(s, key string) string {
 	if key == "" {
 		return s
 	}
 	return strings.ReplaceAll(s, key, "[REDACTED]")
 }
+
+// withheldText replaces an error's text when redactKey cannot remove the
+// key, because the key is part of the replacement marker.
+const withheldText = "openai: error text withheld: it contains the API key"
+
+// redactedError is an error whose text had the API key removed. It unwraps
+// to the original error, so errors.Is and errors.As still see the chain.
+type redactedError struct {
+	text string
+	err  error
+}
+
+func (e *redactedError) Error() string { return e.text }
+
+func (e *redactedError) Unwrap() error { return e.err }
+
+// redact is the backstop every exported method applies to its error: when
+// err's text, wrapped errors included, holds a non-empty key, it returns err
+// with that text redacted. Any other err is returned unchanged.
+func redact(err error, key string) error {
+	if err == nil || key == "" {
+		return err
+	}
+	text := err.Error()
+	if !strings.Contains(text, key) {
+		return err
+	}
+	text = redactKey(text, key)
+	if strings.Contains(text, key) {
+		text = withheldText
+	}
+	return &redactedError{text: text, err: err}
+}
@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -785,3 +786,101 @@ func TestRetryAfter(t *testing.T) {
 		}
 	}
 }
+
+// headerNameKey is an API key that is also a word callers and vendors put in
+// text: an error that quotes such text prints the key.
+const headerNameKey = "Authorization"
+
+// assertNoKey fails if err's text holds key or "Bearer "+key in any case.
+func assertNoKey(t *testing.T, err error, key string) {
+	t.Helper()
+	for _, secret := range []string{key, "Bearer " + key} {
+		if strings.Contains(err.Error(), secret) || strings.Contains(strings.ToLower(err.Error()), strings.ToLower(secret)) {
+			t.Fatalf("error leaks %q: %v", secret, err)
+		}
+	}
+}
+
+// keyServer replies to every request with status and body.
+func keyServer(t *testing.T, status int, body string) *Provider {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(status)
+		io.WriteString(w, body)
+	}))
+	t.Cleanup(srv.Close)
+	p, err := New(Config{BaseURL: srv.URL + "/v1", APIKey: headerNameKey, Model: "test-model", StructuredOutput: PromptOnly})
+	if err != nil {
+		t.Fatal(err)
+	}
+	return p
+}
+
+func TestGenerateInvalidRoleNeverLeaksKey(t *testing.T) {
+	p := keyServer(t, 200, `{}`)
+	_, err := p.Generate(context.Background(), llm.Request{Messages: []llm.Message{{Role: llm.Role(headerNameKey), Content: "hi"}}})
+	if err == nil {
+		t.Fatal("Generate accepted an unknown role")
+	}
+	assertNoKey(t, err, headerNameKey)
+}
+
+func TestGenerateInvalidSchemaNeverLeaksKey(t *testing.T) {
+	p := keyServer(t, 200, `{}`)
+	req := schemaRequest()
+	req.Schema = &llm.JSONSchema{Name: headerNameKey, Schema: json.RawMessage(`[]`)}
+	_, err := p.Generate(context.Background(), req)
+	if err == nil {
+		t.Fatal("Generate accepted a schema that is not a JSON object")
+	}
+	assertNoKey(t, err, headerNameKey)
+}
+
+func TestGenerateBadOutputNeverLeaksKey(t *testing.T) {
+	p := keyServer(t, 200, `{"model":"m","choices":[{"message":{"content":"no JSON here"},"finish_reason":"`+headerNameKey+`"}]}`)
+	resp, err := p.Generate(context.Background(), schemaRequest())
+	if !errors.Is(err, llm.ErrBadOutput) {
+		t.Fatalf("err = %v, want ErrBadOutput", err)
+	}
+	assertNoKey(t, err, headerNameKey)
+	if resp.FinishReason != headerNameKey {
+		t.Fatalf("FinishReason = %q, want it kept on the response", resp.FinishReason)
+	}
+}
+
+func TestProviderErrorBodyNeverLeaksKey(t *testing.T) {
+	p := keyServer(t, 500, `{"error":{"message":"rejected `+headerNameKey+`: Bearer `+headerNameKey+`"}}`)
+	_, genErr := p.Generate(context.Background(), schemaRequest())
+	_, listErr := p.ListModels(context.Background())
+	verifyErr := p.Verify(context.Background())
+	for _, err := range []error{genErr, listErr, verifyErr} {
+		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)
+		}
+		assertNoKey(t, err, headerNameKey)
+	}
+}
+
+// TestRedactKeepsChainAndHidesKey checks the backstop every exported method
+// applies: the key and its Bearer value leave the text, the chain stays.
+func TestRedactKeepsChainAndHidesKey(t *testing.T) {
+	inner := fmt.Errorf("decode: body %q: %w", "Bearer "+testKey, llm.ErrBadOutput)
+	err := redact(inner, testKey)
+	assertNoSecret(t, err)
+	if !errors.Is(err, llm.ErrBadOutput) || errors.Unwrap(err) != inner {
+		t.Fatalf("redact broke the chain of %v", err)
+	}
+	if redact(inner, "") != inner {
+		t.Fatal("redact changed an error for the empty key")
+	}
+	clean := errors.New("openai: generate: no model configured")
+	if redact(clean, testKey) != clean {
+		t.Fatal("redact wrapped an error that holds no key")
+	}
+	// A key inside the redaction marker would survive replacement.
+	if got := redact(errors.New("x E y"), "E").Error(); strings.Contains(got, "E") {
+		t.Fatalf("redact kept the key: %q", got)
+	}
+}