From f33344025614542b780ed0f97ea8c2f705787710 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 3 Oct 2026 18:07:31 -0400 Subject: [PATCH] Never quote caller-supplied text in New errors A managed-header error named the canonical header, which equals the API key when the key is itself a managed header name such as Authorization. The unknown structured-output error quoted the mode after redacting the whole key, which still leaked any fragment of the key. Both now return fixed messages, so no New error interpolates caller input. Co-Authored-By: Claude Opus 5.5 --- internal/ai/llm/openai/openai.go | 13 +++++------ internal/ai/llm/openai/openai_test.go | 32 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/internal/ai/llm/openai/openai.go b/internal/ai/llm/openai/openai.go index cdd4265..44828d7 100644 --- a/internal/ai/llm/openai/openai.go +++ b/internal/ai/llm/openai/openai.go @@ -90,25 +90,22 @@ func New(cfg Config) (*Provider, error) { if u.User != nil || strings.ContainsAny(cfg.BaseURL, "?#") { return nil, errors.New("openai: base URL must not carry credentials, a query or a fragment") } - // An error that names caller-supplied text has the API key redacted from - // it before it is quoted. + // No error from New quotes caller-supplied text or a name derived from + // it: any such text may equal or contain the API key, and redaction + // cannot catch a fragment or a case-changed copy of it. switch cfg.StructuredOutput { case JSONSchema, JSONObject, PromptOnly: default: - return nil, fmt.Errorf("openai: unknown structured output mode %q", redactKey(string(cfg.StructuredOutput), cfg.APIKey)) + return nil, errors.New("openai: unknown structured output mode") } headers := make(http.Header, len(cfg.Headers)+1) for name, value := range cfg.Headers { - // Caller-supplied names and values are never echoed: either may carry - // a credential, and canonicalising a name changes its case, which - // defeats redaction. Only a managed name, which is a fixed known - // string, is named. if !validHeaderName(name) { return nil, errors.New("openai: an extra header name is not a valid HTTP token") } canonical := textproto.CanonicalMIMEHeaderKey(name) if managedHeaders[canonical] { - return nil, fmt.Errorf("openai: extra header %q is set by the adapter", canonical) + return nil, errors.New("openai: an extra header is set by the adapter") } if !validHeaderValue(value) { return nil, errors.New("openai: an extra header value has a control character") diff --git a/internal/ai/llm/openai/openai_test.go b/internal/ai/llm/openai/openai_test.go index 6feff02..745da45 100644 --- a/internal/ai/llm/openai/openai_test.go +++ b/internal/ai/llm/openai/openai_test.go @@ -652,6 +652,38 @@ func TestNewConfigErrorsNeverLeakKey(t *testing.T) { } } +// TestNewManagedHeaderErrorNeverLeaksKey uses an API key that is itself a +// managed header name, so naming the rejected header would print the key. +func TestNewManagedHeaderErrorNeverLeaksKey(t *testing.T) { + for _, key := range []string{"Authorization", "Content-Type", "Host"} { + _, err := New(Config{ + BaseURL: "https://example.com/v1", + APIKey: key, + StructuredOutput: JSONSchema, + Headers: map[string]string{key: "x"}, + }) + if err == nil { + t.Errorf("key %q: New() = nil error", key) + continue + } + for _, secret := range []string{key, "Bearer " + key} { + if strings.Contains(strings.ToLower(err.Error()), strings.ToLower(secret)) { + t.Errorf("key %q: error leaks %q: %v", key, secret, err) + } + } + } +} + +// TestNewModeErrorNeverLeaksKeyFragment uses a structured output mode that is +// a fragment of the API key, which redacting the whole key would not catch. +func TestNewModeErrorNeverLeaksKeyFragment(t *testing.T) { + _, err := New(Config{BaseURL: "https://example.com/v1", APIKey: testKey, StructuredOutput: StructuredOutput(testKey[:20])}) + if err == nil { + t.Fatal("New() = nil error") + } + assertNoSecret(t, err) +} + func TestAcceptHeader(t *testing.T) { for name, tc := range map[string]struct { headers map[string]string