mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-15 02:16:56 +00:00
fix(providers): make provider_type immutable + add argString tests
Prevent SSRF bypass via provider_type change on update — ACP skips URL validation, so changing type post-creation could circumvent the check. Add table-driven unit tests for argString() covering all JSON type coercion paths (float64, int, NaN, json.Number, nil).
This commit is contained in:
1 parent
238d639de7
commit
f4369c51e4
2 files changed
+38
-12
No files matched your search
@@ -376,15 +376,11 @@ func (h *ProvidersHandler) handleUpdateProvider(w http.ResponseWriter, r *http.R
|
||||
}
|
||||
}
|
||||
|
||||
// Validate provider_type if being updated.
|
||||
// IMPORTANT: Do NOT replace this with delete(updates, "provider_type").
|
||||
// We must return 400 so the caller knows the value is invalid,
|
||||
// silently deleting it would hide the error from the end user.
|
||||
if pt, ok := updates["provider_type"]; ok {
|
||||
if s, _ := pt.(string); !store.ValidProviderTypes[s] {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidRequest, "unsupported provider_type")})
|
||||
return
|
||||
}
|
||||
// provider_type is immutable after creation — changing it would bypass
|
||||
// security checks (e.g. ACP skips URL validation). Reject if client tries.
|
||||
if _, ok := updates["provider_type"]; ok {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidRequest, "provider_type cannot be changed after creation")})
|
||||
return
|
||||
}
|
||||
|
||||
// Strip masked API key — don't overwrite real value with "***"
|
||||
@@ -407,9 +403,6 @@ func (h *ProvidersHandler) handleUpdateProvider(w http.ResponseWriter, r *http.R
|
||||
if name, ok := updates["name"].(string); ok && name != "" {
|
||||
candidate.Name = name
|
||||
}
|
||||
if providerType, ok := updates["provider_type"].(string); ok && providerType != "" {
|
||||
candidate.ProviderType = providerType
|
||||
}
|
||||
if apiKey, ok := updates["api_key"].(string); ok {
|
||||
candidate.APIKey = apiKey
|
||||
}
|
||||
|
||||
@@ -2,6 +2,8 @@ package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"math"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
@@ -497,6 +499,37 @@ func TestMessageToolNumericTargetUsesSendPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestArgString(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
args map[string]any
|
||||
key string
|
||||
want string
|
||||
}{
|
||||
{"string value", map[string]any{"k": "hello"}, "k", "hello"},
|
||||
{"string with spaces", map[string]any{"k": " hi "}, "k", "hi"},
|
||||
{"empty string", map[string]any{"k": ""}, "k", ""},
|
||||
{"missing key", map[string]any{}, "k", ""},
|
||||
{"nil value", map[string]any{"k": nil}, "k", ""},
|
||||
{"float64 integer", map[string]any{"k": float64(-1001847298537)}, "k", "-1001847298537"},
|
||||
{"float64 positive", map[string]any{"k": float64(42)}, "k", "42"},
|
||||
{"float64 zero", map[string]any{"k": float64(0)}, "k", "0"},
|
||||
{"float64 NaN", map[string]any{"k": math.NaN()}, "k", ""},
|
||||
{"int", map[string]any{"k": 123}, "k", "123"},
|
||||
{"int64", map[string]any{"k": int64(-999)}, "k", "-999"},
|
||||
{"json.Number", map[string]any{"k": json.Number("7654321")}, "k", "7654321"},
|
||||
{"bool fallback", map[string]any{"k": true}, "k", "true"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := argString(tt.args, tt.key)
|
||||
if got != tt.want {
|
||||
t.Errorf("argString(%v, %q) = %q, want %q", tt.args, tt.key, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeliveredMedia(t *testing.T) {
|
||||
dm := NewDeliveredMedia()
|
||||
|
||||
|
||||
Reference in new issue
Block a user