Files
SYNITYandDangTinh311 3b8bc1bc4d feat(bitrix24): replace two MCP text inputs with a filtered dropdown (#1392)
* feat(bitrix24): replace two MCP text inputs with a filtered dropdown [B24:2794]

Bitrix24 channel creation used to demand two hand-typed strings —
mcp_server_name and mcp_base_url — plus zero indication of which MCP
servers can actually auto-onboard. Typos silently disabled provisioning
and admin had to know which servers implement /api/auto-onboard.

Ship a single dropdown backed by mcp_servers.require_user_credentials,
plus the machinery to make it work end-to-end.

Phase 1 — DB & store
* Promote require_user_credentials from settings JSONB to a top-level
  column on mcp_servers (PG migration 000089, SQLite migration v54).
* Backfill from existing settings blobs so no admin needs to re-tick.
* Add MCPServerData.RequireUserCredentials to the Go store layer, plumb
  through Create / Get / GetByName / List / Update on both stores,
  extend the export DTO, and add require_user_credentials to the HTTP
  allowlist.
* Bump RequiredSchemaVersion 87 -> 89 (jumping 88, which was on disk
  but not wired) and SchemaVersion 53 -> 54 with an
  idempotentColumnMigration guard.

Phase 2 — Bitrix24 channel factory
* Add MCPServerID (UUID string) to bitrix24 InstanceConfig, keep
  MCPServerName + MCPBaseURL as legacy fallback with a "deprecated"
  doc comment.
* Factory validation accepts either mcp_server_id alone or the legacy
  pair; half-config still fails fast.
* initMCPProvisioner prefers GetServer(id) and sources the base URL
  from mcp_servers.url when the id path is used. Legacy name path
  unchanged so pre-migration configs keep working.
* Log line now carries mcp_server_id + require_user_credentials so
  operators can eyeball the wiring.

Phase 3 — Frontend types & MCP form
* Add optional top-level require_user_credentials to MCPServerData /
  MCPServerInput in both ui/web and ui/desktop/frontend types.
* mcp-form-dialog reads the top-level flag first and falls back to
  settings.require_user_credentials so cached responses from
  pre-upgrade backends still render correctly.
* On submit send both the top-level flag AND the legacy settings
  entry so mid-rollout backends stay consistent.

Phase 4 — Bitrix24 channel form dropdown
* New mcp-select field type + MCPServerSelect component. Uses the
  shared useMCP() react-query cache and filters client-side to
  servers whose require_user_credentials is true (OR settings
  JSONB during the migration window).
* Explicit "None (disable MCP provisioning)" option so admins can
  clear the binding without editing config JSON.
* Legacy mcp_server_name / mcp_base_url text inputs kept in the
  Advanced panel, relabelled "(legacy)" with pointer help text.

Phase 5 — channel_instances.config backfill
* PG migration 000090 and SQLite migration v55 rewrite existing
  bitrix24 channel_instances.config to add mcp_server_id by
  resolving mcp_server_name against mcp_servers, tenant-scoped
  via agents.tenant_id (channel_instances doesn't carry tenant_id
  directly).
* Idempotent — only touches rows already carrying
  mcp_server_name that lack mcp_server_id. Legacy keys are left
  in place so provisioner.go can still fall back for unmigrated
  or future-created legacy configs.
* down.sql drops the mcp_server_id key. Provisioner immediately
  reverts to the legacy fallback path.

Tests
* provisioner_test.go: three new cases exercise the mcp_server_id
  path (invalid UUID string, valid UUID with missing row, valid
  UUID with a per-user row). fakeMCPStore gains a serversByID
  map and a real GetServer implementation.
* Existing legacy-config tests unchanged and still green.

Verification
* go build ./... && go build -tags sqliteonly ./...
* go vet ./internal/mcp/... ./internal/channels/bitrix24/...
    ./internal/store/... ./internal/http/...
* go test ./internal/mcp/... ./internal/channels/bitrix24/...  -> ok
* Live-tested against a local docker image on the goclaw-deploy
  postgres. Migrations 89 + 90 applied cleanly. Three existing
  bitrix24 channels (bitrix-sales / nguyen-dao-openline / tieu-vi)
  had their configs backfilled with the b24-syn-mcp UUID and the
  provisioner boots with require_user_credentials=true. UI dropdown
  correctly shows only b24-syn-mcp (the only server with the flag
  ticked) alongside a "None" clearer option.

Surface parity
* Gateway server: store + factory + provisioner + HTTP allowlist.
* API contract: adds require_user_credentials + mcp_server_id
  as optional fields on existing routes. No new endpoints.
* Web UI: MCP form + Bitrix24 channel form + shared types.
* CLI/runtime package: N/A because no CLI subcommand reads the
  mcp_server_id field.

* fix(bitrix24): derive auto-onboard base URL from mcp_servers.url origin [B24:2794]

The Phase 2 refactor swapped provisioner base-URL sourcing from the
legacy per-channel MCPBaseURL config field (which historically stored the
MCP server's ORIGIN, e.g. https://mcp.example.com) to mcp_servers.url,
which stores the JSON-RPC ENDPOINT the agent loop dials (e.g.
https://mcp.example.com/mcp). The two are semantically different but
share a single column.

mcp_client.newMCPClient then appends "/api/auto-onboard" to whatever
baseURL it receives, so the id-path started POSTing to
".../mcp/api/auto-onboard" — 404 for every per-user credential mint and
refresh. Existing users kept working only until their cached access
tokens expired.

Fix: derive the origin (scheme://host[:port]) from server.URL before
handing it to the auto-onboard client. The legacy path is untouched
because MCPBaseURL from channel config is already the origin.

  https://b24-mcp-dev.synity.so/mcp   ->  https://b24-mcp-dev.synity.so
  https://mcp.example.com/mcp/        ->  https://mcp.example.com
  https://mcp.example.com             ->  https://mcp.example.com

Table-driven test covers six shapes plus four error cases (empty,
whitespace-only, no scheme, no host). Updated the existing
TestInitMCPProvisioner_MCPServerID fixture to seed a URL with the /mcp
subpath so it regression-guards the same code path.

Verified live: user 614 sent a message that triggered the expired-cred
refresh branch; goclaw logged "self-refreshed user credentials
created=false" and the agent immediately reported
"mcp.user_tools_loaded user=614 tools=2". Before this fix the same event
logged 'auto-onboard failed: mcp auto-onboard: 404 Not Found'.

Surface parity:
  - Gateway server: provisioner + one new helper (deriveAutoOnboardBaseURL).
  - API contract: N/A because the wire shape hasn't changed.
  - Web UI: N/A because the UI still writes mcp_server_id verbatim.
  - CLI/runtime: N/A because no CLI reads the derived base URL.

---------

Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
2026-07-08 19:23:21 +07:00

87 lines
3.3 KiB
Go

package http
import (
"log/slog"
"regexp"
"github.com/nextlevelbuilder/goclaw/internal/audio"
)
var slugRe = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`)
// isValidSlug checks whether s matches the slug format: lowercase alphanumeric + hyphens,
// cannot start or end with a hyphen.
func isValidSlug(s string) bool {
return slugRe.MatchString(s)
}
// filterAllowedKeys returns a new map containing only keys present in the allowlist.
// Defense-in-depth: prevents column injection and unauthorized field updates.
func filterAllowedKeys(updates map[string]any, allowed map[string]bool) map[string]any {
filtered := make(map[string]any, len(updates))
for k, v := range updates {
if allowed[k] {
filtered[k] = v
} else {
slog.Warn("security.filtered_unknown_field", "field", k)
}
}
return filtered
}
// validateAgentTTSParams is a thin wrapper around audio.ValidateAgentTTSParams
// so HTTP handlers can call it without importing the audio package directly.
// The allow-list is owned by internal/audio (single source of truth, Action D).
func validateAgentTTSParams(ttsParams map[string]any) error {
return audio.ValidateAgentTTSParams(ttsParams)
}
// --- Field allowlists for update endpoints ---
// Each map lists the columns that HTTP clients may update.
// Immutable fields (id, owner_id, created_at, deleted_at) are excluded.
var agentAllowedFields = map[string]bool{
"agent_key": true, "agent_type": true, "display_name": true,
"provider": true, "model": true, "status": true,
"context_window": true, "max_tool_iterations": true,
"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,
// Promoted from other_config
"emoji": true, "agent_description": true, "thinking_level": true, "max_tokens": true,
"self_evolve": true, "skill_evolve": true, "skill_nudge_interval": true,
"reasoning_config": true, "workspace_sharing": true, "chatgpt_oauth_routing": true,
"model_fallback": true, "shell_deny_groups": true, "kg_dedup_config": true,
}
var providerAllowedFields = map[string]bool{
"name": true, "provider_type": true, "api_key": true,
"api_base": true, "base_url": true, "default_model": true,
"extra_headers": true, "config": true, "enabled": true,
"display_name": true, "display_order": true, "settings": true,
}
var customToolAllowedFields = map[string]bool{
"name": true, "description": true, "command": true,
"parameters": true, "agent_id": true, "env": true,
"tags": true, "requires": true, "timeout_seconds": true,
"enabled": true,
}
var mcpServerAllowedFields = map[string]bool{
"name": true, "display_name": true, "transport": true, "command": true, "args": true,
"url": true, "api_key": true, "env": true, "headers": true,
"enabled": true, "tool_prefix": true, "timeout_sec": true,
"require_user_credentials": true,
"agent_id": true, "config": true, "settings": true,
}
var channelInstanceAllowedFields = map[string]bool{
"name": true, "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,
}