Keep base URL out of config errors and always keep a bounded error excerpt
New no longer echoes the supplied base URL in any configuration error, so
a credential in a rejected URL's query or fragment cannot leak.
llm.NewProviderError now takes the single credential sent with the
request ("" when none) and always keeps a body excerpt bounded to
MaxBodyExcerpt bytes, redacting the credential first. Keyless Ollama
errors therefore keep the vendor's explanation, e.g. model not found.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -56,23 +56,15 @@ type ProviderError struct {
|
|||||||
body string
|
body string
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewProviderError returns the error for an unexpected HTTP status.
|
// NewProviderError returns the error for an unexpected HTTP status. It
|
||||||
// secrets must hold the API key (and any other credential sent with the
|
// always keeps a body excerpt of at most MaxBodyExcerpt bytes. secret is the
|
||||||
// request). Every occurrence of each non-empty secret in body is replaced
|
// credential sent with the request, or "" when the request sent none; every
|
||||||
// before the body is cut to MaxBodyExcerpt bytes, so no part of a secret
|
// occurrence of a non-empty secret in body is replaced before the body is
|
||||||
// survives. If secrets holds no non-empty value the body cannot be
|
// cut, so no part of it survives.
|
||||||
// redacted, so no excerpt is kept at all.
|
func NewProviderError(status int, body []byte, secret string) *ProviderError {
|
||||||
func NewProviderError(status int, body []byte, secrets []string) *ProviderError {
|
|
||||||
s := string(body)
|
s := string(body)
|
||||||
redactedAny := false
|
if secret != "" {
|
||||||
for _, secret := range secrets {
|
s = strings.ReplaceAll(s, secret, redacted)
|
||||||
if secret != "" {
|
|
||||||
s = strings.ReplaceAll(s, secret, redacted)
|
|
||||||
redactedAny = true
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if !redactedAny {
|
|
||||||
return &ProviderError{status: status}
|
|
||||||
}
|
}
|
||||||
s = strings.TrimSpace(s)
|
s = strings.TrimSpace(s)
|
||||||
if len(s) > MaxBodyExcerpt {
|
if len(s) > MaxBodyExcerpt {
|
||||||
@@ -89,7 +81,7 @@ func NewProviderError(status int, body []byte, secrets []string) *ProviderError
|
|||||||
func (e *ProviderError) Status() int { return e.status }
|
func (e *ProviderError) Status() int { return e.status }
|
||||||
|
|
||||||
// Body returns the redacted, truncated response-body excerpt; it is empty
|
// Body returns the redacted, truncated response-body excerpt; it is empty
|
||||||
// when the vendor sent no body or no secret was given to redact it with.
|
// when the vendor sent no body.
|
||||||
func (e *ProviderError) Body() string { return e.body }
|
func (e *ProviderError) Body() string { return e.body }
|
||||||
|
|
||||||
func (e *ProviderError) Error() string {
|
func (e *ProviderError) Error() string {
|
||||||
|
|||||||
@@ -46,7 +46,7 @@ func TestRateLimitedError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestProviderError(t *testing.T) {
|
func TestProviderError(t *testing.T) {
|
||||||
err := fmt.Errorf("gemini: %w", NewProviderError(503, []byte(` {"error":"overloaded"} `), []string{"sk-test"}))
|
err := fmt.Errorf("gemini: %w", NewProviderError(503, []byte(` {"error":"overloaded"} `), "sk-test"))
|
||||||
if !errors.Is(err, ErrProvider) {
|
if !errors.Is(err, ErrProvider) {
|
||||||
t.Fatal("errors.Is(err, ErrProvider) = false")
|
t.Fatal("errors.Is(err, ErrProvider) = false")
|
||||||
}
|
}
|
||||||
@@ -63,7 +63,7 @@ func TestProviderError(t *testing.T) {
|
|||||||
if got := err.Error(); got != `gemini: llm: provider error: HTTP 503: {"error":"overloaded"}` {
|
if got := err.Error(); got != `gemini: llm: provider error: HTTP 503: {"error":"overloaded"}` {
|
||||||
t.Fatalf("Error() = %q", got)
|
t.Fatalf("Error() = %q", got)
|
||||||
}
|
}
|
||||||
if got := NewProviderError(500, nil, []string{"sk-test"}).Error(); got != "llm: provider error: HTTP 500" {
|
if got := NewProviderError(500, nil, "sk-test").Error(); got != "llm: provider error: HTTP 500" {
|
||||||
t.Fatalf("Error() without body = %q", got)
|
t.Fatalf("Error() without body = %q", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -71,7 +71,7 @@ func TestProviderError(t *testing.T) {
|
|||||||
func TestProviderErrorRedactsSecrets(t *testing.T) {
|
func TestProviderErrorRedactsSecrets(t *testing.T) {
|
||||||
const key = "sk-live-0123456789abcdef"
|
const key = "sk-live-0123456789abcdef"
|
||||||
body := []byte(`{"error":"invalid key sk-live-0123456789abcdef","echo":"Bearer sk-live-0123456789abcdef"}`)
|
body := []byte(`{"error":"invalid key sk-live-0123456789abcdef","echo":"Bearer sk-live-0123456789abcdef"}`)
|
||||||
pe := NewProviderError(400, body, []string{key, ""})
|
pe := NewProviderError(400, body, key)
|
||||||
for _, s := range []string{pe.Error(), pe.Body()} {
|
for _, s := range []string{pe.Error(), pe.Body()} {
|
||||||
if strings.Contains(s, key) {
|
if strings.Contains(s, key) {
|
||||||
t.Fatalf("secret leaked: %q", s)
|
t.Fatalf("secret leaked: %q", s)
|
||||||
@@ -88,7 +88,7 @@ func TestProviderErrorRedactsBeforeTruncating(t *testing.T) {
|
|||||||
// The key starts just before the excerpt limit, so truncating first
|
// The key starts just before the excerpt limit, so truncating first
|
||||||
// would leave its prefix in the excerpt.
|
// would leave its prefix in the excerpt.
|
||||||
body := strings.Repeat("x", MaxBodyExcerpt-5) + key
|
body := strings.Repeat("x", MaxBodyExcerpt-5) + key
|
||||||
pe := NewProviderError(500, []byte(body), []string{key})
|
pe := NewProviderError(500, []byte(body), key)
|
||||||
if strings.Contains(pe.Body(), "sk-se") {
|
if strings.Contains(pe.Body(), "sk-se") {
|
||||||
t.Fatalf("secret prefix leaked: %q", pe.Body())
|
t.Fatalf("secret prefix leaked: %q", pe.Body())
|
||||||
}
|
}
|
||||||
@@ -96,26 +96,24 @@ func TestProviderErrorRedactsBeforeTruncating(t *testing.T) {
|
|||||||
|
|
||||||
func TestProviderErrorTruncatesOnRuneBoundary(t *testing.T) {
|
func TestProviderErrorTruncatesOnRuneBoundary(t *testing.T) {
|
||||||
body := strings.Repeat("a", MaxBodyExcerpt-1) + "é" + strings.Repeat("b", 100)
|
body := strings.Repeat("a", MaxBodyExcerpt-1) + "é" + strings.Repeat("b", 100)
|
||||||
pe := NewProviderError(500, []byte(body), []string{"sk-test"})
|
pe := NewProviderError(500, []byte(body), "sk-test")
|
||||||
want := strings.Repeat("a", MaxBodyExcerpt-1) + "…"
|
want := strings.Repeat("a", MaxBodyExcerpt-1) + "…"
|
||||||
if pe.Body() != want {
|
if pe.Body() != want {
|
||||||
t.Fatalf("Body() has %d bytes, want %d", len(pe.Body()), len(want))
|
t.Fatalf("Body() has %d bytes, want %d", len(pe.Body()), len(want))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestProviderErrorOmitsBodyWithoutSecret(t *testing.T) {
|
func TestProviderErrorKeepsBoundedBodyWithoutSecret(t *testing.T) {
|
||||||
const key = "sk-live-0123456789abcdef"
|
body := []byte(` {"error":"model \"llama9\" not found"} `)
|
||||||
body := []byte(`{"error":"invalid key sk-live-0123456789abcdef"}`)
|
pe := NewProviderError(404, body, "")
|
||||||
for _, secrets := range [][]string{nil, {}, {""}} {
|
if pe.Body() != `{"error":"model \"llama9\" not found"}` {
|
||||||
pe := NewProviderError(401, body, secrets)
|
t.Fatalf("Body() = %q", pe.Body())
|
||||||
if pe.Body() != "" {
|
}
|
||||||
t.Fatalf("secrets %q: Body() = %q, want empty", secrets, pe.Body())
|
if got := pe.Error(); got != `llm: provider error: HTTP 404: {"error":"model \"llama9\" not found"}` {
|
||||||
}
|
t.Fatalf("Error() = %q", got)
|
||||||
if got := pe.Error(); got != "llm: provider error: HTTP 401" {
|
}
|
||||||
t.Fatalf("secrets %q: Error() = %q", secrets, got)
|
long := NewProviderError(500, []byte(strings.Repeat("x", 4*MaxBodyExcerpt)), "")
|
||||||
}
|
if want := strings.Repeat("x", MaxBodyExcerpt) + "…"; long.Body() != want {
|
||||||
if strings.Contains(pe.Error(), key) {
|
t.Fatalf("Body() has %d bytes, want %d", len(long.Body()), len(want))
|
||||||
t.Fatalf("secrets %q: secret leaked: %q", secrets, pe.Error())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -74,15 +74,17 @@ var _ llm.Provider = (*Provider)(nil)
|
|||||||
|
|
||||||
// New validates cfg and returns a Provider.
|
// New validates cfg and returns a Provider.
|
||||||
func New(cfg Config) (*Provider, error) {
|
func New(cfg Config) (*Provider, error) {
|
||||||
|
// Base URL errors never echo the URL: a rejected one may carry a
|
||||||
|
// credential.
|
||||||
u, err := url.Parse(cfg.BaseURL)
|
u, err := url.Parse(cfg.BaseURL)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, errors.New("openai: base URL does not parse")
|
return nil, errors.New("openai: base URL does not parse")
|
||||||
}
|
}
|
||||||
if (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
|
if (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
|
||||||
return nil, fmt.Errorf("openai: base URL %q must be an absolute http or https URL", u.Redacted())
|
return nil, errors.New("openai: base URL must be an absolute http or https URL")
|
||||||
}
|
}
|
||||||
if u.User != nil || u.RawQuery != "" || u.Fragment != "" {
|
if u.User != nil || u.RawQuery != "" || u.Fragment != "" {
|
||||||
return nil, fmt.Errorf("openai: base URL %q must not carry credentials, a query or a fragment", u.Redacted())
|
return nil, errors.New("openai: base URL must not carry credentials, a query or a fragment")
|
||||||
}
|
}
|
||||||
switch cfg.StructuredOutput {
|
switch cfg.StructuredOutput {
|
||||||
case JSONSchema, JSONObject, PromptOnly:
|
case JSONSchema, JSONObject, PromptOnly:
|
||||||
@@ -217,7 +219,7 @@ func (p *Provider) Generate(ctx context.Context, req llm.Request) (llm.Response,
|
|||||||
}
|
}
|
||||||
if len(cr.Error) > 0 && string(cr.Error) != "null" {
|
if len(cr.Error) > 0 && string(cr.Error) != "null" {
|
||||||
// OpenRouter can report an upstream failure in a 2xx body.
|
// OpenRouter can report an upstream failure in a 2xx body.
|
||||||
return llm.Response{}, fmt.Errorf("openai: generate: %w", llm.NewProviderError(status, raw, p.secrets()))
|
return llm.Response{}, fmt.Errorf("openai: generate: %w", llm.NewProviderError(status, raw, p.apiKey))
|
||||||
}
|
}
|
||||||
if len(cr.Choices) == 0 {
|
if len(cr.Choices) == 0 {
|
||||||
return llm.Response{}, errors.New("openai: generate: response has no choices")
|
return llm.Response{}, errors.New("openai: generate: response has no choices")
|
||||||
@@ -319,8 +321,6 @@ func (p *Provider) Verify(ctx context.Context) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *Provider) secrets() []string { return []string{p.apiKey} }
|
|
||||||
|
|
||||||
// do sends one request and returns the body and status of a 2xx response.
|
// do sends one request and returns the body and status of a 2xx response.
|
||||||
// Any other status becomes the matching llm typed error.
|
// Any other status becomes the matching llm typed error.
|
||||||
func (p *Provider) do(ctx context.Context, method, path string, payload []byte) ([]byte, int, error) {
|
func (p *Provider) do(ctx context.Context, method, path string, payload []byte) ([]byte, int, error) {
|
||||||
@@ -376,7 +376,7 @@ func (p *Provider) statusError(resp *http.Response, body []byte) error {
|
|||||||
case http.StatusRequestTimeout:
|
case http.StatusRequestTimeout:
|
||||||
return fmt.Errorf("HTTP %d: %w", resp.StatusCode, llm.ErrTimeout)
|
return fmt.Errorf("HTTP %d: %w", resp.StatusCode, llm.ErrTimeout)
|
||||||
default:
|
default:
|
||||||
return llm.NewProviderError(resp.StatusCode, body, p.secrets())
|
return llm.NewProviderError(resp.StatusCode, body, p.apiKey)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -498,6 +498,64 @@ func TestTransportErrorHasNoSecret(t *testing.T) {
|
|||||||
assertNoSecret(t, err)
|
assertNoSecret(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestKeyedErrorsNeverLeakKey drives every error path of the keyed OpenAI
|
||||||
|
// and OpenRouter shapes with vendor bodies that echo the key.
|
||||||
|
func TestKeyedErrorsNeverLeakKey(t *testing.T) {
|
||||||
|
shapes := map[string]struct {
|
||||||
|
path string
|
||||||
|
headers map[string]string
|
||||||
|
}{
|
||||||
|
"OpenAI": {path: "/v1"},
|
||||||
|
"OpenRouter": {path: "/api/v1", headers: map[string]string{"HTTP-Referer": "https://mealquest.example", "X-Title": "MealQuest"}},
|
||||||
|
}
|
||||||
|
fixtures := []string{"openai_error_invalid_key.json", "openai_error_server.json", "openrouter_error_rate_limited.json"}
|
||||||
|
statuses := []int{400, 401, 403, 404, 408, 429, 500, 502, 503}
|
||||||
|
for name, shape := range shapes {
|
||||||
|
for _, fixture := range fixtures {
|
||||||
|
for _, status := range statuses {
|
||||||
|
vs := newVendorServer(t, reply{status: status, fixture: fixture})
|
||||||
|
p := newProvider(t, vs.URL+shape.path, JSONSchema, shape.headers)
|
||||||
|
_, genErr := p.Generate(context.Background(), schemaRequest())
|
||||||
|
_, listErr := p.ListModels(context.Background())
|
||||||
|
verifyErr := p.Verify(context.Background())
|
||||||
|
for _, err := range []error{genErr, listErr, verifyErr} {
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("%s %s HTTP %d: nil error", name, fixture, status)
|
||||||
|
}
|
||||||
|
assertNoSecret(t, err)
|
||||||
|
var pe *llm.ProviderError
|
||||||
|
if errors.As(err, &pe) {
|
||||||
|
assertNoSecret(t, errors.New(pe.Body()))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestKeylessOllamaErrorKeepsExcerpt(t *testing.T) {
|
||||||
|
vs := newVendorServer(t, reply{status: 404, fixture: "ollama_error_not_found.json"})
|
||||||
|
p, err := New(Config{BaseURL: vs.URL + "/v1", Model: "llama9", StructuredOutput: PromptOnly})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
_, err = p.Generate(context.Background(), schemaRequest())
|
||||||
|
var pe *llm.ProviderError
|
||||||
|
if !errors.As(err, &pe) || pe.Status() != 404 {
|
||||||
|
t.Fatalf("err = %v, want a 404 ProviderError", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(pe.Body(), `model \"llama9\" not found`) || len(pe.Body()) > llm.MaxBodyExcerpt {
|
||||||
|
t.Fatalf("Body() = %q, want the bounded not-found explanation", pe.Body())
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), pe.Body()) {
|
||||||
|
t.Fatalf("Error() = %q, want it to carry the excerpt", err)
|
||||||
|
}
|
||||||
|
assertNoSecret(t, err)
|
||||||
|
if got := vs.only(t).header.Get("Authorization"); got != "" {
|
||||||
|
t.Fatalf("keyless request sent Authorization %q", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestGenerateRejectsInvalidRequests(t *testing.T) {
|
func TestGenerateRejectsInvalidRequests(t *testing.T) {
|
||||||
vs := newVendorServer(t, reply{status: 200, fixture: "ollama_chat_text.json"})
|
vs := newVendorServer(t, reply{status: 200, fixture: "ollama_chat_text.json"})
|
||||||
p := newProvider(t, vs.URL, JSONSchema, nil)
|
p := newProvider(t, vs.URL, JSONSchema, nil)
|
||||||
@@ -524,7 +582,11 @@ func TestNewRejectsBadConfig(t *testing.T) {
|
|||||||
"relative base": {BaseURL: "api.openai.com/v1", StructuredOutput: JSONSchema},
|
"relative base": {BaseURL: "api.openai.com/v1", StructuredOutput: JSONSchema},
|
||||||
"ftp base": {BaseURL: "ftp://example.com", StructuredOutput: JSONSchema},
|
"ftp base": {BaseURL: "ftp://example.com", StructuredOutput: JSONSchema},
|
||||||
"credentials in URL": {BaseURL: "https://user:" + testKey + "@example.com/v1", StructuredOutput: JSONSchema},
|
"credentials in URL": {BaseURL: "https://user:" + testKey + "@example.com/v1", StructuredOutput: JSONSchema},
|
||||||
"query in URL": {BaseURL: "https://example.com/v1?key=x", StructuredOutput: JSONSchema},
|
"key in query": {BaseURL: "https://example.com/v1?key=" + testKey, StructuredOutput: JSONSchema},
|
||||||
|
"key in fragment": {BaseURL: "https://example.com/v1#" + testKey, StructuredOutput: JSONSchema},
|
||||||
|
"key, relative base": {BaseURL: "example.com/v1?key=" + testKey, StructuredOutput: JSONSchema},
|
||||||
|
"key, ftp base": {BaseURL: "ftp://example.com/v1?key=" + testKey, StructuredOutput: JSONSchema},
|
||||||
|
"key, unparseable": {BaseURL: "https://example.com/%zz?key=" + testKey, StructuredOutput: JSONSchema},
|
||||||
"unknown mode": {BaseURL: "https://example.com/v1", StructuredOutput: "json"},
|
"unknown mode": {BaseURL: "https://example.com/v1", StructuredOutput: "json"},
|
||||||
"no mode": {BaseURL: "https://example.com/v1"},
|
"no mode": {BaseURL: "https://example.com/v1"},
|
||||||
"authorization hdr": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"authorization": "Bearer x"}},
|
"authorization hdr": {BaseURL: "https://example.com/v1", StructuredOutput: JSONSchema, Headers: map[string]string{"authorization": "Bearer x"}},
|
||||||
|
|||||||
Reference in New Issue
Block a user