diff --git a/internal/ai/llm/openai/openai.go b/internal/ai/llm/openai/openai.go index 0258c01..5a1b4e3 100644 --- a/internal/ai/llm/openai/openai.go +++ b/internal/ai/llm/openai/openai.go @@ -54,7 +54,9 @@ type Config struct { // need it. Model string // Headers are extra request headers, e.g. OpenRouter's HTTP-Referer and - // X-Title. They must not set Authorization or Content-Type. + // X-Title. An Accept header replaces the default "application/json". + // Headers the adapter or net/http set themselves (see managedHeaders) + // are rejected. Headers map[string]string // StructuredOutput is how JSON is requested when a Request has a Schema. StructuredOutput StructuredOutput @@ -91,17 +93,24 @@ func New(cfg Config) (*Provider, error) { default: return nil, fmt.Errorf("openai: unknown structured output mode %q", cfg.StructuredOutput) } - headers := make(http.Header, len(cfg.Headers)) + headers := make(http.Header, len(cfg.Headers)+1) for name, value := range cfg.Headers { + // Values are never echoed: one may carry a credential. + if !validHeaderName(name) { + return nil, fmt.Errorf("openai: extra header name %q is not a valid HTTP token", name) + } canonical := textproto.CanonicalMIMEHeaderKey(name) - if canonical == "Authorization" || canonical == "Content-Type" { + if managedHeaders[canonical] { return nil, fmt.Errorf("openai: extra header %q is set by the adapter", canonical) } - if name == "" || strings.ContainsAny(name, " \t\r\n:") || strings.ContainsAny(value, "\r\n") { - return nil, fmt.Errorf("openai: extra header %q is malformed", name) + if !validHeaderValue(value) { + return nil, fmt.Errorf("openai: extra header %q has a control character in its value", canonical) } headers.Set(canonical, value) } + if headers.Get("Accept") == "" { + headers.Set("Accept", "application/json") + } return &Provider{ base: strings.TrimRight(cfg.BaseURL, "/"), apiKey: cfg.APIKey, @@ -112,6 +121,46 @@ func New(cfg Config) (*Provider, error) { }, nil } +// managedHeaders are the request headers the adapter or net/http set; a +// configured value for one would be overwritten or ignored. +var managedHeaders = map[string]bool{ + "Authorization": true, + "Content-Type": true, + "Content-Length": true, + "Transfer-Encoding": true, + "Host": true, + "Connection": true, +} + +// validHeaderName reports whether name is an RFC 9110 field-name: a +// non-empty token of letters, digits and !#$%&'*+-.^_`|~. +func validHeaderName(name string) bool { + if name == "" { + return false + } + for i := 0; i < len(name); i++ { + c := name[i] + switch { + case 'a' <= c && c <= 'z', 'A' <= c && c <= 'Z', '0' <= c && c <= '9': + case strings.IndexByte("!#$%&'*+-.^_`|~", c) >= 0: + default: + return false + } + } + return true +} + +// validHeaderValue reports whether value is an RFC 9110 field-value: no +// control characters other than horizontal tab, so no CR, LF or DEL. +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 +} + // maxResponseBody caps a successful response body; a larger one is an error. const maxResponseBody = 16 << 20 @@ -339,7 +388,6 @@ func (p *Provider) do(ctx context.Context, method, path string, payload []byte) if payload != nil { httpReq.Header.Set("Content-Type", "application/json") } - httpReq.Header.Set("Accept", "application/json") if p.apiKey != "" { httpReq.Header.Set("Authorization", "Bearer "+p.apiKey) } diff --git a/internal/ai/llm/openai/openai_test.go b/internal/ai/llm/openai/openai_test.go index 8548d4b..30d2dfd 100644 --- a/internal/ai/llm/openai/openai_test.go +++ b/internal/ai/llm/openai/openai_test.go @@ -619,6 +619,40 @@ func TestGenerateRejectsInvalidRequests(t *testing.T) { } } +func TestNewRejectsBadHeaderWithoutEchoingValue(t *testing.T) { + _, err := New(Config{BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title": testKey + "\u0019"}}) + 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 + want string + }{ + "default": {nil, "application/json"}, + "configured": {map[string]string{"accept": "application/json, text/event-stream"}, "application/json, text/event-stream"}, + } { + vs := newVendorServer(t, reply{status: 200, fixture: "openai_models.json"}) + p := newProvider(t, vs.URL+"/v1", JSONSchema, tc.headers) + if _, err := p.ListModels(context.Background()); err != nil { + t.Fatalf("%s: %v", name, err) + } + if got := vs.only(t).header.Values("Accept"); len(got) != 1 || got[0] != tc.want { + t.Errorf("%s: Accept = %q, want %q", name, got, tc.want) + } + } +} + +func TestNewAcceptsValidHeaders(t *testing.T) { + headers := map[string]string{"X-Trace_ID.v2!#$%&'*+^`|~": "tab\tand obs-text \xe2\x9c\x93", "X-Empty": ""} + if _, err := New(Config{BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: headers}); err != nil { + t.Fatal(err) + } +} + func TestNewRejectsBadConfig(t *testing.T) { cases := map[string]Config{ "empty base": {StructuredOutput: JSONSchema}, @@ -636,6 +670,15 @@ func TestNewRejectsBadConfig(t *testing.T) { "content-type hdr": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"Content-Type": "text/plain"}}, "header newline": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title": "a\r\nb"}}, "header bad name": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X Title": "a"}}, + "header empty name": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"": "a"}}, + "header bracket": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X[Trace]": "a"}}, + "header colon": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title:": "a"}}, + "header non-ASCII": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Tïtle": "a"}}, + "header U+0019": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title": "a\u0019b"}}, + "header NUL": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title": "a\x00b"}}, + "header DEL": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"X-Title": "a\x7fb"}}, + "host hdr": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"host": "other.example"}}, + "content-length hdr": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"Content-Length": "1"}}, } for name, cfg := range cases { cfg.APIKey = testKey