Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions pkg/custom_mcp_server/handler/custom_mcp_server_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type CustomMcpServerHandler interface {
Update(c *gin.Context)
Delete(c *gin.Context)
Test(c *gin.Context)
TestConnection(c *gin.Context)
}

// customMcpServerHandler implements the CustomMcpServerHandler interface.
Expand Down Expand Up @@ -79,6 +80,12 @@ func (h *customMcpServerHandler) RegisterRoutesMiddleware(router gin.IRouter) {
customMcpServers.GET("/:id/test",
permissionMiddleware.RequirePermission("ai_custom_mcp_servers", "read"),
h.Test)
// EVO-1739: stateless test-before-save (validates url/headers typed in the wizard).
// `create`, not `read`: it fires an outbound request from the processor to a
// caller-supplied url and reports the outcome. `read` must not grant that.
customMcpServers.POST("/test-connection",
permissionMiddleware.RequirePermission("ai_custom_mcp_servers", "create"),
h.TestConnection)
}
}

Expand Down Expand Up @@ -309,3 +316,24 @@ func (h *customMcpServerHandler) Test(c *gin.Context) {

response.SuccessResponse(c, customMcpServer, "Custom MCP server test completed successfully", http.StatusOK)
}

// TestConnection tests an UNSAVED MCP server's url/headers (test-before-save). EVO-1739.
func (h *customMcpServerHandler) TestConnection(c *gin.Context) {
var req struct {
URL string `json:"url" binding:"required"`
Headers map[string]string `json:"headers"`
}
if err := c.ShouldBindJSON(&req); err != nil {
response.ValidationErrorResponse(c, err)
return
}

Comment on lines +321 to +330

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Consider adding stricter validation/constraints on the URL to avoid SSRF-style misuse.

Because this endpoint forwards a client-provided URL directly to TestConnection, it may allow SSRF (e.g., internal services, metadata endpoints), depending on that implementation. Please consider tightening input by restricting schemes (e.g., HTTPS only), adding host/IP allow/deny lists, and/or reusing any existing URL validation used for persisted MCP servers so this “test” path isn’t more permissive than the saved one.

testResult, err := h.customMcpServerService.TestConnection(c.Request.Context(), req.URL, req.Headers)
if err != nil {
code, message, httpCode := errors.HandleError(err)
response.ErrorResponse(c, code, message, nil, httpCode)
return
}

response.SuccessResponse(c, gin.H{"test_result": testResult}, "Custom MCP server test completed successfully", http.StatusOK)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
package handler

import (
"bytes"
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"

"evo-ai-core-service/internal/httpclient/errors"
"evo-ai-core-service/pkg/custom_mcp_server/model"
"evo-ai-core-service/pkg/custom_mcp_server/service"

"github.com/gin-gonic/gin"
)

// EVO-1739: covers the test-before-save handler — the only route taking a caller-supplied
// url. Embeds the service interface as nil, so any call other than TestConnection panics.
type testConnectionStub struct {
service.CustomMcpServerService

gotURL string
gotHeaders map[string]string
result *model.TestResult
err error
}

func (s *testConnectionStub) TestConnection(_ context.Context, url string, headers map[string]string) (*model.TestResult, error) {
s.gotURL = url
s.gotHeaders = headers
return s.result, s.err
}

func newTestConnectionRouter(stub *testConnectionStub) *gin.Engine {
gin.SetMode(gin.TestMode)
h := &customMcpServerHandler{customMcpServerService: stub}
r := gin.New()
r.POST("/custom-mcp-servers/test-connection", h.TestConnection)
return r
}

func doTestConnection(t *testing.T, stub *testConnectionStub, body string) *httptest.ResponseRecorder {
t.Helper()
w := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodPost, "/custom-mcp-servers/test-connection", bytes.NewBufferString(body))
req.Header.Set("Content-Type", "application/json")
newTestConnectionRouter(stub).ServeHTTP(w, req)
return w
}

func TestTestConnectionHandler_ForwardsURLAndHeaders(t *testing.T) {
stub := &testConnectionStub{
result: &model.TestResult{Success: true, StatusCode: http.StatusOK, ToolsCount: 4},
}

w := doTestConnection(t, stub, `{"url":"https://mcp.example/mcp","headers":{"Authorization":"Bearer sk-live"}}`)

if w.Code != http.StatusOK {
t.Fatalf("status: got %d want %d (body %s)", w.Code, http.StatusOK, w.Body.String())
}
if stub.gotURL != "https://mcp.example/mcp" {
t.Fatalf("url: got %q", stub.gotURL)
}
if stub.gotHeaders["Authorization"] != "Bearer sk-live" {
t.Fatalf("headers: got %v", stub.gotHeaders)
}

var envelope struct {
Data struct {
TestResult model.TestResult `json:"test_result"`
} `json:"data"`
}
if err := json.Unmarshal(w.Body.Bytes(), &envelope); err != nil {
t.Fatalf("decode: %v (body %s)", err, w.Body.String())
}
// Pins the shape the frontend reads: data.test_result.tools_count.
if !envelope.Data.TestResult.Success || envelope.Data.TestResult.ToolsCount != 4 {
t.Fatalf("test_result: got %+v", envelope.Data.TestResult)
}
}

