feat(EVO-1739): stateless POST /custom-mcp-servers/test-connection (test-before-save) - #21
Merged
Merged
Conversation
…est-before-save)
The MCP wizard needs to test a server's url/headers BEFORE it is saved, but the only
test endpoint was GET /:id/test (requires a persisted server). Add a stateless
POST /custom-mcp-servers/test-connection that accepts {url, headers} and returns the
same TestResult (success, status_code, tools_count, message/error).
- service: expose TestConnection(ctx, url, headers) — a thin public wrapper over the
existing private testConnection (which already delegates the real MCP handshake to
the processor's /custom-mcp-servers/test-connection, EVO-2139, with tenant-header
propagation). No new outbound surface beyond what a saved-then-tested server already
allows; gated by ai_custom_mcp_servers:read.
- handler: TestConnection binds {url(required), headers}, ValidationErrorResponse on
bad body; route POST /custom-mcp-servers/test-connection.
- test: TestConnection public wrapper delegates to the processor test-connection path
and returns the result unchanged. go build/vet/test ./pkg/custom_mcp_server/... green.
Backend half of the EVO-1739 wizard "Test" button; the frontend wires it to the wizard.
Note: the file is pre-existing CRLF on develop, so it stays out of gofmt to avoid a
line-ending-only diff.
Reviewer's GuideAdds a stateless POST /custom-mcp-servers/test-connection endpoint and a corresponding service wrapper so the MCP wizard can test arbitrary URL/headers before persisting a server, reusing the existing processor-based MCP handshake and exposing the same TestResult structure. Sequence diagram for stateless POST /custom-mcp-servers/test-connectionsequenceDiagram
actor WizardFrontend
participant CustomMcpServerHandler
participant CustomMcpServerService
participant Processor
WizardFrontend->>CustomMcpServerHandler: POST /custom-mcp-servers/test-connection
CustomMcpServerHandler->>CustomMcpServerHandler: ShouldBindJSON
alt invalid body
CustomMcpServerHandler-->>WizardFrontend: ValidationErrorResponse
else valid body
CustomMcpServerHandler->>CustomMcpServerService: TestConnection(ctx, url, headers)
CustomMcpServerService->>Processor: testConnection(ctx, url, headers)
Processor-->>CustomMcpServerService: TestResult
CustomMcpServerService-->>CustomMcpServerHandler: TestResult
CustomMcpServerHandler-->>WizardFrontend: SuccessResponse(test_result)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pkg/custom_mcp_server/handler/custom_mcp_server_handler.go" line_range="319-328" />
<code_context>
}
+
+// 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
+ }
+
+ testResult, err := h.customMcpServerService.TestConnection(c.Request.Context(), req.URL, req.Headers)
+ if err != nil {
+ code, message, httpCode := errors.HandleError(err)
</code_context>
<issue_to_address>
**🚨 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.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+319
to
+328
| 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 | ||
| } | ||
|
|
There was a problem hiding this comment.
🚨 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.
The new endpoint was gated on `ai_custom_mcp_servers:read`. That is not the same surface a saved-and-tested server already exposes: GET /:id/test can only reach a url somebody with `create` persisted, whereas this one drives an arbitrary outbound request from the processor with caller-supplied url and headers, and hands back the status code, latency and error. Under `read` that gives every read-only user a working SSRF probe against the internal network. `create` is the permission that already means "point the processor at a url of my choosing", so the gate now matches the capability. The url is also validated before anything goes out: absolute http/https with a host, which rejects file://, gopher:// and hostless urls. Deliberately NOT a private-IP blocklist -- a self-hosted Evolution routinely runs its MCP servers on the same private network, so that would break the common case while barely inconveniencing an attacker who already holds `create`. Tests: the handler package had none at all; adds 5 covering the binding (missing url, non-string header values), the pass-through of a service validation error, the forwarded payload and the response shape the frontend reads, plus 7 service cases for the url rules. Also unorphans the EVO-2139 tenant-header comment, which the new test was inserted between.
…g part The comments added in 6edfae6 argued the case instead of stating it. Keeps the two non-obvious decisions -- why the gate is `create` and why there is no private-IP blocklist -- and drops the rest. No behaviour change.
gomessguii
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EVO-1739 (backend) —
POST /custom-mcp-servers/test-connection(test-before-save)Por quê
O wizard precisa testar
url/headersantes de salvar, mas só haviaGET /:id/test(exige servidor persistido). Adiciona um endpoint stateless.Mudança
service:TestConnection(ctx, url, headers)— wrapper público fino sobre otestConnectionprivado (que já delega o handshake MCP real ao processor/custom-mcp-servers/test-connection, EVO-2139, com propagação deX-Evo-Tenant-Id). Retorna o mesmoTestResult(success,status_code,tools_count,message/error).handler:TestConnectionbind{url(obrigatório), headers}(ValidationErrorResponseem corpo inválido); rotaPOST /custom-mcp-servers/test-connection, gateai_custom_mcp_servers:read.Testes
TestTestConnection_PublicWrapper_DelegatesAndReturnsResult(reusa o processor httptest scriptável) — delega pro path certo e devolve o resultado.go build/vet/test ./pkg/custom_mcp_server/...verde.Summary by Sourcery
Introduce a stateless endpoint to test custom MCP server connections before saving configuration.
New Features:
Enhancements:
Tests: