Skip to content

fix(import): sanitize server names before the import filter check#729

Merged
Dumbris merged 1 commit into
smart-mcp-proxy:mainfrom
HaloCollar:upstream-pr/import-sanitize-before-filter
Jun 19, 2026
Merged

fix(import): sanitize server names before the import filter check#729
Dumbris merged 1 commit into
smart-mcp-proxy:mainfrom
HaloCollar:upstream-pr/import-sanitize-before-filter

Conversation

@electrolobzik

Copy link
Copy Markdown
Contributor

Problem

The two-step config-import flow in the Web UI (preview, then import the selected servers) silently imports zero servers when a server name needs sanitizing — e.g. a name containing a space like Figma Desktop:

{ "mcpServers": { "Figma Desktop": { "url": "http://127.0.0.1:3845/mcp" } } }

The preview says "Will import 1 server", but after clicking Import the UI shows a success toast reading "0 servers imported".

Root cause

Server names are validated against ^[a-zA-Z0-9_-]+$ and sanitized on import (Figma DesktopFigma_Desktop). In configimport.Import:

  1. The preview call (no server_names filter) sanitizes the name and surfaces Figma_Desktop to the client, which pre-selects it.
  2. The import call sends server_names: ["Figma_Desktop"], but the filter check ran against the un-sanitized parsed name (Figma Desktop) before sanitization, so filterSet["Figma_Desktop"] never matched. The server was reported filtered_out and Summary.Imported came back 0.

Fix

Resolve the effective (sanitized) name before the filter, dedup, and found-server tracking. Match the filter against either the raw source name (the CLI --server flag passes it verbatim) or the sanitized name (the Web UI selects it from the preview), and track both names so the "requested server not found" warning stays correct on both paths. ImportedServer.OriginalName keeps pointing at the true source name so the httpapi rename map continues to resolve.

Test

Adds TestImport_FilterMatchesSanitizedName, which reproduces the two-step flow for a space-containing name and asserts the server is imported via both the Web UI (sanitized-name) and CLI (raw-name) filter paths, with no spurious not-found warning and OriginalName preserved.

Repro before the fix

Import the Figma config above through the UI → "0 servers imported" despite a green toast. After the fix it imports 1 server (quarantined, as expected).

The two-step Web UI import (preview, then import-the-selected-servers)
silently imported zero servers when a server name needed sanitizing —
e.g. "Figma Desktop". The preview surfaced the sanitized name
("Figma_Desktop") and the import call passed that name in server_names,
but the filter compared it against the still-un-sanitized parsed name,
so the server was reported filtered_out and Summary.Imported was 0
despite a success response.

Resolve the effective (sanitized) name before the filter, dedup, and
found-server tracking. Match the filter against either the raw source
name (the CLI --server flag passes it verbatim) or the sanitized name
(the Web UI selects it from the preview), and track both names so the
"requested server not found" warning stays correct on both paths. Keep
ImportedServer.OriginalName pointing at the true source name so the
httpapi rename map continues to resolve.

Add a regression test covering both the Web UI (sanitized-name) and CLI
(raw-name) filter paths for a space-containing server name.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 55.00000% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/configimport/import.go 55.00% 7 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@mcpproxy-gatekeeper mcpproxy-gatekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

APPROVE — reviewed via Claude Code. Fixes 'import adds 0 servers': configimport now resolves effectiveName=SanitizeServerName(name) BEFORE the filter check and matches filter against raw OR sanitized name (Web UI sends sanitized, CLI sends raw). Strong new test covers both paths; edge cases (empty/unsanitizable/post-sanitize collision/slash) verified. go build + configimport/httpapi/cmd tests pass, vet+gofmt clean. One NON-BLOCKING finding flagged for follow-up: OriginalName now=raw widens a latent frontend rename-map key mismatch (narrow intersection, no test coverage either way) — follow-up issue filed, not a merge blocker.

@Dumbris Dumbris merged commit 7b5bf16 into smart-mcp-proxy:main Jun 19, 2026
35 checks passed
Dumbris added a commit that referenced this pull request Jun 20, 2026
#731)

After #729, ImportedServer.OriginalName carries the raw source name while
the Web UI wizard keys its conflict-rename map by the sanitized preview
name (Server.Name). For a server whose name needs sanitizing and is
selected from 2+ sources, the httpapi rename lookup against OriginalName
missed and the conflict rename was silently dropped.

Match the rename map against either the raw OriginalName (CLI/documented
contract) or the sanitized Server.Name (wizard), keeping both paths
resolving. Add a regression test covering the conflict-rename +
sanitizable-name combination on both key shapes, and update the rename
field doc + regenerated OpenAPI.

Related #729
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants