Fix: preserve MCP connection settings across import and edit (#19518)

Fix MCP settings lost on import/export/edit (headers, substitution variables, auth scheme).
This commit is contained in:
George Pickett authored and GitHub committed 2026-09-20 16:25:32 +08:00
1 parent 1e5fc587f0
commit 163ab2e032
15 files changed
+965 -164

No files matched your search

+21 -4
View File
@@ -44,10 +44,12 @@ def _export_mcp_servers(mcp_ids: list[str]) -> dict | None:
if e and mcp_server.tenant_id == current_user.id:
server_key = mcp_server.name
exported_servers[server_key] = {
**mcp_server.variables,
"type": mcp_server.server_type,
"url": mcp_server.url,
"name": mcp_server.name,
"name": mcp_server.variables.get("name", mcp_server.name),
"authorization_token": mcp_server.variables.get("authorization_token", ""),
"headers": mcp_server.headers or {},
"tools": mcp_server.variables.get("tools", {}),
}
@@ -279,6 +281,21 @@ async def import_multiple() -> Response:
results.append({"server": server_name, "success": False, "message": url_error})
continue
headers = config.get("headers", {})
if not isinstance(headers, dict) or not all(isinstance(k, str) and isinstance(v, str) for k, v in headers.items()):
results.append({"server": server_name, "success": False, "message": "MCP headers must be a string-to-string object."})
continue
headers = dict(headers)
if "authorization_token" in config:
if not isinstance(config["authorization_token"], str):
results.append({"server": server_name, "success": False, "message": "authorization_token must be a string."})
continue
# Explicit headers are the complete request configuration, even when empty.
if "headers" not in config:
headers["authorization_token"] = config["authorization_token"]
variables = {k: v for k, v in config.items() if k not in {"type", "url", "headers"}}
variables.setdefault("authorization_token", "")
base_name = server_name
new_name = base_name
counter = 0
@@ -296,11 +313,11 @@ async def import_multiple() -> Response:
"name": new_name,
"url": config["url"],
"server_type": config["type"],
"variables": {"authorization_token": config.get("authorization_token", "")},
# Use the same headers for discovery and later agent calls.
"headers": headers,
"variables": variables,
}
headers = {"authorization_token": config["authorization_token"]} if "authorization_token" in config else {}
variables = {k: v for k, v in config.items() if k not in {"type", "url", "headers"}}
mcp_server = MCPServer(id=new_name, name=new_name, url=config["url"], server_type=config["type"], variables=variables, headers=headers)
with pin_dns_global(hostname, resolved_ip):
server_tools, err_message = await thread_pool_exec(get_mcp_tools, [mcp_server], timeout)
@@ -0,0 +1,48 @@
---
sidebar_position: 4
title: Connect external MCP tools
sidebar_label: Connect external MCP tools
slug: /connect_external_mcp_servers
sidebar_custom_props: {
categoryIcon: LucideBookMarked
}
---
# Connect external MCP tools
RAGFlow agents can use an external MCP server alongside their existing tools. This is different from [running RAGFlow's MCP server](./launch_mcp_server.md), which exposes your RAGFlow resources to other clients.
## Add web research with Parallel
[Parallel Search MCP](https://docs.parallel.ai/integrations/mcp/search-mcp) provides `web_search` for finding web sources and `web_fetch` for reading known public URLs. Its anonymous endpoint does not require a Parallel account or API key. Free access is rate limited.
1. Save the [example configuration](https://github.com/infiniflow/ragflow/blob/main/example/mcp/parallel_search.json) as a JSON file:
```json
{
"mcpServers": {
"parallel-search": {
"type": "streamable-http",
"url": "https://search.parallel.ai/mcp",
"headers": {
"User-Agent": "ragflow"
}
}
}
}
```
2. Open **Settings → MCP** and import the file. RAGFlow connects to the server and discovers its tools. Use **Streamable HTTP**, not SSE, if entering the connection manually. Leave the Parallel authorization token empty.
3. In an **Agent** component, add the MCP tools from `parallel-search` and enable `web_search` and `web_fetch` as needed. Importing the server does not replace an existing search tool or make Parallel the default.
4. Try a public-web question such as: “Find RAGFlow's current release notes and summarize the latest changes with source links.” The agent can search, then fetch a relevant result. No private dataset is needed for this example.
The `User-Agent` identifies RAGFlow so Parallel can measure aggregate usage by project. Keep it project-wide; do not add a user, installation or device identifier. Imported headers are used during discovery and subsequent tool calls and are retained when exporting the server configuration.
When enabled, the agent may call these tools during its normal workflow. Supplied search queries, requested URLs, objectives/context and request metadata go to Parallel. Review its [privacy policy](https://parallel.ai/privacy-policy) before sending sensitive content. Anonymous Parallel access does not remove authentication required by your RAGFlow installation.
## Check a connection
Confirm that tool discovery returns `web_search` and `web_fetch`. A successful connection alone does not prove that the agent selected a tool; inspect the agent run's tool calls and returned source links.
For connection errors, check the transport, outbound access to `search.parallel.ai`, and the configured headers. The free example deliberately omits authentication headers. If you configured an authenticated connection, investigate its credentials instead of silently removing them. Respect rate-limit responses and avoid repeated immediate retries.
For other MCP servers, use their documented URL, transport and authentication. Custom `headers` must be an object with string names and string values. Never commit real credentials in an exported configuration.
+2
View File
@@ -11,6 +11,8 @@ sidebar_custom_props: {
Python and curl MCP client examples.
To use third-party tools inside a RAGFlow agent, see [Connect external MCP tools](./connect_external_mcp_servers.md).
------
## Example MCP Python Client
+11
View File
@@ -0,0 +1,11 @@
{
"mcpServers": {
"parallel-search": {
"type": "streamable-http",
"url": "https://search.parallel.ai/mcp",
"headers": {
"User-Agent": "ragflow"
}
}
}
}
+109 -63
View File
@@ -94,18 +94,6 @@ type MCPServerListItem struct {
UpdateDate *string `json:"update_date"`
}
type ExportMCPServer struct {
Type string `json:"type"`
URL string `json:"url"`
Name string `json:"name"`
AuthorizationToken interface{} `json:"authorization_token"`
Tools interface{} `json:"tools"`
}
type ExportMCPServerResponse struct {
MCPServers map[string]ExportMCPServer `json:"mcpServers"`
}
// ListMCPServersResponse is the response payload for listing MCP servers.
type ListMCPServersResponse struct {
MCPServers []*MCPServerListItem `json:"mcp_servers"`
@@ -211,7 +199,7 @@ func fetchMCPTools(ctx context.Context, url, serverType string, headers, variabl
tools, err := utility.FetchTools(ctx, utility.FetchOptions{
URL: url,
ServerType: serverType,
Headers: jsonMapStringValues(headers),
Headers: jsonMapHeaderValues(headers),
Variables: jsonMapStringValues(variables),
Timeout: timeout,
})
@@ -222,6 +210,16 @@ func fetchMCPTools(ctx context.Context, url, serverType string, headers, variabl
}
func jsonMapStringValues(values entity.JSONMap) map[string]string {
out := map[string]string{}
for key, value := range values {
if text, ok := jsonValueString(value); ok {
out[key] = text
}
}
return out
}
func jsonMapHeaderValues(values entity.JSONMap) map[string]string {
out := map[string]string{}
for key, value := range values {
if text, ok := value.(string); ok {
@@ -231,6 +229,84 @@ func jsonMapStringValues(values entity.JSONMap) map[string]string {
return out
}
// jsonValueString renders the scalar JSON values that can occur in an MCP
// variable map. Python's string.Template.safe_substitute stringifies these
// values before replacing a placeholder; the original values remain in the
// JSON map for persistence. Compound values are left unavailable because
// there is no stable header representation for them.
func jsonValueString(value interface{}) (string, bool) {
switch value := value.(type) {
case string:
return value, true
case json.Number:
return value.String(), true
case bool:
if value {
return "True", true
}
return "False", true
case nil:
return "None", true
case float64:
return strconv.FormatFloat(value, 'f', -1, 64), true
case float32:
return strconv.FormatFloat(float64(value), 'f', -1, 32), true
case int:
return strconv.Itoa(value), true
case int8:
return strconv.FormatInt(int64(value), 10), true
case int16:
return strconv.FormatInt(int64(value), 10), true
case int32:
return strconv.FormatInt(int64(value), 10), true
case int64:
return strconv.FormatInt(value, 10), true
case uint:
return strconv.FormatUint(uint64(value), 10), true
case uint8:
return strconv.FormatUint(uint64(value), 10), true
case uint16:
return strconv.FormatUint(uint64(value), 10), true
case uint32:
return strconv.FormatUint(uint64(value), 10), true
case uint64:
return strconv.FormatUint(value, 10), true
default:
return "", false
}
}
// importedMCPHeaders validates and copies the header object from an imported
// config. Keeping a separate copy means adding the legacy token fallback
// cannot mutate the caller's decoded configuration, and a present empty map
// remains distinguishable from an omitted headers field.
func importedMCPHeaders(raw interface{}) (map[string]string, map[string]interface{}, bool) {
headers := map[string]string{}
values := map[string]interface{}{}
copyHeader := func(key string, value interface{}) bool {
text, ok := value.(string)
if !ok {
return false
}
headers[key] = text
values[key] = value
return true
}
switch raw := raw.(type) {
case map[string]interface{}:
for key, value := range raw {
if !copyHeader(key, value) {
return nil, nil, false
}
}
default:
return nil, nil, false
}
return headers, values, true
}
func (s *MCPService) GetMCPServer(ctx context.Context, tenantID, mcpID string) (*entity.MCPServer, common.ErrorCode, error) {
server, err := s.mcpServerDAO.GetByIDAndTenant(ctx, dao.DB, mcpID, tenantID)
if err != nil {
@@ -253,33 +329,6 @@ func (s *MCPService) ExportMCPServer(ctx context.Context, userID, mcpID string)
return newExportMCPServerResponse(server), common.CodeSuccess, nil
}
func newExportMCPServerResponse(server *entity.MCPServer) *ExportMCPServerResponse {
vars := server.Variables
if vars == nil {
vars = entity.JSONMap{}
}
token := interface{}("")
if value, ok := vars["authorization_token"]; ok {
token = value
}
tools := vars["tools"]
if tools == nil {
tools = map[string]interface{}{}
}
return &ExportMCPServerResponse{
MCPServers: map[string]ExportMCPServer{
server.Name: {
Type: server.ServerType,
URL: server.URL,
Name: server.Name,
AuthorizationToken: token,
Tools: tools,
},
},
}
}
// UpdateMCPServer updates an MCP server owned by a tenant.
func (s *MCPService) UpdateMCPServer(ctx context.Context, tenantID, mcpID string, req UpdateMCPServerRequest) (*entity.MCPServer, common.ErrorCode, error) {
server, err := s.mcpServerDAO.GetByIDAndTenant(ctx, dao.DB, mcpID, tenantID)
@@ -595,18 +644,13 @@ func (s *MCPService) ImportServers(ctx context.Context, tenantID string, servers
}
variables := map[string]interface{}{}
stringVars := map[string]string{}
for k, v := range config {
if k == "type" || k == "url" || k == "headers" {
continue
}
variables[k] = v
if sv, ok := v.(string); ok {
stringVars[k] = sv
}
}
delete(variables, "tools")
delete(stringVars, "tools")
// Headers can be provided either as a top-level "headers" map
// (preferred — matches the Python import shape) or as a flat
@@ -616,25 +660,32 @@ func (s *MCPService) ImportServers(ctx context.Context, tenantID string, servers
// trip.
headers := map[string]string{}
headerVals := map[string]interface{}{}
if rawHeaders, ok := config["headers"].(map[string]interface{}); ok {
for k, v := range rawHeaders {
if sv, ok := v.(string); ok {
headers[k] = sv
}
headerVals[k] = v
rawHeaders, headersProvided := config["headers"]
if headersProvided {
var ok bool
headers, headerVals, ok = importedMCPHeaders(rawHeaders)
if !ok {
results = append(results, ImportResult{Server: baseName, Success: false, Message: "MCP headers must be a string-to-string object."})
continue
}
}
if token, exists := config["authorization_token"]; exists {
if _, ok := token.(string); !ok {
results = append(results, ImportResult{Server: baseName, Success: false, Message: "authorization_token must be a string."})
continue
}
}
if token, ok := config["authorization_token"].(string); ok && strings.TrimSpace(token) != "" {
variables["authorization_token"] = token
stringVars["authorization_token"] = token
if _, exists := headers["Authorization"]; !exists {
// An explicit header map is the complete request configuration,
// including {}. Retain the token as a variable for explicit
// placeholders, but only add the legacy fallback header when the
// import omitted headers altogether.
if !headersProvided {
headers["Authorization"] = "Bearer ${authorization_token}"
}
if _, exists := headerVals["Authorization"]; !exists {
headerVals["Authorization"] = "Bearer ${authorization_token}"
}
}
stringVars := jsonMapStringValues(entity.JSONMap(variables))
mcpCtx, cancel := context.WithTimeout(ctx, timeout)
tools, fetchErr := utility.FetchTools(mcpCtx, utility.FetchOptions{
@@ -732,12 +783,7 @@ func (s *MCPService) TestServer(ctx context.Context, mcpID string, req *TestServ
headers[k] = sv
}
}
vars := map[string]string{}
for k, v := range req.Variables {
if sv, ok := v.(string); ok {
vars[k] = sv
}
}
vars := jsonMapStringValues(entity.JSONMap(req.Variables))
mcpCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
+87
View File
@@ -0,0 +1,87 @@
//
// Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
package service
import (
"encoding/json"
"ragflow/internal/entity"
)
type ExportMCPServer struct {
Type string `json:"type"`
URL string `json:"url"`
Name string `json:"name"`
AuthorizationToken interface{} `json:"authorization_token"`
Tools interface{} `json:"tools"`
Headers entity.JSONMap `json:"headers"`
variables entity.JSONMap
}
// The import format keeps substitution variables alongside the connection
// fields. Preserve them without changing the saved server during export.
func (server ExportMCPServer) MarshalJSON() ([]byte, error) {
config := make(map[string]interface{}, len(server.variables)+6)
for key, value := range server.variables {
config[key] = value
}
config["type"] = server.Type
config["url"] = server.URL
if _, exists := config["name"]; !exists {
config["name"] = server.Name
}
config["authorization_token"] = server.AuthorizationToken
config["tools"] = server.Tools
headers := server.Headers
if headers == nil {
headers = entity.JSONMap{}
}
config["headers"] = headers
return json.Marshal(config)
}
type ExportMCPServerResponse struct {
MCPServers map[string]ExportMCPServer `json:"mcpServers"`
}
func newExportMCPServerResponse(server *entity.MCPServer) *ExportMCPServerResponse {
vars := server.Variables
if vars == nil {
vars = entity.JSONMap{}
}
token := interface{}("")
if value, ok := vars["authorization_token"]; ok {
token = value
}
tools := vars["tools"]
if tools == nil {
tools = map[string]interface{}{}
}
return &ExportMCPServerResponse{
MCPServers: map[string]ExportMCPServer{
server.Name: {
Type: server.ServerType,
URL: server.URL,
Name: server.Name,
AuthorizationToken: token,
Tools: tools,
Headers: server.Headers,
variables: vars,
},
},
}
}
+104
View File
@@ -0,0 +1,104 @@
//
// Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
package service
import (
"encoding/json"
"ragflow/internal/entity"
"testing"
)
func TestNewExportMCPServerResponseMatchesPythonDownloadShape(t *testing.T) {
response := newExportMCPServerResponse(&entity.MCPServer{
Name: "weather",
URL: "https://example.com/mcp",
ServerType: "streamable-http",
Variables: entity.JSONMap{
"authorization_token": "secret-token",
"tools": map[string]interface{}{
"forecast": map[string]interface{}{"name": "forecast"},
},
},
})
payload, err := json.Marshal(response)
if err != nil {
t.Fatalf("marshal response: %v", err)
}
var decoded map[string]map[string]map[string]interface{}
if err := json.Unmarshal(payload, &decoded); err != nil {
t.Fatalf("unmarshal response: %v", err)
}
server := decoded["mcpServers"]["weather"]
if server["type"] != "streamable-http" {
t.Fatalf("type = %v, want %s", server["type"], "streamable-http")
}
if server["url"] != "https://example.com/mcp" {
t.Fatalf("url = %v", server["url"])
}
if server["name"] != "weather" {
t.Fatalf("name = %v", server["name"])
}
if server["authorization_token"] != "secret-token" {
t.Fatalf("authorization_token = %v", server["authorization_token"])
}
tools, ok := server["tools"].(map[string]interface{})
if !ok || tools["forecast"] == nil {
t.Fatalf("tools = %#v, want forecast tool", server["tools"])
}
}
func TestNewExportMCPServerResponseDefaultsMissingVariablesLikePython(t *testing.T) {
response := newExportMCPServerResponse(&entity.MCPServer{
Name: "empty-vars",
URL: "https://example.com/mcp",
ServerType: "sse",
})
server := response.MCPServers["empty-vars"]
if server.AuthorizationToken != "" {
t.Fatalf("authorization_token = %#v, want empty string", server.AuthorizationToken)
}
tools, ok := server.Tools.(map[string]interface{})
if !ok {
t.Fatalf("tools type = %T, want map[string]interface{}", server.Tools)
}
if len(tools) != 0 {
t.Fatalf("tools = %#v, want empty map", tools)
}
}
func TestExportPreservesCustomHeadersAndNameVariable(t *testing.T) {
input := &entity.MCPServer{Name: "server-label", URL: "https://example.com/mcp", ServerType: "streamable-http", Headers: entity.JSONMap{"User-Agent": "ragflow", "X-Token": "${custom}", "X-Name": "${name}"}, Variables: entity.JSONMap{"custom": "secret", "name": "header-identity"}}
payload, err := json.Marshal(newExportMCPServerResponse(input))
if err != nil {
t.Fatal(err)
}
var decoded map[string]map[string]map[string]interface{}
if err = json.Unmarshal(payload, &decoded); err != nil {
t.Fatal(err)
}
entry := decoded["mcpServers"]["server-label"]
if entry["headers"] == nil || entry["custom"] != "secret" || entry["name"] != "header-identity" {
t.Fatalf("lost connection configuration: %#v", entry)
}
if input.Variables["name"] != "header-identity" {
t.Fatal("export mutated the source")
}
}
+220 -49
View File
@@ -20,11 +20,17 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"
"time"
"ragflow/internal/common"
"ragflow/internal/entity"
"ragflow/internal/utility"
)
func TestIsValidMCPServerType(t *testing.T) {
@@ -138,66 +144,231 @@ func TestImportServersValidationErrors(t *testing.T) {
}
}
func TestNewExportMCPServerResponseMatchesPythonDownloadShape(t *testing.T) {
response := newExportMCPServerResponse(&entity.MCPServer{
Name: "weather",
URL: "https://example.com/mcp",
ServerType: mcpServerTypeStreamableHTTP,
Variables: entity.JSONMap{
"authorization_token": "secret-token",
"tools": map[string]interface{}{
"forecast": map[string]interface{}{"name": "forecast"},
},
},
})
payload, err := json.Marshal(response)
if err != nil {
t.Fatalf("marshal response: %v", err)
func TestJSONMapStringValuesRendersScalarVariables(t *testing.T) {
values := entity.JSONMap{
"text": "value",
"count": float64(123),
"ratio": float64(1.5),
"enabled": true,
"missing": nil,
"object": map[string]interface{}{"key": "value"},
}
var decoded map[string]map[string]map[string]interface{}
if err := json.Unmarshal(payload, &decoded); err != nil {
t.Fatalf("unmarshal response: %v", err)
got := jsonMapStringValues(values)
for key, want := range map[string]string{
"text": "value",
"count": "123",
"ratio": "1.5",
"enabled": "True",
"missing": "None",
} {
if got[key] != want {
t.Errorf("jsonMapStringValues[%q]=%q, want %q", key, got[key], want)
}
}
server := decoded["mcpServers"]["weather"]
if server["type"] != mcpServerTypeStreamableHTTP {
t.Fatalf("type = %v, want %s", server["type"], mcpServerTypeStreamableHTTP)
if _, ok := got["object"]; ok {
t.Errorf("compound variable should not be rendered as a header value: %#v", got["object"])
}
if server["url"] != "https://example.com/mcp" {
t.Fatalf("url = %v", server["url"])
}
if server["name"] != "weather" {
t.Fatalf("name = %v", server["name"])
}
if server["authorization_token"] != "secret-token" {
t.Fatalf("authorization_token = %v", server["authorization_token"])
}
tools, ok := server["tools"].(map[string]interface{})
if !ok || tools["forecast"] == nil {
t.Fatalf("tools = %#v, want forecast tool", server["tools"])
if headerValues := jsonMapHeaderValues(values); len(headerValues) != 1 || headerValues["text"] != "value" {
t.Errorf("jsonMapHeaderValues = %#v, want string headers only", headerValues)
}
}
func TestNewExportMCPServerResponseDefaultsMissingVariablesLikePython(t *testing.T) {
response := newExportMCPServerResponse(&entity.MCPServer{
Name: "empty-vars",
URL: "https://example.com/mcp",
ServerType: mcpServerTypeSSE,
})
func TestImportServersPreservesExplicitHeadersAndVariables(t *testing.T) {
for _, tc := range []struct {
name string
headers map[string]interface{}
wantHeaderValues map[string]string
}{
{
name: "custom headers",
headers: map[string]interface{}{
"User-Agent": "ragflow",
"X-Token": "Bearer ${authorization_token}",
"X-Count": "count=$count",
},
wantHeaderValues: map[string]string{
"User-Agent": "ragflow",
"X-Token": "Bearer secret-token",
"X-Count": "count=123",
},
},
{
name: "explicit empty headers",
headers: map[string]interface{}{},
wantHeaderValues: map[string]string{},
},
} {
t.Run(tc.name, func(t *testing.T) {
testDB := setupServiceTestDB(t)
if err := testDB.AutoMigrate(&entity.MCPServer{}); err != nil {
t.Fatalf("migrate mcp server: %v", err)
}
pushServiceDB(t, testDB)
server := response.MCPServers["empty-vars"]
if server.AuthorizationToken != "" {
t.Fatalf("authorization_token = %#v, want empty string", server.AuthorizationToken)
srv, captured := newMCPDiscoveryTestServer(t)
defer srv.Close()
withMCPDiscoveryOverrides(t, srv)
results, err := NewMCPService().ImportServers(t.Context(), "tenant-1", map[string]map[string]interface{}{
"parallel": {
"type": mcpServerTypeStreamableHTTP,
"url": srv.URL,
"authorization_token": "secret-token",
"count": float64(123),
"headers": tc.headers,
},
}, 2)
if err != nil {
t.Fatalf("ImportServers: %v", err)
}
if len(results) != 1 || !results[0].Success {
t.Fatalf("ImportServers results = %#v", results)
}
for _, headers := range captured() {
if got := headers.Get("Authorization"); got != "" {
t.Errorf("unexpected generated Authorization header %q", got)
}
for key, want := range tc.wantHeaderValues {
if got := headers.Get(key); got != want {
t.Errorf("%s=%q, want %q", key, got, want)
}
}
}
var stored entity.MCPServer
if err := testDB.Where("name = ?", "parallel").First(&stored).Error; err != nil {
t.Fatalf("load imported server: %v", err)
}
if len(stored.Headers) != len(tc.headers) {
t.Fatalf("stored headers = %#v, want %#v", stored.Headers, tc.headers)
}
for key, want := range tc.headers {
if got := stored.Headers[key]; got != want {
t.Errorf("stored headers[%q]=%#v, want %#v", key, got, want)
}
}
if got, ok := stored.Variables["authorization_token"].(string); !ok || got != "secret-token" {
t.Errorf("stored authorization_token = %#v, want original token", stored.Variables["authorization_token"])
}
if got, ok := stored.Variables["count"].(float64); !ok || got != 123 {
t.Errorf("stored count = %#v, want float64(123)", stored.Variables["count"])
}
// Export the saved record, decode the wire JSON, and import that
// configuration again. This is the round-trip that must not grow
// an implicit authentication header when headers is present.
exported, err := json.Marshal(newExportMCPServerResponse(&stored))
if err != nil {
t.Fatalf("marshal export: %v", err)
}
var envelope struct {
MCPServers map[string]map[string]interface{} `json:"mcpServers"`
}
if err := json.Unmarshal(exported, &envelope); err != nil {
t.Fatalf("decode export: %v", err)
}
reimportConfig := envelope.MCPServers["parallel"]
if reimportConfig == nil {
t.Fatalf("export did not contain parallel config: %s", exported)
}
capturedBefore := len(captured())
reimported, err := NewMCPService().ImportServers(t.Context(), "tenant-1", map[string]map[string]interface{}{
"parallel": reimportConfig,
}, 2)
if err != nil {
t.Fatalf("reimport exported config: %v", err)
}
if len(reimported) != 1 || !reimported[0].Success {
t.Fatalf("reimport results = %#v", reimported)
}
for _, headers := range captured()[capturedBefore:] {
if got := headers.Get("Authorization"); got != "" {
t.Errorf("reimport generated Authorization header %q", got)
}
for key, want := range tc.wantHeaderValues {
if got := headers.Get(key); got != want {
t.Errorf("reimport %s=%q, want %q", key, got, want)
}
}
}
var roundTripped entity.MCPServer
if err := testDB.Where("name = ?", "parallel_0").First(&roundTripped).Error; err != nil {
t.Fatalf("load reimported server: %v", err)
}
if len(roundTripped.Headers) != len(tc.headers) {
t.Fatalf("reimported headers = %#v, want %#v", roundTripped.Headers, tc.headers)
}
})
}
tools, ok := server.Tools.(map[string]interface{})
if !ok {
t.Fatalf("tools type = %T, want map[string]interface{}", server.Tools)
}
func newMCPDiscoveryTestServer(t *testing.T) (*httptest.Server, func() []http.Header) {
t.Helper()
var mu sync.Mutex
var captured []http.Header
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
captured = append(captured, r.Header.Clone())
mu.Unlock()
if r.Method == http.MethodDelete {
w.WriteHeader(http.StatusNoContent)
return
}
body, err := io.ReadAll(r.Body)
if err != nil {
t.Errorf("read MCP request: %v", err)
w.WriteHeader(http.StatusBadRequest)
return
}
var req struct {
ID interface{} `json:"id"`
Method string `json:"method"`
}
if err := json.Unmarshal(body, &req); err != nil {
t.Errorf("decode MCP request: %v", err)
w.WriteHeader(http.StatusBadRequest)
return
}
w.Header().Set("Content-Type", "application/json")
switch req.Method {
case "initialize":
w.Header().Set("Mcp-Session-Id", "test-session")
_, _ = fmt.Fprintf(w, `{"jsonrpc":"2.0","id":%v,"result":{"capabilities":{}}}`, req.ID)
case "notifications/initialized":
w.WriteHeader(http.StatusAccepted)
case "tools/list":
_, _ = fmt.Fprintf(w, `{"jsonrpc":"2.0","id":%v,"result":{"tools":[{"name":"search"}]}}`, req.ID)
default:
t.Errorf("unexpected MCP method %q", req.Method)
w.WriteHeader(http.StatusBadRequest)
}
}))
return server, func() []http.Header {
mu.Lock()
defer mu.Unlock()
out := make([]http.Header, len(captured))
copy(out, captured)
return out
}
if len(tools) != 0 {
t.Fatalf("tools = %#v, want empty map", tools)
}
func withMCPDiscoveryOverrides(t *testing.T, server *httptest.Server) {
t.Helper()
originalAssert := utility.AssertURLSafe
originalPinned := utility.PinnedHTTPClient
utility.AssertURLSafe = func(string) (string, string, error) {
return "mcp.test", "127.0.0.1", nil
}
utility.PinnedHTTPClient = func(string, string, time.Duration) *http.Client {
return server.Client()
}
t.Cleanup(func() {
utility.AssertURLSafe = originalAssert
utility.PinnedHTTPClient = originalPinned
})
}
func TestPaginateMCPServersNegativeValuesMatchPythonSlice(t *testing.T) {
+64 -25
View File
@@ -121,9 +121,8 @@ func FetchTools(ctx context.Context, opts FetchOptions) ([]Tool, error) {
}
}
// renderHeaders applies ${name} substitution to header keys and values using
// the supplied variables map, mirroring the Template.safe_substitute pass in
// common/mcp_tool_call_conn.py. Empty keys (after substitution) are dropped.
// renderHeaders applies Python string.Template.safe_substitute semantics to
// header keys and values. Empty keys (after substitution) are dropped.
func renderHeaders(raw map[string]string, vars map[string]string) (map[string]string, error) {
rendered := map[string]string{}
for k, v := range raw {
@@ -137,42 +136,82 @@ func renderHeaders(raw map[string]string, vars map[string]string) (map[string]st
return rendered, nil
}
// substituteTemplate replaces ${name} occurrences (Python string.Template
// safe-substitute semantics) with values from vars. Unknown keys are left
// in place, matching safe_substitute's behavior.
// substituteTemplate mirrors Python's string.Template.safe_substitute for the
// identifier forms used by MCP configurations: $name, ${name}, and $$. Values
// are written directly into the result, so a placeholder in a value is not
// expanded recursively. Unknown or malformed placeholders are preserved.
func substituteTemplate(s string, vars map[string]string) string {
if vars == nil || !strings.Contains(s, "${") {
if !strings.Contains(s, "$") {
return s
}
var b strings.Builder
i := 0
for i < len(s) {
idx := strings.Index(s[i:], "${")
if idx == -1 {
b.WriteString(s[i:])
break
for i := 0; i < len(s); {
if s[i] != '$' {
b.WriteByte(s[i])
i++
continue
}
b.WriteString(s[i : i+idx])
i += idx + 2
end := strings.Index(s[i:], "}")
if end == -1 {
b.WriteString("${")
b.WriteString(s[i:])
break
// A trailing dollar and a dollar followed by a non-template token are
// literal dollars under safe_substitute.
if i+1 >= len(s) {
b.WriteByte('$')
i++
continue
}
key := s[i : i+end]
i += end + 1
if s[i+1] == '$' {
b.WriteByte('$')
i += 2
continue
}
start := i
nameStart := i + 1
if s[nameStart] == '{' {
nameStart++
}
if nameStart >= len(s) || !templateIdentifierStart(s[nameStart]) {
b.WriteByte('$')
i++
continue
}
nameEnd := nameStart + 1
for nameEnd < len(s) && templateIdentifierContinue(s[nameEnd]) {
nameEnd++
}
end := nameEnd
if s[i+1] == '{' {
if nameEnd >= len(s) || s[nameEnd] != '}' {
// Invalid braced placeholders are emitted literally. Advance
// only past the dollar; any later dollar tokens still follow
// the normal safe_substitute scan.
b.WriteByte('$')
i++
continue
}
end++
}
key := s[nameStart:nameEnd]
if val, ok := vars[key]; ok {
b.WriteString(val)
} else {
b.WriteString("${")
b.WriteString(key)
b.WriteString("}")
b.WriteString(s[start:end])
}
i = end
}
return b.String()
}
func templateIdentifierStart(c byte) bool {
return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')
}
func templateIdentifierContinue(c byte) bool {
return templateIdentifierStart(c) || (c >= '0' && c <= '9')
}
// jsonRPCRequest is a JSON-RPC 2.0 request envelope.
type jsonRPCRequest struct {
JSONRPC string `json:"jsonrpc"`
+26 -13
View File
@@ -75,6 +75,9 @@ func TestFetchToolsStreamableHTTPJSON(t *testing.T) {
if got := r.Header.Get("Authorization"); got != "Bearer fetch-token" {
t.Errorf("Authorization=%q, want rendered header", got)
}
if got := r.Header.Get("X-Template"); got != "cost $5" {
t.Errorf("X-Template=%q, want safe-substituted header", got)
}
if r.Method == http.MethodDelete {
atomic.AddInt32(&deleteCount, 1)
recordRequest(http.MethodDelete)
@@ -131,7 +134,8 @@ func TestFetchToolsStreamableHTTPJSON(t *testing.T) {
URL: srv.URL,
ServerType: TransportStreamableHTTP,
Headers: map[string]string{
"${header_name}": "Bearer ${token}",
"${header_name}": "Bearer $token",
"X-Template": "cost $$5",
sessionHeader: "stale-session",
},
Variables: map[string]string{
@@ -281,18 +285,27 @@ func TestFetchToolsEmptyURL(t *testing.T) {
}
func TestSubstituteTemplate(t *testing.T) {
vars := map[string]string{"token": "abc123"}
if got := substituteTemplate("Bearer ${token}", vars); got != "Bearer abc123" {
t.Errorf("got %q", got)
}
if got := substituteTemplate("Bearer ${missing}", vars); got != "Bearer ${missing}" {
t.Errorf("got %q", got)
}
if got := substituteTemplate("no-var", vars); got != "no-var" {
t.Errorf("got %q", got)
}
if got := substituteTemplate("${a}-${token}", map[string]string{"a": "1", "token": "2"}); got != "1-2" {
t.Errorf("got %q", got)
vars := map[string]string{"token": "abc123", "name": "search", "nested": "${name}"}
for _, tc := range []struct {
name string
in string
want string
}{
{name: "braced", in: "Bearer ${token}", want: "Bearer abc123"},
{name: "unbraced", in: "X-$name", want: "X-search"},
{name: "escaped dollar", in: "cost $$5", want: "cost $5"},
{name: "unknown", in: "Bearer ${missing}", want: "Bearer ${missing}"},
{name: "multiple", in: "${name}-${token}", want: "search-abc123"},
{name: "non recursive", in: "$nested", want: "${name}"},
{name: "malformed braced", in: "${name", want: "${name"},
{name: "invalid identifier", in: "${1name}", want: "${1name}"},
{name: "literal dollar", in: "no-var$", want: "no-var$"},
} {
t.Run(tc.name, func(t *testing.T) {
if got := substituteTemplate(tc.in, vars); got != tc.want {
t.Errorf("substituteTemplate(%q)=%q, want %q", tc.in, got, tc.want)
}
})
}
}
@@ -610,6 +610,90 @@ def test_import_multiple_missing_servers_and_exception(monkeypatch):
assert "import explode" in res["message"]
@pytest.mark.p2
@pytest.mark.parametrize("headers", [{"User-Agent": "ragflow", "authorization": "Token ${authorization_token}"}, {"X-Custom-Token": "${custom_token}"}, {}])
def test_import_custom_headers_survive_discovery_persistence_and_export(monkeypatch, headers):
module = _load_mcp_api(monkeypatch)
_stub_url_safety(monkeypatch, module)
_set_request_json(
monkeypatch,
module,
{
"mcpServers": {
"research": {
"type": "streamable-http",
"url": "https://example.com/mcp",
"headers": headers,
"authorization_token": "saved-token",
"custom_token": "example-token",
"name": "header-identity",
}
}
},
)
discovered = []
stored = []
async def discover(_func, servers, _timeout):
discovered.append(servers[0].headers)
return {servers[0].name: [{"name": "web_search"}]}, None
monkeypatch.setattr(module, "thread_pool_exec", discover)
monkeypatch.setattr(module.MCPServerService, "insert", lambda **data: stored.append(data) or True)
result = _run(module.import_multiple.__wrapped__())
assert result["data"]["results"][0]["success"] is True
assert discovered == [headers]
assert stored[0]["headers"] == headers
assert stored[0]["variables"]["custom_token"] == "example-token"
assert stored[0]["variables"]["name"] == "header-identity"
saved = _DummyMCPServer(**stored[0])
monkeypatch.setattr(module.MCPServerService, "get_by_id", lambda _id: (True, saved))
assert module._export_mcp_servers([saved.id])["mcpServers"]["research"]["headers"] == headers
assert module._export_mcp_servers([saved.id])["mcpServers"]["research"]["custom_token"] == "example-token"
assert module._export_mcp_servers([saved.id])["mcpServers"]["research"]["name"] == "header-identity"
exported = module._export_mcp_servers([saved.id])
_set_request_json(monkeypatch, module, exported)
result = _run(module.import_multiple.__wrapped__())
assert result["data"]["results"][0]["success"] is True
assert discovered == [headers, headers]
assert stored[1]["headers"] == headers
assert stored[1]["variables"]["authorization_token"] == "saved-token"
@pytest.mark.p2
@pytest.mark.parametrize("headers", [[], {"User-Agent": 123}, "not-json"])
def test_import_invalid_headers_do_not_discover_or_insert(monkeypatch, headers):
module = _load_mcp_api(monkeypatch)
_stub_url_safety(monkeypatch, module)
_set_request_json(monkeypatch, module, {"mcpServers": {"bad": {"type": "streamable-http", "url": "https://example.com/mcp", "headers": headers}}})
monkeypatch.setattr(module.MCPServerService, "insert", lambda **_data: pytest.fail("Invalid headers must not be saved"))
async def forbidden_discovery(*_args):
pytest.fail("Invalid headers must not reach discovery")
monkeypatch.setattr(module, "thread_pool_exec", forbidden_discovery)
result = _run(module.import_multiple.__wrapped__())
assert result["data"]["results"][0]["success"] is False
assert "headers" in result["data"]["results"][0]["message"].lower()
@pytest.mark.p2
@pytest.mark.parametrize("token", [123, None, {}, []])
def test_import_rejects_non_string_authorization_token_before_discovery(monkeypatch, token):
module = _load_mcp_api(monkeypatch)
_stub_url_safety(monkeypatch, module)
_set_request_json(monkeypatch, module, {"mcpServers": {"bad": {"type": "streamable-http", "url": "https://example.com/mcp", "authorization_token": token}}})
async def forbidden_discovery(*_args):
pytest.fail("Invalid authorization token must not reach discovery")
monkeypatch.setattr(module, "thread_pool_exec", forbidden_discovery)
monkeypatch.setattr(module.MCPServerService, "insert", lambda **_data: pytest.fail("Invalid token must not be saved"))
result = _run(module.import_multiple.__wrapped__())
assert result["data"]["results"][0]["success"] is False
assert "authorization_token" in result["data"]["results"][0]["message"]
@pytest.mark.p2
def test_import_multiple_mixed_results(monkeypatch):
module = _load_mcp_api(monkeypatch)
@@ -643,6 +727,8 @@ def test_import_multiple_mixed_results(monkeypatch):
async def _thread_pool_exec(func, servers, _timeout):
mcp_server = servers[0]
if mcp_server.name == "dup_0":
assert mcp_server.headers == {"authorization_token": "dup-token"}
if mcp_server.name == "tool_err":
return None, "tool call failed"
return {mcp_server.name: [{"name": "tool_a"}, {"invalid": True}]}, None
+1
View File
@@ -1,4 +1,5 @@
export interface IMcpServer {
headers?: Record<string, string>;
create_date: string;
description: null;
id: string;
@@ -43,6 +43,7 @@ import {
useBuildFormSchema,
} from './edit-mcp-form';
import { McpToolCard } from './tool-card';
import { connectionFields, TokenMask } from './mcp-connection';
function transferToolToArray(tools: IMCPToolObject) {
return Object.entries(tools).reduce<IMCPTool[]>((pre, [name, tool]) => {
@@ -58,10 +59,6 @@ const DefaultValues = {
authorization_token: '',
};
// Mask shown in the token field when editing an existing server that already
// has a token, so the user knows a secret is stored without exposing it.
const TokenMask = '********';
export function EditMcpDialog({
hideModal,
loading,
@@ -105,14 +102,9 @@ export function EditMcpDialog({
const handleOk = async (values: z.infer<typeof FormSchema>) => {
// When the field still holds the mask, the user did not change the token;
// send the previously stored token so it is not overwritten with empty.
const token =
values.authorization_token === TokenMask
? data.variables?.authorization_token
: values.authorization_token;
const nextValues = {
...omit(values, 'authorization_token'),
variables: { authorization_token: token },
headers: { Authorization: 'Bearer ${authorization_token}' },
...connectionFields(values.authorization_token, data),
};
if (isTriggeredBySaving) {
onOk?.(nextValues);
@@ -0,0 +1,126 @@
import { connectionFields, TokenMask } from './mcp-connection';
it('preserves imported headers and template variables without mutating the saved connection', () => {
const saved = {
headers: { 'User-Agent': 'ragflow', 'X-Token': '${custom}' },
variables: { custom: 'secret', name: 'header-identity' },
};
const before = JSON.stringify(saved);
expect(connectionFields('', saved)).toEqual({
headers: saved.headers,
variables: { ...saved.variables, authorization_token: '' },
});
expect(JSON.stringify(saved)).toBe(before);
});
it('keeps a masked token and the imported authentication scheme', () => {
const saved = {
headers: {
authorization: 'Token ${authorization_token}',
'User-Agent': 'ragflow',
},
variables: { authorization_token: 'secret' },
};
expect(connectionFields(TokenMask, saved)).toEqual(saved);
});
it('changes only authorization when the user explicitly changes or clears the token', () => {
const saved = {
headers: {
authorization: 'Bearer ${authorization_token}',
'User-Agent': 'ragflow',
},
variables: { authorization_token: 'old', other: 'keep' },
};
expect(connectionFields('new', saved)).toEqual({
headers: {
Authorization: 'Bearer ${authorization_token}',
'User-Agent': 'ragflow',
},
variables: { authorization_token: 'new', other: 'keep' },
});
expect(connectionFields('', saved)).toEqual({
headers: { 'User-Agent': 'ragflow' },
variables: { authorization_token: '', other: 'keep' },
});
});
it('does not add an authentication placeholder to a new no-key connection', () => {
expect(connectionFields('', {})).toEqual({
headers: {},
variables: { authorization_token: '' },
});
});
it.each(['authorization_token', 'Authorization_Token'])(
'removes the Python-imported %s credential when the token is changed or cleared',
(header) => {
const saved = {
headers: { [header]: 'old-secret', 'User-Agent': 'ragflow' },
variables: { authorization_token: 'old-secret' },
};
expect(connectionFields(TokenMask, saved)).toEqual(saved);
expect(connectionFields('', saved)).toEqual({
headers: { 'User-Agent': 'ragflow' },
variables: { authorization_token: '' },
});
expect(connectionFields('new-secret', saved)).toEqual({
headers: {
'User-Agent': 'ragflow',
Authorization: 'Bearer ${authorization_token}',
},
variables: { authorization_token: 'new-secret' },
});
},
);
it.each(['${auth_header}', '$auth_header', '${prefix}ization', '${go header}'])(
'removes authentication resolved from %s while preserving unrelated templates',
(header) => {
const saved = {
headers: {
[header]: 'Bearer old-secret',
'${agent_header}': '${agent}',
$$literal: 'keep',
},
variables: {
authorization_token: 'old-secret',
auth_header: 'Authorization',
prefix: 'Author',
'go header': 'Authorization',
agent_header: 'User-Agent',
agent: 'ragflow',
},
};
expect(connectionFields(TokenMask, saved)).toEqual(saved);
expect(connectionFields('', saved).headers).toEqual({
'${agent_header}': '${agent}',
$$literal: 'keep',
});
expect(connectionFields('new-secret', saved).headers).toEqual({
'${agent_header}': '${agent}',
$$literal: 'keep',
Authorization: 'Bearer ${authorization_token}',
});
},
);
it('classifies token-dependent names before and after replacement without recursive expansion', () => {
const saved = {
headers: {
'${authorization_token}': 'old-secret',
'$${auth}': 'keep',
'${alias}': 'keep',
},
variables: {
authorization_token: 'X-Old',
auth: 'Authorization',
alias: '${auth}',
},
};
expect(connectionFields('Authorization', saved).headers).toEqual({
'$${auth}': 'keep',
'${alias}': 'keep',
Authorization: 'Bearer ${authorization_token}',
});
});
@@ -0,0 +1,58 @@
// Editing an imported server must not replace its authentication scheme or
// custom headers unless the user actually changes the token field.
export const TokenMask = '********';
type Connection = {
headers?: Record<string, string>;
variables?: Record<string, unknown>;
};
// Python accepts $name/${name} and $$ escapes; Go accepts arbitrary ${name}
// keys. Classify both interpretations without rewriting unrelated templates.
function isAuthorizationHeader(
key: string,
variables: Record<string, unknown>,
) {
const lookup = (match: string, name: string) =>
Object.prototype.hasOwnProperty.call(variables, name)
? String(variables[name])
: match;
const python = key.replace(
/\$\$|\$\{([a-z_][a-z0-9_]*)\}|\$([a-z_][a-z0-9_]*)/gi,
(match, braced, plain) =>
match === '$$' ? '$' : lookup(match, braced ?? plain),
);
const go = key.replace(/\$\{([^}]*)\}/g, lookup);
return [key, python, go].some((name) =>
['authorization', 'authorization_token'].includes(name.toLowerCase()),
);
}
export function connectionFields(
value: string | undefined,
current: Connection,
) {
const storedToken = current.variables?.authorization_token;
const token = value === TokenMask ? storedToken : value;
const unchanged =
value === TokenMask || (token || '') === (storedToken || '');
const headers = { ...current.headers };
if (!unchanged) {
for (const key of Object.keys(headers)) {
if (
isAuthorizationHeader(key, current.variables ?? {}) ||
isAuthorizationHeader(key, {
...current.variables,
authorization_token: token,
})
) {
delete headers[key];
}
}
if (token) headers.Authorization = 'Bearer ${authorization_token}';
}
return {
variables: { ...current.variables, authorization_token: token },
headers,
};
}