Harden MCP config diagnostics in CLI parser

Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>

Author: cursoragent

src/cli/mcp-config.test.ts

@@ -53,6 +53,30 @@ describe("parseMCPServersConfig", () => {
     );
   });
 
+  it("sanitizes and truncates oversized JSON parse diagnostics", () => {
+    const parseSpy = jest.spyOn(JSON, "parse").mockImplementation(() => {
+      throw new Error(`parse\u0000\n${"x".repeat(10_000)}`);
+    });
+
+    try {
+      expect(() => parseMCPServersConfig('{"servers":[{"command":"npx"}]}')).toThrow(
+        "[truncated"
+      );
+      expect(() => parseMCPServersConfig('{"servers":[{"command":"npx"}]}')).toThrow(
+        /Invalid MCP config JSON:/
+      );
+      try {
+        parseMCPServersConfig('{"servers":[{"command":"npx"}]}');
+      } catch (error) {
+        const message = String(error instanceof Error ? error.message : error);
+        expect(message).not.toContain("\u0000");
+        expect(message).not.toContain("\n");
+      }
+    } finally {
+      parseSpy.mockRestore();
+    }
+  });
+
   it("throws clear message when config input is not a string", () => {
     expect(() => parseMCPServersConfig(42 as unknown as string)).toThrow(
       "Invalid MCP config JSON: config must be a string."
@@ -253,7 +277,7 @@ describe("parseMCPServersConfig", () => {
     expect(() =>
       parseMCPServersConfig('[{"connectionType":"sse\\u0007","command":"npx"}]')
     ).toThrow(
-      'MCP server entry at index 0 has unsupported connectionType "sse\u0007". Supported values are "stdio" and "sse".'
+      'MCP server entry at index 0 has unsupported connectionType "sse". Supported values are "stdio" and "sse".'
     );
 
     expect(() =>
@@ -611,7 +635,7 @@ describe("parseMCPServersConfig", () => {
         '[{"connectionType":"sse","sseUrl":"https://example.com/sse\\u0007"}]'
       )
     ).toThrow(
-      'MCP server entry at index 0 has invalid "sseUrl" value "https://example.com/sse\u0007".'
+      'MCP server entry at index 0 has invalid "sseUrl" value "https://example.com/sse".'
     );
 
     expect(() =>
@@ -749,6 +773,33 @@ describe("loadMCPServersFromFile", () => {
     );
   });
 
+  it("sanitizes and truncates oversized config read diagnostics", async () => {
+    const statSpy = jest.spyOn(fs.promises, "stat").mockResolvedValue({
+      isFile: () => true,
+      size: 1,
+    } as unknown as fs.Stats);
+    const readFileSpy = jest
+      .spyOn(fs.promises, "readFile")
+      .mockRejectedValue(new Error(`read\u0000\n${"x".repeat(10_000)}`));
+
+    try {
+      await loadMCPServersFromFile("/tmp/mcp-config-test.json")
+        .then(() => {
+          throw new Error("expected loadMCPServersFromFile to reject");
+        })
+        .catch((error) => {
+          const message = String(error instanceof Error ? error.message : error);
+          expect(message).toContain("[truncated");
+          expect(message).not.toContain("\u0000");
+          expect(message).not.toContain("\n");
+          expect(message.length).toBeLessThan(700);
+        });
+    } finally {
+      statSpy.mockRestore();
+      readFileSpy.mockRestore();
+    }
+  });
+
   it("throws readable error when config path is not a regular file", async () => {
     const tempDir = await fs.promises.mkdtemp(
       path.join(os.tmpdir(), "hyperagent-mcp-config-")

src/cli/mcp-config.ts

@@ -37,13 +37,20 @@ function hasAnyControlChars(value: string): boolean {
 }
 
 function formatMCPConfigDiagnostic(value: unknown): string {
-  const normalized =
-    typeof value === "string" ? value : formatUnknownError(value);
-  if (normalized.length <= MAX_MCP_CONFIG_DIAGNOSTIC_CHARS) {
-    return normalized;
-  }
-  const omitted = normalized.length - MAX_MCP_CONFIG_DIAGNOSTIC_CHARS;
-  return `${normalized.slice(0, MAX_MCP_CONFIG_DIAGNOSTIC_CHARS)}... [truncated ${omitted} chars]`;
+  const raw = typeof value === "string" ? value : formatUnknownError(value);
+  const normalized = Array.from(raw, (char) => {
+    const code = char.charCodeAt(0);
+    return (code >= 0 && code < 32) || code === 127 ? " " : char;
+  })
+    .join("")
+    .replace(/\s+/g, " ")
+    .trim();
+  const fallback = normalized.length > 0 ? normalized : "unknown error";
+  if (fallback.length <= MAX_MCP_CONFIG_DIAGNOSTIC_CHARS) {
+    return fallback;
+  }
+  const omitted = fallback.length - MAX_MCP_CONFIG_DIAGNOSTIC_CHARS;
+  return `${fallback.slice(0, MAX_MCP_CONFIG_DIAGNOSTIC_CHARS)}... [truncated ${omitted} chars]`;
 }
 
 const isRecord = (value: unknown): value is Record<string, unknown> =>
@@ -389,7 +396,7 @@ export function parseMCPServersConfig(rawConfig: string): MCPServerConfig[] {
     parsed = JSON.parse(normalizedConfig);
   } catch (error) {
     throw new Error(
-      `Invalid MCP config JSON: ${formatUnknownError(error)}`
+      `Invalid MCP config JSON: ${formatMCPConfigDiagnostic(error)}`
     );
   }
 
@@ -618,7 +625,9 @@ export async function loadMCPServersFromFile(
     fileContent = await fs.promises.readFile(normalizedFilePath, "utf-8");
   } catch (error) {
     throw new Error(
-      `Failed to read MCP config file "${normalizedFilePath}": ${formatUnknownError(error)}`
+      `Failed to read MCP config file "${normalizedFilePath}": ${formatMCPConfigDiagnostic(
+        error
+      )}`
     );
   }
 
@@ -632,7 +641,9 @@ export async function loadMCPServersFromFile(
     return parseMCPServersConfig(fileContent);
   } catch (error) {
     throw new Error(
-      `Invalid MCP config file "${normalizedFilePath}": ${formatUnknownError(error)}`
+      `Invalid MCP config file "${normalizedFilePath}": ${formatMCPConfigDiagnostic(
+        error
+      )}`
     );
   }
 }