mirror of
https://gitcode.com/JianFeeeee/ModelRouter.git
synced 2026-10-03 23:54:06 +00:00
fix(sources): implement PUT and stop partial edits from clobbering api_key
Two defects on the admin source write path, both found while adding a model
to a live source by hand.
PUT /api/sources/{name} was advertised in the API index but never
implemented — handleSourcesAPI only switched on GET/POST/DELETE, so the
documented update verb answered 405 while the POST upsert behind it worked.
POST is an upsert that replaces the whole source, so a partial edit that did
not carry api_key persisted an empty or placeholder credential. The source
kept its name, base_url and models, the write returned 200, and the source
then answered 401 on the next request — long after the writing script exited
0. The WebUI had been routing around this by loading the real key through
?reveal=credentials; any script or partial update went straight into it.
- implement PUT, taking the name from the path and rejecting a body name
that disagrees rather than silently resolving to one of them
- inherit the stored credential when api_key is omitted or sent as the
literal "__KEEP__"; an explicit new key still rotates
- an empty api_key on a source that does not exist yet stays empty, since
credential-less local upstreams are legitimate
- add model_ids, an additive shorthand, so "add these models" never has to
read and echo the existing list back
- align the API index with the implementation
The model_ids merge had a first cut that dropped the existing list when the
request carried no models field; TestSourceModelIDsIsAdditive caught it.
Verified by mutation: removing PUT turns three tests red, flattening
resolveAPIKey into a pass-through turns TestSourceUpsertKeepsAPIKey red
on both subtests, and making model_ids replace instead of merge turns
TestSourceModelIDsIsAdditive red.
This commit is contained in:
@ -78,6 +78,62 @@ type sourcePayload struct {
|
||||
MaxTokens int `json:"max_tokens"`
|
||||
MaxConcurrent int `json:"max_concurrent"`
|
||||
RPM int `json:"rpm"` // optional requests-per-minute cap, 0 = unlimited
|
||||
// ModelIDs is an additive-only shorthand: when set, the named models are
|
||||
// merged into an existing source instead of Models replacing the list.
|
||||
// It exists because "add one model" is the most common scripted edit and a
|
||||
// full Models list cannot be written without reading the source first.
|
||||
ModelIDs []string `json:"model_ids,omitempty"`
|
||||
}
|
||||
|
||||
// keepExistingAPIKey is the mask a client sends when it means "keep the
|
||||
// current credential". POST /api/sources is an upsert, so a script that edits
|
||||
// one field and echoes a placeholder back would otherwise persist that
|
||||
// placeholder as the live api_key — which fails closed as 401 on the next
|
||||
// request, long after the script exited successfully. See TestSourceUpsertKeepsAPIKey.
|
||||
const keepExistingAPIKey = "__KEEP__"
|
||||
|
||||
// resolveAPIKey returns the credential to store, inheriting the current one
|
||||
// when the payload does not supply a new one.
|
||||
//
|
||||
// Two distinct "no new credential" signals exist and they must not collapse:
|
||||
// - the explicit mask keepExistingAPIKey — always inherit.
|
||||
// - the empty string — inherit only for a source that already has a
|
||||
// credential. Empty is a legitimate value for a *new* credential-less
|
||||
// source (dshcodebuddy uses "dsh-local-noauth", local servers use ""),
|
||||
// so a fresh source with an empty key must stay empty rather than inherit
|
||||
// nothing and fail differently.
|
||||
func (g *Gateway) resolveAPIKey(name, want string, exists bool) string {
|
||||
if want == keepExistingAPIKey {
|
||||
return g.currentAPIKey(name)
|
||||
}
|
||||
if exists && want == "" {
|
||||
return g.currentAPIKey(name)
|
||||
}
|
||||
return want
|
||||
}
|
||||
|
||||
func (g *Gateway) currentAPIKey(name string) string {
|
||||
if s := g.sourceByName(name); s != nil {
|
||||
return s.APIKey
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// sourceByName returns a live view of one source, or nil when it does not
|
||||
// exist. It goes through core.Sources() so the key is already unsealed and
|
||||
// defaults applied, matching what the routes actually use.
|
||||
func (g *Gateway) sourceByName(name string) *config.Source {
|
||||
for _, s := range g.core.Sources() {
|
||||
if s.Name == name {
|
||||
ss := s
|
||||
return &ss
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *Gateway) sourceExists(name string) bool {
|
||||
return g.sourceByName(name) != nil
|
||||
}
|
||||
|
||||
func (g *Gateway) handleSourcesAPI(w http.ResponseWriter, r *http.Request) {
|
||||
@ -91,17 +147,34 @@ func (g *Gateway) handleSourcesAPI(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.Method {
|
||||
case http.MethodGet:
|
||||
writeJSON(w, http.StatusOK, map[string]interface{}{"sources": g.core.Sources()})
|
||||
case http.MethodPost:
|
||||
case http.MethodPost, http.MethodPut:
|
||||
var p sourcePayload
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
if err := json.Unmarshal(body, &p); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "invalid_request", "invalid json: "+err.Error())
|
||||
return
|
||||
}
|
||||
// PUT addresses the source by path; the body name is only a fallback so
|
||||
// a single-field edit does not have to echo the name back. A body that
|
||||
// disagrees with the path is a caller bug, not something to silently
|
||||
// resolve — it usually means the wrong source is about to be written.
|
||||
if r.Method == http.MethodPut {
|
||||
if path == "" {
|
||||
writeError(w, http.StatusBadRequest, "invalid_request", "source name required in path")
|
||||
return
|
||||
}
|
||||
if p.Name != "" && p.Name != path {
|
||||
writeError(w, http.StatusBadRequest, "invalid_request",
|
||||
"source name in body ("+p.Name+") does not match path ("+path+")")
|
||||
return
|
||||
}
|
||||
p.Name = path
|
||||
}
|
||||
exists := g.sourceExists(p.Name)
|
||||
src := config.Source{
|
||||
Name: p.Name,
|
||||
BaseURL: p.BaseURL,
|
||||
APIKey: p.APIKey,
|
||||
APIKey: g.resolveAPIKey(p.Name, p.APIKey, exists),
|
||||
Adapter: p.Adapter,
|
||||
Endpoint: p.Endpoint,
|
||||
ImageEndpoint: p.ImageEndpoint,
|
||||
@ -113,6 +186,29 @@ func (g *Gateway) handleSourcesAPI(w http.ResponseWriter, r *http.Request) {
|
||||
MaxConcurrent: p.MaxConcurrent,
|
||||
RPM: p.RPM,
|
||||
}
|
||||
// model_ids is additive: "add these models" is the common scripted edit
|
||||
// and it must not require reading (and echoing) the whole list back.
|
||||
// A request that omits models entirely is therefore a pure add, not a
|
||||
// request to empty the list -- that distinction is the whole point.
|
||||
if len(p.ModelIDs) > 0 {
|
||||
if len(src.Models) == 0 {
|
||||
if existing := g.sourceByName(p.Name); existing != nil {
|
||||
src.Models = append(src.Models, existing.Models...)
|
||||
}
|
||||
}
|
||||
have := map[string]bool{}
|
||||
for _, m := range src.Models {
|
||||
have[m.ID] = true
|
||||
}
|
||||
for _, id := range p.ModelIDs {
|
||||
id = strings.TrimSpace(id)
|
||||
if id == "" || have[id] {
|
||||
continue
|
||||
}
|
||||
have[id] = true
|
||||
src.Models = append(src.Models, config.Model{ID: id, Kind: "chat"})
|
||||
}
|
||||
}
|
||||
if err := g.core.AddSource(src); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "source_error", err.Error())
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user