Never quote caller-supplied text in New errors
CI / ci (push) Successful in 1m29s
Independent review Codex gpt-6-sol high: CHANGES_REQUIRED (1 high)
CI / ci (pull_request) Successful in 1m30s

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 <noreply@anthropic.com>
This commit is contained in:
2026-10-03 18:07:31 -04:00
co-authored by Claude Opus 5.5
parent 095c03f112
commit f333440256
2 changed files with 37 additions and 8 deletions
+5 -8
View File
@@ -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")
+32
View File
@@ -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