Reject API keys that are not valid header values at construction

FromSettings now fails with llm.ErrInvalidKey for a key with a control
character such as CR or LF, for every preset that sends a key, and
openai.New rejects such a key too, so neither defers the failure to the
first request. anthropic.New already rejected it. No error quotes the key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/ai/llm/openai/openai.go

@@ -53,6 +53,7 @@ type Config struct {
 	BaseURL string
 	// APIKey is sent as "Authorization: Bearer <APIKey>". Empty sends no
 	// Authorization header, for endpoints such as Ollama that need none.
+	// It must not have a control character other than tab.
 	APIKey string
 	// Model is the model Generate requests. ListModels and Verify do not
 	// need it.
@@ -102,6 +103,9 @@ func newFromConfig(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")
 	}
+	if !validHeaderValue(cfg.APIKey) {
+		return nil, errors.New("openai: API key has a control character")
+	}
 	// 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.

internal/ai/llm/openai/openai_test.go

@@ -628,6 +628,20 @@ func TestNewRejectsBadHeaderWithoutEchoingValue(t *testing.T) {
 	assertNoSecret(t, err)
 }
 
+func TestNewRejectsKeyWithControlCharacter(t *testing.T) {
+	for _, key := range []string{testKey + "\n", testKey + "\r\nX-Injected: 1", "\x00" + testKey, testKey + "\x7f"} {
+		_, err := New(Config{BaseURL: "https://example.com/v1", APIKey: key, StructuredOutput: JSONSchema})
+		if err == nil {
+			t.Errorf("New(%q) = nil error", key)
+			continue
+		}
+		assertNoSecret(t, err)
+		if strings.Contains(err.Error()+fmt.Sprintf("%#v", err), key) {
+			t.Errorf("error leaks the key: %v", err)
+		}
+	}
+}
+
 // TestNewConfigErrorsNeverLeakKey covers every New error that names
 // caller-supplied text, each with the API key embedded in that text.
 func TestNewConfigErrorsNeverLeakKey(t *testing.T) {

internal/ai/llm/presets/presets.go

@@ -6,12 +6,14 @@
 // FromSettings lives here rather than in package llm because it builds the
 // adapters, and every adapter imports llm.
 //
-// Nothing in this package logs, and no error it returns contains the API
-// key or quotes a setting's value.
+// Nothing in this package logs, and no error it returns or unwraps to
+// contains the API key, even where the key equals a setting's or request's
+// value or text the error spells anyway.
 package presets
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math"
 	"regexp"
@@ -46,14 +48,16 @@ const (
 	KeyNone
 )
 
-// TemperatureRejectingModels is the one list of model-id patterns that
-// reject the temperature field: OpenAI's o1 and o3 reasoning models. A
-// preset whose OmitTemperature holds it never sends a temperature to a
-// matching model.
-var TemperatureRejectingModels = []*regexp.Regexp{
-	regexp.MustCompile(`^o1`),
-	regexp.MustCompile(`^o3`),
-}
+// The reasoning models that reject the temperature field are OpenAI's o1
+// and o3 families: the bare family name or the name followed by a "-"
+// variant (o1, o1-mini, o3-2025-04-16), never a longer word such as o123.
+// OpenRouter names them with an "openai/" prefix and may add a ":" variant
+// (openai/o1:online). A preset whose OmitTemperature holds a pattern never
+// sends a temperature to a matching model.
+var (
+	openAIReasoningModels     = regexp.MustCompile(`^o[13](-.*)?$`)
+	openRouterReasoningModels = regexp.MustCompile(`^openai/o[13]([-:].*)?$`)
+)
 
 // Preset is one vendor's fixed configuration.
 type Preset struct {
@@ -127,7 +131,7 @@ var all = map[string]Preset{
 		Key:               KeyRequired,
 		StructuredOutput:  openai.JSONSchema,
 		StructuredOutputs: []openai.StructuredOutput{openai.JSONSchema},
-		OmitTemperature:   TemperatureRejectingModels,
+		OmitTemperature:   []*regexp.Regexp{openAIReasoningModels},
 	},
 	"openrouter": {
 		Name:                     "openrouter",
@@ -139,6 +143,7 @@ var all = map[string]Preset{
 		StructuredOutput:         openai.JSONSchema,
 		FallbackStructuredOutput: openai.JSONObject,
 		StructuredOutputs:        []openai.StructuredOutput{openai.JSONSchema, openai.JSONObject},
+		OmitTemperature:          []*regexp.Regexp{openRouterReasoningModels},
 	},
 	"ollama": {
 		Name:    "ollama",
@@ -151,19 +156,29 @@ var all = map[string]Preset{
 		Key:               KeyOptional,
 		StructuredOutput:  openai.JSONSchema,
 		StructuredOutputs: []openai.StructuredOutput{openai.JSONSchema, openai.JSONObject, openai.PromptOnly},
-		OmitTemperature:   TemperatureRejectingModels,
+		// The server may be OpenAI itself or a gateway that names models
+		// the OpenRouter way.
+		OmitTemperature: []*regexp.Regexp{openAIReasoningModels, openRouterReasoningModels},
 	},
 }
 
 // FromSettings returns the Provider s describes, using key as its API key.
 // It fails with a configuration error from package llm — ErrUnknownProvider,
-// ErrMissingKey, ErrUnexpectedKey, ErrMissingBaseURL, ErrMissingModel,
+// ErrMissingKey, ErrUnexpectedKey, ErrInvalidKey, ErrMissingBaseURL, ErrMissingModel,
 // ErrMissingPublicURL or ErrInvalidSetting — before contacting any vendor.
 //
 // The Provider sends s.Temperature with every request whose Temperature is
 // nil; a request that sets one keeps it. Neither is sent to a model the
 // preset's OmitTemperature matches.
 func FromSettings(s llm.Settings, key string) (llm.Provider, error) {
+	cp, err := fromSettings(s, key)
+	if err != nil {
+		return nil, redact(err, key)
+	}
+	return cp, nil
+}
+
+func fromSettings(s llm.Settings, key string) (*configured, error) {
 	p, ok := Lookup(s.Provider)
 	if !ok {
 		return nil, llm.ErrUnknownProvider
@@ -200,7 +215,7 @@ func FromSettings(s llm.Settings, key string) (llm.Provider, error) {
 		return nil, fmt.Errorf("%s: %w: %w", p.Name, llm.ErrInvalidSetting, err)
 	}
 
-	cp := &configured{Provider: inner}
+	cp := &configured{Provider: inner, key: key}
 	if !p.omitsTemperature(s.Model) {
 		t := s.Temperature
 		cp.temperature = &t
@@ -215,6 +230,8 @@ func check(p Preset, s llm.Settings, key string) error {
 		return llm.ErrMissingKey
 	case p.Key == KeyNone && key != "":
 		return llm.ErrUnexpectedKey
+	case !validHeaderValue(key):
+		return llm.ErrInvalidKey
 	}
 	switch {
 	case p.BaseURL == "" && s.BaseURL == "":
@@ -237,16 +254,36 @@ func check(p Preset, s llm.Settings, key string) error {
 	return nil
 }
 
+// validHeaderValue reports whether value has no control character other
+// than tab, so it can be sent as an HTTP header value.
+func validHeaderValue(value string) bool {
+	for i := 0; i < len(value); i++ {
+		if c := value[i]; (c < 0x20 && c != '\t') || c == 0x7f {
+			return false
+		}
+	}
+	return true
+}
+
 // configured applies the household temperature and the preset's
 // temperature quirk to every Generate.
 type configured struct {
 	llm.Provider
+	// key is the API key, redacted from validation errors; the adapter
+	// redacts its own.
+	key string
 	// temperature is sent when a request sets none; nil means the model
 	// rejects temperature, so none is ever sent.
 	temperature *float64
 }
 
+// Generate validates req as sent by the caller before changing its
+// temperature, so clearing it for a model that rejects temperature never
+// hides an invalid one.
 func (c *configured) Generate(ctx context.Context, req llm.Request) (llm.Response, error) {
+	if err := req.Validate(); err != nil {
+		return llm.Response{}, redact(fmt.Errorf("presets: generate: %w", err), c.key)
+	}
 	if c.temperature == nil {
 		req.Temperature = nil
 	} else if req.Temperature == nil {
@@ -255,3 +292,28 @@ func (c *configured) Generate(ctx context.Context, req llm.Request) (llm.Respons
 	}
 	return c.Provider.Generate(ctx, req)
 }
+
+// redactedError is an error with the API key removed from its text. It
+// does not unwrap, because every layer below may hold the key; Is still
+// matches the sentinels the original error wraps.
+type redactedError struct {
+	text string
+	err  error
+}
+
+func (e *redactedError) Error() string { return e.text }
+
+// GoString keeps %#v to the redacted Error text instead of the fields.
+func (e *redactedError) GoString() string { return e.text }
+
+// Is reports whether the original error matches target.
+func (e *redactedError) Is(target error) bool { return errors.Is(e.err, target) }
+
+// redact returns err with key removed from its text; a nil err or an empty
+// key returns err unchanged.
+func redact(err error, key string) error {
+	if err == nil || key == "" {
+		return err
+	}
+	return &redactedError{text: llm.Redact(err.Error(), key), err: err}
+}

internal/ai/llm/presets/presets_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"strings"
@@ -223,11 +224,19 @@ func TestOpenAICompatiblePreset(t *testing.T) {
 }
 
 func TestTemperatureOmittedForRejectingModels(t *testing.T) {
-	for _, provider := range []string{"openai", "openai_compatible"} {
-		for _, model := range []string{"o1", "o1-mini", "o3-mini"} {
-			s := llm.Settings{Provider: provider, BaseURL: "http://llm.lan/v1", Model: model, Temperature: 0.8}
-			if provider == "openai" {
-				s.BaseURL = ""
+	cases := map[string][]string{
+		"openai":            {"o1", "o1-mini", "o1-2024-12-17", "o3", "o3-mini"},
+		"openai_compatible": {"o1", "o1-mini", "o3-mini", "openai/o1"},
+		"openrouter":        {"openai/o1", "openai/o1-mini", "openai/o3", "openai/o3-mini", "openai/o1:online"},
+	}
+	for provider, models := range cases {
+		for _, model := range models {
+			s := llm.Settings{Provider: provider, Model: model, Temperature: 0.8}
+			switch provider {
+			case "openai_compatible":
+				s.BaseURL = "http://llm.lan/v1"
+			case "openrouter":
+				s.PublicURL = "https://mealquest.example"
 			}
 			for _, req := range []llm.Request{plainRequest(), func() llm.Request { r := plainRequest(); r.Temperature = ptr(0.2); return r }()} {
 				if got, ok := temperature(t, generate(t, s, testKey, req)); ok {
@@ -239,10 +248,19 @@ func TestTemperatureOmittedForRejectingModels(t *testing.T) {
 }
 
 func TestTemperatureSentForOtherModels(t *testing.T) {
-	for _, model := range []string{"gpt-4o", "gpt-4o-mini", "chatgpt-o1-like"} {
-		c := generate(t, llm.Settings{Provider: "openai", Model: model, Temperature: 0.8}, testKey, plainRequest())
-		if got, ok := temperature(t, c); !ok || got != 0.8 {
-			t.Errorf("%s: temperature = %v, %v; want 0.8", model, got, ok)
+	cases := map[string][]string{
+		"openai":     {"gpt-4o", "gpt-4o-mini", "chatgpt-o1-like", "o123", "o3custom", "o1mini", "openai/o1"},
+		"openrouter": {"openai/gpt-4o", "openai/o123", "openai/o3custom", "acme/o1", "o1-mini"},
+	}
+	for provider, models := range cases {
+		for _, model := range models {
+			s := llm.Settings{Provider: provider, Model: model, Temperature: 0.8}
+			if provider == "openrouter" {
+				s.PublicURL = "https://mealquest.example"
+			}
+			if got, ok := temperature(t, generate(t, s, testKey, plainRequest())); !ok || got != 0.8 {
+				t.Errorf("%s %s: temperature = %v, %v; want 0.8", provider, model, got, ok)
+			}
 		}
 	}
 	// A request's own temperature wins over the household's.
@@ -254,8 +272,35 @@ func TestTemperatureSentForOtherModels(t *testing.T) {
 	}
 }
 
+// A request is validated before the temperature quirk clears its
+// temperature, so a negative one is an error and nothing is sent.
+func TestNegativeTemperatureRejectedForRejectingModels(t *testing.T) {
+	rt := &recordingTransport{}
+	orig := http.DefaultTransport
+	http.DefaultTransport = rt
+	t.Cleanup(func() { http.DefaultTransport = orig })
+
+	for _, s := range []llm.Settings{
+		{Provider: "openai", Model: "o1"},
+		{Provider: "openrouter", Model: "openai/o1", PublicURL: "https://mealquest.example"},
+	} {
+		p, err := FromSettings(s, testKey)
+		if err != nil {
+			t.Fatalf("%s: FromSettings: %v", s.Model, err)
+		}
+		req := plainRequest()
+		req.Temperature = ptr(-0.5)
+		if _, err := p.Generate(context.Background(), req); err == nil || errors.Is(err, errStop) || !strings.Contains(err.Error(), "temperature") {
+			t.Errorf("%s: Generate = %v, want a temperature validation error", s.Model, err)
+		}
+	}
+	if len(rt.reqs) != 0 {
+		t.Fatalf("sent %d requests, want 0", len(rt.reqs))
+	}
+}
+
 func TestTemperatureQuirkPresets(t *testing.T) {
-	want := map[string]bool{"openai": true, "openai_compatible": true}
+	want := map[string]bool{"openai": true, "openrouter": true, "openai_compatible": true}
 	for name, p := range all {
 		if got := len(p.OmitTemperature) > 0; got != want[name] {
 			t.Errorf("%s: lists temperature quirk = %v, want %v", name, got, want[name])
@@ -292,7 +337,7 @@ func TestPresetTable(t *testing.T) {
 
 func TestConfigErrors(t *testing.T) {
 	sentinels := []error{
-		llm.ErrUnknownProvider, llm.ErrMissingKey, llm.ErrUnexpectedKey, llm.ErrMissingBaseURL,
+		llm.ErrUnknownProvider, llm.ErrMissingKey, llm.ErrUnexpectedKey, llm.ErrInvalidKey, llm.ErrMissingBaseURL,
 		llm.ErrMissingModel, llm.ErrMissingPublicURL, llm.ErrInvalidSetting,
 	}
 	cases := []struct {
@@ -306,6 +351,10 @@ func TestConfigErrors(t *testing.T) {
 		{"openai no key", llm.Settings{Provider: "openai", Model: "m"}, "", llm.ErrMissingKey},
 		{"openrouter no key", llm.Settings{Provider: "openrouter", Model: "m", PublicURL: "https://x.example"}, "", llm.ErrMissingKey},
 		{"ollama with key", llm.Settings{Provider: "ollama", BaseURL: "http://o:11434", Model: "m"}, testKey, llm.ErrUnexpectedKey},
+		{"anthropic key with newline", llm.Settings{Provider: "anthropic", Model: "m"}, testKey + "\n", llm.ErrInvalidKey},
+		{"openai key with newline", llm.Settings{Provider: "openai", Model: "m"}, testKey + "\n", llm.ErrInvalidKey},
+		{"openrouter key with CR LF", llm.Settings{Provider: "openrouter", Model: "m", PublicURL: "https://x.example"}, testKey + "\r\nX-Injected: 1", llm.ErrInvalidKey},
+		{"openai_compatible key with NUL", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m"}, "\x00" + testKey, llm.ErrInvalidKey},
 		{"ollama no base URL", llm.Settings{Provider: "ollama", Model: "m"}, "", llm.ErrMissingBaseURL},
 		{"openai_compatible no base URL", llm.Settings{Provider: "openai_compatible", Model: "m"}, "", llm.ErrMissingBaseURL},
 		{"anthropic no model", llm.Settings{Provider: "anthropic"}, testKey, llm.ErrMissingModel},
@@ -355,3 +404,69 @@ func TestReadyProviderForEveryPreset(t *testing.T) {
 		}
 	}
 }
+
+// A key that matches a value the request carries, or text the error spells
+// anyway, never appears in a validation error, and nothing is sent.
+func TestGenerateValidationErrorsNeverLeakKey(t *testing.T) {
+	rt := &recordingTransport{}
+	orig := http.DefaultTransport
+	http.DefaultTransport = rt
+	t.Cleanup(func() { http.DefaultTransport = orig })
+
+	for _, key := range []string{"-0.5", "temperature", "llm", "negative"} {
+		p, err := FromSettings(llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m"}, key)
+		if err != nil {
+			t.Fatalf("%s: FromSettings: %v", key, err)
+		}
+		req := plainRequest()
+		req.Temperature = ptr(-0.5)
+		_, err = p.Generate(context.Background(), req)
+		if err == nil || errors.Is(err, errStop) {
+			t.Fatalf("%s: Generate = %v, want a validation error", key, err)
+		}
+		for e := error(err); e != nil; e = errors.Unwrap(e) {
+			if s := e.Error() + fmt.Sprintf("%#v", e); strings.Contains(s, key) {
+				t.Fatalf("%s: error leaks the key: %s", key, s)
+			}
+		}
+	}
+	if len(rt.reqs) != 0 {
+		t.Fatalf("sent %d requests, want 0", len(rt.reqs))
+	}
+}
+
+// A configuration error keeps its sentinel but never holds the key, even
+// when the key is also a setting's value or text the error spells anyway.
+func TestConfigErrorsNeverLeakKey(t *testing.T) {
+	cases := []struct {
+		name string
+		s    llm.Settings
+		key  string
+		want error
+	}{
+		{"unknown provider named by key", llm.Settings{Provider: "llm", Model: "m"}, "llm", llm.ErrUnknownProvider},
+		{"no base URL", llm.Settings{Provider: "openai_compatible", Model: "m"}, "base URL", llm.ErrMissingBaseURL},
+		{"no model", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1"}, "model", llm.ErrMissingModel},
+		{"key as model, no public URL", llm.Settings{Provider: "openrouter", Model: "missing"}, "missing", llm.ErrMissingPublicURL},
+		{"key as base URL", llm.Settings{Provider: "openai", BaseURL: "llm", Model: "m"}, "llm", llm.ErrInvalidSetting},
+		{"key as mode", llm.Settings{Provider: "openai_compatible", BaseURL: "http://l/v1", Model: "m", StructuredOutput: "xml"}, "xml", llm.ErrInvalidSetting},
+		{"key as temperature", llm.Settings{Provider: "openai", Model: "m", Temperature: -0.1}, "-0.1", llm.ErrInvalidSetting},
+		{"key in bad base URL", llm.Settings{Provider: "openai_compatible", BaseURL: "ftp://k3y", Model: "m"}, "k3y", llm.ErrInvalidSetting},
+		{"ollama with key", llm.Settings{Provider: "ollama", BaseURL: "http://o:11434", Model: "m"}, "key", llm.ErrUnexpectedKey},
+		{"key with newline", llm.Settings{Provider: "openai", Model: "m"}, "sk-test\n", llm.ErrInvalidKey},
+		{"key with newline as text", llm.Settings{Provider: "openai", Model: "m"}, "header\n", llm.ErrInvalidKey},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			p, err := FromSettings(c.s, c.key)
+			if p != nil || !errors.Is(err, c.want) {
+				t.Fatalf("FromSettings = %v, %v; want %v", p, err, c.want)
+			}
+			for e := error(err); e != nil; e = errors.Unwrap(e) {
+				if s := e.Error() + fmt.Sprintf("%#v", e); strings.Contains(s, c.key) {
+					t.Fatalf("error leaks the key: %s", s)
+				}
+			}
+		})
+	}
+}

internal/ai/llm/settings.go

@@ -50,6 +50,9 @@ var (
 	// ErrUnexpectedKey means an API key was given to a preset that sends
 	// none.
 	ErrUnexpectedKey = errors.New("llm: provider takes no API key")
+	// ErrInvalidKey means the API key is not a valid HTTP header value:
+	// it has a control character such as CR or LF.
+	ErrInvalidKey = errors.New("llm: API key is not a valid header value")
 	// ErrMissingBaseURL means the preset requires a base URL and none was
 	// given.
 	ErrMissingBaseURL = errors.New("llm: base URL is required")