From 4b6ed8cfce38bbe908cce22245020556341bb78d Mon Sep 17 00:00:00 2001 From: viettranx Date: Sat, 28 Mar 2026 16:55:59 +0700 Subject: [PATCH] fix(agent): broken cache invalidation + WS handler parity gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: InvalidateAgent() and Remove() used plain agentKey for cache deletion, but Get() stores with tenant-scoped key "tenantID:agentKey" — invalidation never matched, so old provider/model persisted up to 10min (TTL). Also fixed: - WS agents.update missing provider, status, context_window, max_tool_iterations, frontmatter, is_default, budget fields - WS agents.create ignoring user provider/model (hardcoded defaults) - WS channel_instances.update bypassing column allowlist (security) - HTTP agentAllowedFields missing sandbox_config, context_pruning --- internal/agent/resolver.go | 10 +++- internal/agent/router.go | 9 +++- internal/gateway/methods/agents_create.go | 35 +++++++++---- internal/gateway/methods/agents_update.go | 50 +++++++++++++++++-- internal/gateway/methods/channel_instances.go | 22 +++++++- internal/http/validate.go | 1 + 6 files changed, 109 insertions(+), 18 deletions(-) diff --git a/internal/agent/resolver.go b/internal/agent/resolver.go index 6c4cb914..9051342c 100644 --- a/internal/agent/resolver.go +++ b/internal/agent/resolver.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "path/filepath" + "strings" "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/bootstrap" @@ -406,10 +407,17 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc { // InvalidateAgent removes an agent from the router cache, forcing re-resolution. // Used when agent config is updated via API. +// Matches both plain key ("agentKey") and tenant-scoped key ("tenantID:agentKey") +// because callers only pass the agentKey without tenant context. func (r *Router) InvalidateAgent(agentKey string) { r.mu.Lock() defer r.mu.Unlock() - delete(r.agents, agentKey) + suffix := ":" + agentKey + for key := range r.agents { + if key == agentKey || strings.HasSuffix(key, suffix) { + delete(r.agents, key) + } + } slog.Debug("invalidated agent cache", "agent", agentKey) } diff --git a/internal/agent/router.go b/internal/agent/router.go index 486624e2..1ef3a776 100644 --- a/internal/agent/router.go +++ b/internal/agent/router.go @@ -3,6 +3,7 @@ package agent import ( "context" "fmt" + "strings" "sync" "time" @@ -118,10 +119,16 @@ func agentCacheKey(ctx context.Context, agentID string) string { } // Remove removes an agent from the router. +// Matches both plain and tenant-scoped keys (same as InvalidateAgent). func (r *Router) Remove(agentID string) { r.mu.Lock() defer r.mu.Unlock() - delete(r.agents, agentID) + suffix := ":" + agentID + for key := range r.agents { + if key == agentID || strings.HasSuffix(key, suffix) { + delete(r.agents, key) + } + } } // List returns all registered agent IDs. diff --git a/internal/gateway/methods/agents_create.go b/internal/gateway/methods/agents_create.go index 3bbd3378..a8d8965c 100644 --- a/internal/gateway/methods/agents_create.go +++ b/internal/gateway/methods/agents_create.go @@ -24,13 +24,18 @@ import ( func (m *AgentsMethods) handleCreate(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) { locale := store.LocaleFromContext(ctx) var params struct { - Name string `json:"name"` - Workspace string `json:"workspace"` - Emoji string `json:"emoji"` - Avatar string `json:"avatar"` - AgentType string `json:"agent_type"` // "open" (default) or "predefined" - OwnerIDs []string `json:"owner_ids,omitempty"` // first entry used as DB owner_id; falls back to "system" - TenantID string `json:"tenant_id"` // required for cross-tenant callers; ignored otherwise + Name string `json:"name"` + Workspace string `json:"workspace"` + Emoji string `json:"emoji"` + Avatar string `json:"avatar"` + Provider string `json:"provider"` + Model string `json:"model"` + AgentType string `json:"agent_type"` // "open" (default) or "predefined" + OwnerIDs []string `json:"owner_ids,omitempty"` // first entry used as DB owner_id; falls back to "system" + TenantID string `json:"tenant_id"` // required for cross-tenant callers; ignored otherwise + ContextWindow int `json:"context_window"` + MaxToolIterations int `json:"max_tool_iterations"` + BudgetCents *int `json:"budget_monthly_cents"` // Per-agent config overrides ToolsConfig json.RawMessage `json:"tools_config,omitempty"` SubagentsConfig json.RawMessage `json:"subagents_config,omitempty"` @@ -101,15 +106,27 @@ func (m *AgentsMethods) handleCreate(ctx context.Context, client *gateway.Client tenantID = client.TenantID() } + provider := params.Provider + if provider == "" { + provider = m.cfg.Agents.Defaults.Provider + } + model := params.Model + if model == "" { + model = m.cfg.Agents.Defaults.Model + } + agentData := &store.AgentData{ AgentKey: agentID, DisplayName: params.Name, OwnerID: ownerID, TenantID: tenantID, AgentType: agentType, - Provider: m.cfg.Agents.Defaults.Provider, - Model: m.cfg.Agents.Defaults.Model, + Provider: provider, + Model: model, Workspace: ws, + ContextWindow: params.ContextWindow, + MaxToolIterations: params.MaxToolIterations, + BudgetMonthlyCents: params.BudgetCents, Status: store.AgentStatusActive, ToolsConfig: params.ToolsConfig, SubagentsConfig: params.SubagentsConfig, diff --git a/internal/gateway/methods/agents_update.go b/internal/gateway/methods/agents_update.go index 9dee9639..1ce4f36b 100644 --- a/internal/gateway/methods/agents_update.go +++ b/internal/gateway/methods/agents_update.go @@ -21,11 +21,18 @@ import ( func (m *AgentsMethods) handleUpdate(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) { locale := store.LocaleFromContext(ctx) var params struct { - AgentID string `json:"agentId"` - Name string `json:"name"` - Workspace string `json:"workspace"` - Model string `json:"model"` - Avatar string `json:"avatar"` + AgentID string `json:"agentId"` + Name string `json:"name"` + Workspace string `json:"workspace"` + Provider string `json:"provider"` + Model string `json:"model"` + Avatar string `json:"avatar"` + Status string `json:"status"` + Frontmatter string `json:"frontmatter"` + ContextWindow *int `json:"context_window"` + MaxToolIterations *int `json:"max_tool_iterations"` + IsDefault *bool `json:"is_default"` + BudgetCents *int `json:"budget_monthly_cents"` // Per-agent config overrides ToolsConfig json.RawMessage `json:"tools_config,omitempty"` SubagentsConfig json.RawMessage `json:"subagents_config,omitempty"` @@ -61,9 +68,30 @@ func (m *AgentsMethods) handleUpdate(ctx context.Context, client *gateway.Client updates["workspace"] = ws os.MkdirAll(ws, 0755) } + if params.Provider != "" { + updates["provider"] = params.Provider + } if params.Model != "" { updates["model"] = params.Model } + if params.Status != "" { + updates["status"] = params.Status + } + if params.Frontmatter != "" { + updates["frontmatter"] = params.Frontmatter + } + if params.ContextWindow != nil { + updates["context_window"] = *params.ContextWindow + } + if params.MaxToolIterations != nil { + updates["max_tool_iterations"] = *params.MaxToolIterations + } + if params.IsDefault != nil { + updates["is_default"] = *params.IsDefault + } + if params.BudgetCents != nil { + updates["budget_monthly_cents"] = *params.BudgetCents + } // Per-agent JSONB config overrides if len(params.ToolsConfig) > 0 { updates["tools_config"] = []byte(params.ToolsConfig) @@ -146,11 +174,23 @@ func (m *AgentsMethods) handleUpdate(ctx context.Context, client *gateway.Client spec.Workspace = config.ExpandHome(params.Workspace) os.MkdirAll(spec.Workspace, 0755) } + if params.Provider != "" { + spec.Provider = params.Provider + } if params.Model != "" { spec.Model = params.Model } + if params.ContextWindow != nil { + spec.ContextWindow = *params.ContextWindow + } + if params.MaxToolIterations != nil { + spec.MaxToolIterations = *params.MaxToolIterations + } if params.AgentID == "default" { + if params.Provider != "" { + m.cfg.Agents.Defaults.Provider = params.Provider + } if params.Model != "" { m.cfg.Agents.Defaults.Model = params.Model } diff --git a/internal/gateway/methods/channel_instances.go b/internal/gateway/methods/channel_instances.go index 7f50ce98..7872a1ba 100644 --- a/internal/gateway/methods/channel_instances.go +++ b/internal/gateway/methods/channel_instances.go @@ -14,6 +14,14 @@ import ( "github.com/nextlevelbuilder/goclaw/pkg/protocol" ) +// channelInstanceAllowed mirrors the HTTP allowlist in internal/http/validate.go. +var channelInstanceAllowed = map[string]bool{ + "channel_type": true, "credentials": true, "agent_id": true, + "enabled": true, "group_policy": true, "allow_from": true, + "metadata": true, "webhook_secret": true, "config": true, + "display_name": true, +} + // ChannelInstancesMethods handles channel instance CRUD via WebSocket RPC. type ChannelInstancesMethods struct { store store.ChannelInstanceStore @@ -162,12 +170,22 @@ func (m *ChannelInstancesMethods) handleUpdate(ctx context.Context, client *gate return } - var updates map[string]any - if err := json.Unmarshal(params.Updates, &updates); err != nil { + var raw map[string]any + if err := json.Unmarshal(params.Updates, &raw); err != nil { client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidUpdates))) return } + // Allowlist: only permit known channel instance columns (matches HTTP handler). + updates := make(map[string]any, len(raw)) + for k, v := range raw { + if channelInstanceAllowed[k] { + updates[k] = v + } else { + slog.Warn("security.filtered_unknown_field", "field", k, "handler", "channels.instances.update") + } + } + if err := m.store.Update(ctx, id, updates); err != nil { slog.Error("channels.instances.update", "error", err) client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInternal, i18n.T(locale, i18n.MsgFailedToUpdate, "instance", err.Error()))) diff --git a/internal/http/validate.go b/internal/http/validate.go index 7e45e909..a18c9e29 100644 --- a/internal/http/validate.go +++ b/internal/http/validate.go @@ -38,6 +38,7 @@ var agentAllowedFields = map[string]bool{ "workspace": true, "frontmatter": true, "compaction_config": true, "memory_config": true, "other_config": true, "tools_config": true, + "sandbox_config": true, "context_pruning": true, "is_default": true, "budget_monthly_cents": true, "subagents_config": true, }