func TestTestConnectionHandler_RejectsMissingURL(t *testing.T) {
stub := &testConnectionStub{result: &model.TestResult{Success: true}}

w := doTestConnection(t, stub, `{"headers":{"X-Key":"v"}}`)

if w.Code != http.StatusBadRequest {
t.Fatalf("status: got %d want %d (body %s)", w.Code, http.StatusBadRequest, w.Body.String())
}
if stub.gotURL != "" {
t.Fatalf("service must not be called on an invalid body, got url %q", stub.gotURL)
}
}

func TestTestConnectionHandler_RejectsNonStringHeaderValues(t *testing.T) {
stub := &testConnectionStub{result: &model.TestResult{Success: true}}

// The wizard's advanced-JSON mode can produce this; must 400, not reach the service.
w := doTestConnection(t, stub, `{"url":"https://mcp.example/mcp","headers":{"X-Api-Key":123}}`)

if w.Code != http.StatusBadRequest {
t.Fatalf("status: got %d want %d (body %s)", w.Code, http.StatusBadRequest, w.Body.String())
}
if stub.gotURL != "" {
t.Fatalf("service must not be called, got url %q", stub.gotURL)
}
}

func TestTestConnectionHandler_PropagatesServiceValidationError(t *testing.T) {
stub := &testConnectionStub{
err: errors.New(errors.ValidationError, "url must use the http or https scheme", http.StatusBadRequest),
}

w := doTestConnection(t, stub, `{"url":"file:///etc/passwd"}`)

if w.Code != http.StatusBadRequest {
t.Fatalf("status: got %d want %d (body %s)", w.Code, http.StatusBadRequest, w.Body.String())
}
}

// A failed handshake is still a successful call — the failure lives inside test_result.
func TestTestConnectionHandler_FailedHandshakeIsStill200(t *testing.T) {
stub := &testConnectionStub{
result: &model.TestResult{Success: false, StatusCode: http.StatusBadGateway, Error: "connection refused"},
}

w := doTestConnection(t, stub, `{"url":"https://mcp.example/mcp"}`)

if w.Code != http.StatusOK {
t.Fatalf("status: got %d want %d (body %s)", w.Code, http.StatusOK, w.Body.String())
}
var envelope struct {
Data struct {
TestResult model.TestResult `json:"test_result"`
} `json:"data"`
}
if err := json.Unmarshal(w.Body.Bytes(), &envelope); err != nil {
t.Fatalf("decode: %v", err)
}
if envelope.Data.TestResult.Success || envelope.Data.TestResult.Error != "connection refused" {
t.Fatalf("test_result: got %+v", envelope.Data.TestResult)
}
}
38 changes: 38 additions & 0 deletions pkg/custom_mcp_server/service/custom_mcp_server_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"evo-ai-core-service/internal/config"
"evo-ai-core-service/internal/httpclient"
apierrors "evo-ai-core-service/internal/httpclient/errors"
errorsPostgres "evo-ai-core-service/internal/infra/postgres"
"evo-ai-core-service/internal/utils/contextutils"
"evo-ai-core-service/internal/utils/stringutils"
Expand All @@ -14,6 +15,8 @@ import (
"evo-ai-core-service/pkg/evoextensions/runtimecontext"
"fmt"
"net/http"
neturl "net/url"
"strings"

"github.com/google/uuid"
)
Expand All @@ -26,6 +29,9 @@ type CustomMcpServerService interface {
Delete(ctx context.Context, id uuid.UUID) (bool, error)
GetByAgentConfig(ctx context.Context, serverIDs []uuid.UUID) ([]*model.CustomMcpServer, error)
Test(ctx context.Context, id uuid.UUID) (*model.CustomMcpServerTestResponse, error)
// EVO-1739: stateless test of an UNSAVED server's url/headers, so the wizard can
// "test before save". Reuses the same processor MCP handshake as Test.
TestConnection(ctx context.Context, url string, headers map[string]string) (*model.TestResult, error)
}

type customMcpServerService struct {
Expand Down Expand Up @@ -212,6 +218,38 @@ func (s *customMcpServerService) Test(ctx context.Context, id uuid.UUID) (*model
}, nil
}

// TestConnection runs the MCP handshake against arbitrary url/headers without a saved
// server — powers the wizard's "test before save" (EVO-1739).
func (s *customMcpServerService) TestConnection(ctx context.Context, rawURL string, headers map[string]string) (*model.TestResult, error) {
if err := validateTestConnectionURL(rawURL); err != nil {
return nil, err
}
return s.testConnection(ctx, rawURL, headers)
}

