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>
This commit is contained in:
@@ -53,6 +53,7 @@ type Config struct {
|
|||||||
BaseURL string
|
BaseURL string
|
||||||
// APIKey is sent as "Authorization: Bearer <APIKey>". Empty sends no
|
// APIKey is sent as "Authorization: Bearer <APIKey>". Empty sends no
|
||||||
// Authorization header, for endpoints such as Ollama that need none.
|
// Authorization header, for endpoints such as Ollama that need none.
|
||||||
|
// It must not have a control character other than tab.
|
||||||
APIKey string
|
APIKey string
|
||||||
// Model is the model Generate requests. ListModels and Verify do not
|
// Model is the model Generate requests. ListModels and Verify do not
|
||||||
// need it.
|
// need it.
|
||||||
@@ -102,6 +103,9 @@ func newFromConfig(cfg Config) (*Provider, error) {
|
|||||||
if u.User != nil || strings.ContainsAny(cfg.BaseURL, "?#") {
|
if u.User != nil || strings.ContainsAny(cfg.BaseURL, "?#") {
|
||||||
return nil, errors.New("openai: base URL must not carry credentials, a query or a fragment")
|
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
|
// 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
|
// it: any such text may equal or contain the API key, and redaction
|
||||||
// cannot catch a fragment or a case-changed copy of it.
|
// cannot catch a fragment or a case-changed copy of it.
|
||||||
|
|||||||
@@ -628,6 +628,20 @@ func TestNewRejectsBadHeaderWithoutEchoingValue(t *testing.T) {
|
|||||||
assertNoSecret(t, err)
|
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
|
// TestNewConfigErrorsNeverLeakKey covers every New error that names
|
||||||
// caller-supplied text, each with the API key embedded in that text.
|
// caller-supplied text, each with the API key embedded in that text.
|
||||||
func TestNewConfigErrorsNeverLeakKey(t *testing.T) {
|
func TestNewConfigErrorsNeverLeakKey(t *testing.T) {
|
||||||
|
|||||||
@@ -164,7 +164,7 @@ var all = map[string]Preset{
|
|||||||
|
|
||||||
// FromSettings returns the Provider s describes, using key as its API key.
|
// FromSettings returns the Provider s describes, using key as its API key.
|
||||||
// It fails with a configuration error from package llm — ErrUnknownProvider,
|
// 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.
|
// ErrMissingPublicURL or ErrInvalidSetting — before contacting any vendor.
|
||||||
//
|
//
|
||||||
// The Provider sends s.Temperature with every request whose Temperature is
|
// The Provider sends s.Temperature with every request whose Temperature is
|
||||||
@@ -230,6 +230,8 @@ func check(p Preset, s llm.Settings, key string) error {
|
|||||||
return llm.ErrMissingKey
|
return llm.ErrMissingKey
|
||||||
case p.Key == KeyNone && key != "":
|
case p.Key == KeyNone && key != "":
|
||||||
return llm.ErrUnexpectedKey
|
return llm.ErrUnexpectedKey
|
||||||
|
case !validHeaderValue(key):
|
||||||
|
return llm.ErrInvalidKey
|
||||||
}
|
}
|
||||||
switch {
|
switch {
|
||||||
case p.BaseURL == "" && s.BaseURL == "":
|
case p.BaseURL == "" && s.BaseURL == "":
|
||||||
@@ -252,6 +254,17 @@ func check(p Preset, s llm.Settings, key string) error {
|
|||||||
return nil
|
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
|
// configured applies the household temperature and the preset's
|
||||||
// temperature quirk to every Generate.
|
// temperature quirk to every Generate.
|
||||||
type configured struct {
|
type configured struct {
|
||||||
|
|||||||
@@ -337,7 +337,7 @@ func TestPresetTable(t *testing.T) {
|
|||||||
|
|
||||||
func TestConfigErrors(t *testing.T) {
|
func TestConfigErrors(t *testing.T) {
|
||||||
sentinels := []error{
|
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,
|
llm.ErrMissingModel, llm.ErrMissingPublicURL, llm.ErrInvalidSetting,
|
||||||
}
|
}
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
@@ -351,6 +351,10 @@ func TestConfigErrors(t *testing.T) {
|
|||||||
{"openai no key", llm.Settings{Provider: "openai", Model: "m"}, "", llm.ErrMissingKey},
|
{"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},
|
{"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},
|
{"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},
|
{"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},
|
{"openai_compatible no base URL", llm.Settings{Provider: "openai_compatible", Model: "m"}, "", llm.ErrMissingBaseURL},
|
||||||
{"anthropic no model", llm.Settings{Provider: "anthropic"}, testKey, llm.ErrMissingModel},
|
{"anthropic no model", llm.Settings{Provider: "anthropic"}, testKey, llm.ErrMissingModel},
|
||||||
@@ -449,6 +453,8 @@ func TestConfigErrorsNeverLeakKey(t *testing.T) {
|
|||||||
{"key as temperature", llm.Settings{Provider: "openai", Model: "m", Temperature: -0.1}, "-0.1", 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},
|
{"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},
|
{"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 {
|
for _, c := range cases {
|
||||||
t.Run(c.name, func(t *testing.T) {
|
t.Run(c.name, func(t *testing.T) {
|
||||||
|
|||||||
@@ -50,6 +50,9 @@ var (
|
|||||||
// ErrUnexpectedKey means an API key was given to a preset that sends
|
// ErrUnexpectedKey means an API key was given to a preset that sends
|
||||||
// none.
|
// none.
|
||||||
ErrUnexpectedKey = errors.New("llm: provider takes no API key")
|
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
|
// ErrMissingBaseURL means the preset requires a base URL and none was
|
||||||
// given.
|
// given.
|
||||||
ErrMissingBaseURL = errors.New("llm: base URL is required")
|
ErrMissingBaseURL = errors.New("llm: base URL is required")
|
||||||
|
|||||||
Reference in New Issue
Block a user