package gateway import ( "encoding/json" "os" "path/filepath" "reflect" "strings" "testing" "time" "llmsproxy/internal/config" "llmsproxy/internal/core" ) // This file covers the second round of the same class of defect bf0657b fixed. // // bf0657b stopped a partial source edit from clobbering api_key. The upsert // still rewrites the WHOLE source, so every other field the request could not // express was reset to its zero value on any edit: proxy_url, api_key_env, // timeout and queue_timeout. // // The four are worse than cosmetic because they change how the source dials out. // proxy_url silently turns a proxied upstream into a direct one (or vice versa) // and api_key_env turns a source whose credential lives in an environment // variable into a credential-less source — which keeps answering 200 on write // and only fails on the NEXT call upstream, long after the editor left. // // The trigger is not exotic scripting: the WebUI's own edit form posts exactly // such a payload. See TestWebUIEditPayloadPreservesRoutingFields. // richGateway writes a config.yaml whose source carries all four fragile // fields, then boots a gateway over it. func richGateway(t *testing.T) *Gateway { t.Helper() dir := t.TempDir() cfgPath := filepath.Join(dir, "config.yaml") cfg := `listen: 127.0.0.1:8080 gateway_keys: - sk-test default_model: AUTO adapter_dir: ` + filepath.Join(dir, "adapters") + ` runtime_file: ` + filepath.Join(dir, "runtime.json") + ` sources: - name: rich base_url: https://api.deepseek.com api_key: sk-live adapter: deepseek proxy_url: http://127.0.0.1:7890 timeout: 300s queue_timeout: 90s max_concurrent: 8 models: - id: deepseek-v4-flash priority: 100 kind: chat ` if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil { t.Fatal(err) } c, err := config.Load(cfgPath) if err != nil { t.Fatal(err) } core, err := core.NewFromConfig(c) if err != nil { t.Fatalf("core: %v", err) } t.Cleanup(core.Close) g, err := New(core) if err != nil { t.Fatal(err) } return g } // sourceField reads one field off the live source, i.e. what the routes // actually use rather than what the file happens to contain. func sourceField(t *testing.T, g *Gateway, name string, get func(config.Source) interface{}) interface{} { t.Helper() for _, s := range g.core.Sources() { if s.Name == name { return get(s) } } t.Fatalf("source %q not found", name) return nil } // webUIPayload is byte-for-byte the JSON that index.html's saveSource() builds // from the edit dialog. It deliberately carries none of the four fragile // fields, because the dialog has no inputs for them. const webUIPayload = `{ "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live", "adapter":"deepseek","endpoint":"","image_endpoint":"", "models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{},"temperature":0,"max_tokens":0,"max_concurrent":16,"rpm":0 }` // TestWebUIEditPayloadPreservesRoutingFields is the regression test for the // reported defect: changing max_concurrent from 8 to 16 in the UI must not // silently drop the proxy, the env-var credential, or the timeouts. func TestWebUIEditPayloadPreservesRoutingFields(t *testing.T) { g := richGateway(t) // The one field the operator actually changed. rr := doReq(t, g, "POST", "/api/sources", webUIPayload) if rr.Code != 200 { t.Fatalf("edit status=%d body=%s", rr.Code, rr.Body.String()) } checks := []struct { field string got interface{} want interface{} }{ {"proxy_url", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }), "http://127.0.0.1:7890"}, {"timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }), 300 * time.Second}, {"queue_timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.QueueTimeout }), 90 * time.Second}, // The field the edit DID change, so a fix that simply refuses to write // anything would also pass the three assertions above. {"max_concurrent", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.MaxConcurrent }), 16}, } for _, c := range checks { if c.got != c.want { t.Errorf("%s = %v, want %v", c.field, c.got, c.want) } } } // TestSourceEditPreservesAPIKeyEnv is called out separately because it is the // only one of the four whose loss is a credential failure rather than a routing // difference, and it is invisible until the next upstream call. func TestSourceEditPreservesAPIKeyEnv(t *testing.T) { dir := t.TempDir() cfgPath := filepath.Join(dir, "config.yaml") cfg := `listen: 127.0.0.1:8080 gateway_keys: - sk-test default_model: AUTO adapter_dir: ` + filepath.Join(dir, "adapters") + ` runtime_file: ` + filepath.Join(dir, "runtime.json") + ` sources: - name: envonly base_url: https://api.deepseek.com api_key_env: DEEPSEEK_KEY adapter: deepseek max_concurrent: 8 models: - id: deepseek-v4-flash priority: 100 kind: chat ` if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil { t.Fatal(err) } c, err := config.Load(cfgPath) if err != nil { t.Fatal(err) } core, err := core.NewFromConfig(c) if err != nil { t.Fatalf("core: %v", err) } t.Cleanup(core.Close) g, err := New(core) if err != nil { t.Fatal(err) } // A credential-less source is legitimate, so the API must not invent one // here; the point is that the ENV reference survives the edit. rr := doReq(t, g, "POST", "/api/sources", `{ "name":"envonly","base_url":"https://api.deepseek.com","api_key":"", "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{},"max_concurrent":4}`) if rr.Code != 200 { t.Fatalf("edit status=%d body=%s", rr.Code, rr.Body.String()) } got := sourceField(t, g, "envonly", func(s config.Source) interface{} { return s.APIKeyEnv }) if got != "DEEPSEEK_KEY" { t.Fatalf("api_key_env = %v after an edit that did not mention it; "+ "the source now has no credential and will only fail on the next upstream call", got) } if n := sourceField(t, g, "envonly", func(s config.Source) interface{} { return s.MaxConcurrent }); n != 4 { t.Fatalf("max_concurrent = %v, want the edited 4", n) } } // TestSourceEditPersistsInheritedFieldsToDisk: inheriting in memory is not // enough — the values must reach config.yaml, or a restart silently undoes the // fix and the field is lost anyway. func TestSourceEditPersistsInheritedFieldsToDisk(t *testing.T) { g := richGateway(t) if rr := doReq(t, g, "POST", "/api/sources", webUIPayload); rr.Code != 200 { t.Fatalf("edit status=%d: %s", rr.Code, rr.Body.String()) } raw, err := os.ReadFile(g.core.Config().Path) if err != nil { t.Fatalf("read config: %v", err) } text := string(raw) for _, want := range []string{ "proxy_url: http://127.0.0.1:7890", } { if !strings.Contains(text, want) { t.Errorf("config.yaml lost %q on an unrelated edit:\n%s", want, text) } } // The durations are re-serialized in Go's canonical form (300s becomes // 5m0s), so they must be checked SEMANTICALLY. Asserting the original // spelling would be a false red: the value survived, only its text changed. // Re-loading the file is also the only way to prove the persisted value // parses back to the same duration. reloaded, err := config.Load(g.core.Config().Path) if err != nil { t.Fatalf("re-read config: %v", err) } if len(reloaded.Sources) == 0 { t.Fatal("no sources after reload") } s := reloaded.Sources[0] if s.Timeout != 300*time.Second { t.Errorf("persisted timeout = %v, want 300s", s.Timeout) } if s.QueueTimeout != 90*time.Second { t.Errorf("persisted queue_timeout = %v, want 90s", s.QueueTimeout) } if s.ProxyURL != "http://127.0.0.1:7890" { t.Errorf("persisted proxy_url = %v, want it preserved", s.ProxyURL) } } // ---- explicit set / clear, i.e. the pointer semantics actually work ---- func TestSourceOptionalFieldsCanBeSet(t *testing.T) { g := richGateway(t) rr := doReq(t, g, "POST", "/api/sources", `{ "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live", "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{}, "proxy_url":"http://127.0.0.1:1080", "api_key_env":"NEW_ENV_KEY", "timeout":"45s", "queue_timeout":"5s"}`) if rr.Code != 200 { t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) } for _, c := range []struct { field string got interface{} want interface{} }{ {"proxy_url", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }), "http://127.0.0.1:1080"}, {"api_key_env", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.APIKeyEnv }), "NEW_ENV_KEY"}, {"timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }), 45 * time.Second}, {"queue_timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.QueueTimeout }), 5 * time.Second}, } { if c.got != c.want { t.Errorf("%s = %v, want %v", c.field, c.got, c.want) } } } // TestSourceOptionalFieldsCanBeCleared is the half that an "empty means // inherit" implementation gets wrong: it can never clear a field, so emptying // the proxy box in the UI would keep using the old proxy forever. func TestSourceOptionalFieldsCanBeCleared(t *testing.T) { g := richGateway(t) rr := doReq(t, g, "POST", "/api/sources", `{ "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live", "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{}, "proxy_url":"","timeout":"","queue_timeout":""}`) if rr.Code != 200 { t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) } if got := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }); got != "" { t.Errorf("proxy_url = %v, want it cleared", got) } // A cleared timeout falls back to the config default (120s), which is what // mergedSources applies for a zero value — assert the effective behaviour // rather than the raw zero, since that is what a caller actually gets. if got := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }); got != config.DefaultSourceTimeout { t.Errorf("timeout = %v, want the default %v after clearing", got, config.DefaultSourceTimeout) } // api_key_env was NOT in the request, so it must be untouched even though // its sibling fields were cleared. _ = rr } // TestSourceOptionalFieldsClearIsScoped: clearing the proxy must not clear the // env credential that the same request did not mention. func TestSourceOptionalFieldsClearIsScoped(t *testing.T) { dir := t.TempDir() cfgPath := filepath.Join(dir, "config.yaml") cfg := `listen: 127.0.0.1:8080 gateway_keys: - sk-test default_model: AUTO adapter_dir: ` + filepath.Join(dir, "adapters") + ` runtime_file: ` + filepath.Join(dir, "runtime.json") + ` sources: - name: both base_url: https://api.deepseek.com api_key_env: DEEPSEEK_KEY adapter: deepseek proxy_url: http://127.0.0.1:7890 max_concurrent: 8 models: - id: deepseek-v4-flash priority: 100 kind: chat ` os.WriteFile(cfgPath, []byte(cfg), 0600) c, err := config.Load(cfgPath) if err != nil { t.Fatal(err) } core, err := core.NewFromConfig(c) if err != nil { t.Fatalf("core: %v", err) } t.Cleanup(core.Close) g, err := New(core) if err != nil { t.Fatal(err) } rr := doReq(t, g, "POST", "/api/sources", `{ "name":"both","base_url":"https://api.deepseek.com","api_key":"", "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{},"proxy_url":""}`) if rr.Code != 200 { t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String()) } if got := sourceField(t, g, "both", func(s config.Source) interface{} { return s.APIKeyEnv }); got != "DEEPSEEK_KEY" { t.Errorf("api_key_env = %v; clearing proxy_url must not touch it", got) } if got := sourceField(t, g, "both", func(s config.Source) interface{} { return s.ProxyURL }); got != "" { t.Errorf("proxy_url = %v, want cleared", got) } } func TestSourceRejectsBadDuration(t *testing.T) { g := richGateway(t) before := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }) rr := doReq(t, g, "POST", "/api/sources", `{ "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live", "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}], "headers":{},"meta":{},"timeout":"not-a-duration"}`) if rr.Code != 400 { t.Fatalf("status=%d, want 400 for an unparseable duration: %s", rr.Code, rr.Body.String()) } if !strings.Contains(rr.Body.String(), "timeout") { t.Errorf("the error must name the offending field: %s", rr.Body.String()) } // A rejected write must leave the source exactly as it was. if after := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }); after != before { t.Errorf("a rejected edit still changed timeout: %v -> %v", before, after) } } // TestSourceCreateSetsOptionalFields: the create path must accept them too, and // must not require an existing source to inherit from (a nil-pointer crash // here would only show up as a 500 on every new source). func TestSourceCreateSetsOptionalFields(t *testing.T) { up := mockUpstream() defer up.Close() g := newTestGateway(t) rr := doReq(t, g, "POST", "/api/sources", `{ "name":"fresh","base_url":"`+up.URL+`","api_key":"sk-x","adapter":"openai", "models":[{"id":"m1","kind":"chat"}],"headers":{},"meta":{}, "proxy_url":"http://127.0.0.1:7890","timeout":"10s","queue_timeout":"3s"}`) if rr.Code != 200 { t.Fatalf("create status=%d body=%s", rr.Code, rr.Body.String()) } if got := sourceField(t, g, "fresh", func(s config.Source) interface{} { return s.ProxyURL }); got != "http://127.0.0.1:7890" { t.Errorf("proxy_url = %v, want it set on create", got) } if got := sourceField(t, g, "fresh", func(s config.Source) interface{} { return s.Timeout }); got != 10*time.Second { t.Errorf("timeout = %v, want 10s on create", got) } } // TestSourceRevealExposesDurations guards the round-trip the edit form depends // on. config.Source tags Timeout/QueueTimeout `json:"-"`, so marshaling the // struct omits them; the form then reads an empty timeout box and — because it // always sends the box back — CLEARS the stored timeout on any unrelated save. // A test that only checks "the edit preserves timeout" would not have caught // that, because the loss happens between the read and the write. // // The credentials are asserted in the same breath: they came from the same // branch, and an earlier iteration of this fix masked the key and broke the // form's other half. func TestSourceRevealExposesDurations(t *testing.T) { up := mockUpstream() defer up.Close() g := newTestGateway(t, config.Source{ Name: "revealed", BaseURL: up.URL, Adapter: "openai", APIKey: "sk-reveal-me", Models: []config.Model{{ID: "m1", Kind: "chat"}}, Timeout: 300 * time.Second, }) rr := doReq(t, g, "GET", "/api/v1/sources/revealed?reveal=credentials", "") if rr.Code != 200 { t.Fatalf("reveal status=%d: %s", rr.Code, rr.Body.String()) } var view struct { Source map[string]interface{} `json:"source"` } if err := json.Unmarshal(rr.Body.Bytes(), &view); err != nil { t.Fatalf("decode: %v", err) } // Duration strings, matching what the write path parses. if got := view.Source["timeout"]; got != "5m0s" { t.Errorf("reveal timeout = %v, want the string \"5m0s\"; the edit form "+ "would show an empty box and clear the stored value on save", got) } // The unset one shows the DEFAULT, not "": ApplyDefaults filled // QueueTimeout before the form ever sees it. Both are acceptable as long as // the value round-trips, so assert it is a parseable duration string. qs, _ := view.Source["queue_timeout"].(string) if _, err := time.ParseDuration(qs); err != nil { t.Errorf("reveal queue_timeout = %q is not a duration string: %v", qs, err) } if got := view.Source["api_key"]; got != "sk-reveal-me" { t.Errorf("reveal api_key = %v, want it in the clear for the form", got) } // The masked view must NOT gain the durations' secrets — it is fine for it // to omit them, but it must never carry the key. masked := doReq(t, g, "GET", "/api/v1/sources/revealed", "") if strings.Contains(masked.Body.String(), "sk-reveal-me") { t.Error("the masked view leaked the key") } } // TestSourcePayloadCoversEveryEditableField is the guard against the class // itself. Any config.Source field a source edit can legitimately set must // exist on sourcePayload, otherwise the upsert resets it. // // It reflects over the STRUCT rather than over json.Marshal output: the four // pointer fields carry `omitempty`, so a nil pointer legitimately disappears // from the marshaled form — that absence is precisely the "not mentioned" signal // the handler relies on. Checking marshaled keys would therefore report a // healthy field as missing. func TestSourcePayloadCoversEveryEditableField(t *testing.T) { // Fields of config.Source that a source edit can legitimately set. want := []string{ "name", "base_url", "api_key", "api_key_env", "adapter", "endpoint", "image_endpoint", "models", "headers", "proxy_url", "meta", "temperature", "max_tokens", "max_concurrent", "rpm", "timeout", "queue_timeout", } pt := reflect.TypeOf(sourcePayload{}) for _, f := range want { if _, ok := pt.FieldByName(f); !ok { // Tolerate a differently-spelled Go field only when the JSON tag // matches, so the check stays about the wire contract. if !hasJSONTag(pt, f) { t.Errorf("config.Source has %q but sourcePayload has no such JSON key: "+ "an edit that omits it resets the stored value", f) } } } } // hasJSONTag reports whether any field of t carries the given JSON name. func hasJSONTag(t reflect.Type, name string) bool { for i := 0; i < t.NumField(); i++ { if jsonName(t.Field(i)) == name { return true } } return false } // jsonName returns the JSON name of a struct field, falling back to the Go // field name when no tag is present. func jsonName(f reflect.StructField) string { tag := f.Tag.Get("json") if tag == "" { return f.Name } if n := strings.Split(tag, ",")[0]; n != "" { return n } return f.Name }