// validateTestConnectionURL requires an absolute http/https url with a host, since the
// caller supplies it and the processor dials it. No private/loopback blocklist: a
// self-hosted Evolution normally runs its MCP servers on the same private network.
func validateTestConnectionURL(rawURL string) error {
trimmed := strings.TrimSpace(rawURL)
if trimmed == "" {
return apierrors.New(apierrors.ValidationError, "url is required", http.StatusBadRequest)
}

parsed, err := neturl.Parse(trimmed)
if err != nil {
return apierrors.New(apierrors.ValidationError, "url is not a valid URL", http.StatusBadRequest)
}
if parsed.Scheme != "http" && parsed.Scheme != "https" {
return apierrors.New(apierrors.ValidationError, "url must use the http or https scheme", http.StatusBadRequest)
}
if parsed.Host == "" {
return apierrors.New(apierrors.ValidationError, "url must include a host", http.StatusBadRequest)
}

return nil
}

// EVO-2139: delegate the MCP connection test to the processor, which owns
// the real handshake (POST JSON-RPC 2.0 `initialize`) via Google ADK's
// MCPToolset. The previous implementation did a raw `GET /health` from Go
Expand Down
69 changes: 69 additions & 0 deletions pkg/custom_mcp_server/service/custom_mcp_server_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,75 @@ func TestTestConnection_DelegatesToProcessor(t *testing.T) {
}
}

// EVO-1739: the public TestConnection wrapper (test-before-save) must delegate to the
// same processor handshake and surface the TestResult unchanged.
func TestTestConnection_PublicWrapper_DelegatesAndReturnsResult(t *testing.T) {
cs := newCaptureTestServer(t, `{"success":true,"status_code":200,"response_time":0.1,"url_tested":"https://mcp.example/mcp","message":"ok","tools_count":2}`)
svc := newServiceForTest(cs.URL)
ctx := context.WithValue(context.Background(), "token", "tok-abc")

result, err := svc.TestConnection(ctx, "https://mcp.example/mcp", map[string]string{})
if err != nil {
t.Fatalf("TestConnection: %v", err)
}
if want := "/api/v1/custom-mcp-servers/test-connection"; cs.gotPath != want {
t.Fatalf("path: got %q want %q", cs.gotPath, want)
}
if !result.Success || result.StatusCode != http.StatusOK {
t.Fatalf("want success 200, got success=%v code=%d", result.Success, result.StatusCode)
}
}

// EVO-1739: bad schemes and hostless urls are rejected before any request goes out —
// asserted by the processor stub never being hit.
func TestTestConnection_RejectsNonHTTPURLs(t *testing.T) {
cases := []struct {
name string
url string
}{
{"empty", ""},
{"blank", " "},
{"file scheme", "file:///etc/passwd"},
{"gopher scheme", "gopher://internal:70/_dict"},
{"no scheme", "mcp.example/mcp"},
{"scheme without host", "http:///mcp"},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
cs := newCaptureTestServer(t, `{"success":true,"status_code":200,"tools_count":0}`)
svc := newServiceForTest(cs.URL)
ctx := context.WithValue(context.Background(), "token", "tok-abc")

result, err := svc.TestConnection(ctx, tc.url, map[string]string{})
if err == nil {
t.Fatalf("TestConnection(%q): want a validation error, got result %+v", tc.url, result)
}
if result != nil {
t.Fatalf("TestConnection(%q): want nil result alongside the error, got %+v", tc.url, result)
}
if cs.gotPath != "" {
t.Fatalf("TestConnection(%q): processor was called at %q — the url must be rejected before any outbound request", tc.url, cs.gotPath)
}
})
}
}

// EVO-1739: guards against the validation degrading into a private-IP blocklist —
// self-hosted MCP servers routinely sit on the same private network.
func TestTestConnection_AllowsPlainHTTPAndPrivateHosts(t *testing.T) {
cs := newCaptureTestServer(t, `{"success":true,"status_code":200,"tools_count":1}`)
svc := newServiceForTest(cs.URL)
ctx := context.WithValue(context.Background(), "token", "tok-abc")

if _, err := svc.TestConnection(ctx, "http://mcp.internal:8080/mcp", map[string]string{}); err != nil {
t.Fatalf("TestConnection: %v", err)
}
if cs.gotPath == "" {
t.Fatal("processor was not called for a valid private-network http URL")
}
}

// EVO-2139: propagate X-Evo-Tenant-Id on the test call too, so the
// processor's runtime_context middleware (PY-1) can authorize it.
// Paridade com TestDiscoverTools_PropagatesTenantHeader_WhenBound.
Expand Down
Loading