diff --git a/src/application/ClientConfigurationDocument.shape.test.ts b/src/application/ClientConfigurationDocument.shape.test.ts new file mode 100644 index 000000000..67c557a44 --- /dev/null +++ b/src/application/ClientConfigurationDocument.shape.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; + +import { parseClientConfiguration } from "./ClientConfigurationDocument.js"; + +describe("parseClientConfiguration shape diagnostics", () => { + it.each([ + ["null", "null"], + ["array", "[]"], + ["string", '"x"'], + ] as const)("names a non-object JSON root (%s)", (_label, text) => { + expect(() => parseClientConfiguration(text, "json")).toThrow( + new TypeError("The configuration root must be an object"), + ); + }); + + it.each([ + ["null", "null"], + ["array", "[]"], + ["string", '"x"'], + ] as const)( + 'names a non-object "mcpServers" value (%s)', + (_label, servers) => { + expect(() => + parseClientConfiguration(`{"mcpServers":${servers},"keep":1}`, "json"), + ).toThrow(new TypeError('"mcpServers" must be an object')); + }, + ); + + it("still rejects unparsable JSON with SyntaxError", () => { + expect(() => parseClientConfiguration("not-json", "json")).toThrow( + SyntaxError, + ); + }); +}); diff --git a/src/application/ClientConfigurationDocument.ts b/src/application/ClientConfigurationDocument.ts index cecbd440a..b32df3fd0 100644 --- a/src/application/ClientConfigurationDocument.ts +++ b/src/application/ClientConfigurationDocument.ts @@ -107,18 +107,21 @@ const parseDocument = ( format: ClientConfigurationFormat, ): Record => { if (format === "toml" || format === "grok") - return objectSchema.parse(parseToml(text)); + return requireObject(parseToml(text), "The configuration root"); // Pi loads `mcp.json` with a strict `JSON.parse` and no BOM tolerance, so // comments, trailing commas, a BOM, or an empty existing file must be // rejected instead of registering into a document Pi cannot load. - if (format === "pi") return objectSchema.parse(JSON.parse(text)); + if (format === "pi") + return requireObject(JSON.parse(text), "The configuration root"); // An empty file, which some clients create before any server is added, // holds no settings to preserve. if (isEmptyClientConfigurationText(text)) return {}; // Hermes stores MCP servers in `mcp_servers` inside a YAML document. - if (format === "hermes") return objectSchema.parse(parseYaml(text)); + if (format === "hermes") + return requireObject(parseYaml(text), "The configuration root"); // ZCode loads nonempty native files with strict JSON.parse. - if (format === "zcode") return objectSchema.parse(JSON.parse(text)); + if (format === "zcode") + return requireObject(JSON.parse(text), "The configuration root"); const errors: ParseError[] = []; // Accept a UTF-8 BOM without shifting diagnostics or editing the original text. const jsonText = text.startsWith("\uFEFF") ? ` ${text.slice(1)}` : text; @@ -128,12 +131,13 @@ const parseDocument = ( throw new SyntaxError( `Invalid JSON/JSONC at offset ${firstError.offset}: ${printParseErrorCode(firstError.error)}`, ); - return objectSchema.parse(document); + return requireObject(document, "The configuration root"); }; /** A parsed configuration whose structure, not its syntax, cannot hold MCP servers. */ export class ClientConfigurationShapeError extends TypeError {} +/** Validate and copy own properties, excluding non-record YAML values and parser-injected prototypes. */ const requireObject = ( value: unknown, description: string, @@ -174,17 +178,7 @@ export const parseClientConfiguration = ( throw new TypeError("client does not have a supported MCP config format"); // DeepSeek Harness stores a Cordis patch list, not a server table. if (format === "dsh") return parseDshCordisDocument(text); - let document: Record; - try { - document = parseDocument(text, format); - } catch (cause: unknown) { - if (cause instanceof z.ZodError) - throw new ClientConfigurationShapeError( - "The configuration root must be an object", - { cause }, - ); - throw cause; - } + const document = parseDocument(text, format); const serversKey = clientConfigurationServersKey(format); const value = document[serversKey]; const servers = diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 2d814a5f3..506a6d6df 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1661,8 +1661,9 @@ describe("uninstall configuration preflight", () => { }); it.each([ - ['{"mcpServers": null}', '"mcpServers" must be an object'], + ['{"mcpServers":null,"keep":1}', '"mcpServers" must be an object'], ["[]", "The configuration root must be an object"], + ['{"mcpServers":"x"}', '"mcpServers" must be an object'], ])( "names the structural problem in valid JSON %s", async (content, problem) => { @@ -1679,12 +1680,14 @@ describe("uninstall configuration preflight", () => { }, }); const result = await runUninstall(false, systemUninstallHost(home)); + expect(result.status).toBe("failed"); expect(result.items).toContainEqual({ name: "cursor", status: "failed", detail: `Configuration was not changed: ${problem}. Repair it, then rerun uninstall.`, }); expect(await readFile(config, "utf8")).toBe(content); + await expect(readFile(`${config}.rea.backup`, "utf8")).rejects.toThrow(); }, ); diff --git a/tests/boundary/filesystem/hermesClientConfiguration.test.ts b/tests/boundary/filesystem/hermesClientConfiguration.test.ts index f6b3815ee..43a5f841c 100644 --- a/tests/boundary/filesystem/hermesClientConfiguration.test.ts +++ b/tests/boundary/filesystem/hermesClientConfiguration.test.ts @@ -6,9 +6,15 @@ import { parse as parseYaml } from "yaml"; import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js"; import { npxRegistrationCommand } from "../../../src/application/ClientRegistrationIdentity.js"; -import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js"; +import { + configureClientConfiguration, + inspectClientConfiguration, +} from "../../../src/application/SetupClientConfiguration.js"; import { supportedClients } from "../../../src/application/SupportedClients.js"; -import { systemUninstallHost } from "../../../src/application/Uninstall.js"; +import { + runUninstall, + systemUninstallHost, +} from "../../../src/application/Uninstall.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { clearClientLocationEnvironment } from "../../fixtures/clientEnvironment.js"; @@ -90,18 +96,75 @@ describe("Hermes configuration paths", () => { }); }); -describe("Hermes YAML registration", () => { - const hermesClient = async () => { - const home = await createTestTempDirectory("rea-hermes-"); - const client = supportedClients(home, process.platform, {}).find( - ({ name }) => name === "hermes", - ); - if (client?.markerPath === undefined) - throw new Error("missing Hermes client"); - await mkdir(client.markerPath, { recursive: true }); - return { home, client }; - }; +const hermesClient = async () => { + const home = await createTestTempDirectory("rea-hermes-"); + const client = supportedClients(home, process.platform, {}).find( + ({ name }) => name === "hermes", + ); + if (client?.markerPath === undefined) + throw new Error("missing Hermes client"); + await mkdir(client.markerPath, { recursive: true }); + return { home, client }; +}; +describe("Hermes YAML shape diagnostics", () => { + it.each( + [ + "!!set {rea: null}", + "!!omap [{rea: {command: rea, args: [mcp]}}]", + "!!timestamp 2001-12-15", + "!!binary cmVh", + ].flatMap((value) => [ + { + original: `${value}\n`, + problem: "The configuration root must be an object", + }, + { + original: `mcp_servers: ${value}\n`, + problem: '"mcp_servers" must be an object', + }, + ]), + )( + "blocks setup and uninstall before changes for tagged non-record YAML: $original", + async ({ original, problem }) => { + const { home, client } = await hermesClient(); + const cursor = join(home, ".cursor", "mcp.json"); + const cursorOriginal = + '{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}'; + await mkdir(join(home, ".cursor")); + await writeFile(cursor, cursorOriginal); + await writeFile(client.configPath, original); + expect( + await inspectClientConfiguration(client, {}, command), + ).toMatchObject({ + status: "invalid", + remediation: `Configuration ${client.configPath} is malformed: ${problem}`, + }); + expect(await configureClientConfiguration(client, {}, command)).toEqual({ + status: "failed", + reason: "readback", + }); + const result = await runUninstall( + false, + systemUninstallHost(home, undefined, {}), + ); + expect(result.status).toBe("failed"); + expect(result.items).toContainEqual({ + name: "hermes", + status: "failed", + detail: `Configuration was not changed: ${problem}. Repair it, then rerun uninstall.`, + }); + expect(await readFile(client.configPath, "utf8")).toBe(original); + expect(await readFile(cursor, "utf8")).toBe(cursorOriginal); + await expect( + readFile(`${client.configPath}.rea.backup`, "utf8"), + ).rejects.toThrow(); + await expect(readFile(`${cursor}.rea.backup`, "utf8")).rejects.toThrow(); + }, + ); +}); + +describe("Hermes YAML registration", () => { it("writes mcp_servers, preserves comments, reads back, and uninstalls", async () => { const { home, client } = await hermesClient(); await writeFile( diff --git a/tests/boundary/filesystem/setupConfig.test.ts b/tests/boundary/filesystem/setupConfig.test.ts index 6819b69cd..79b489de5 100644 --- a/tests/boundary/filesystem/setupConfig.test.ts +++ b/tests/boundary/filesystem/setupConfig.test.ts @@ -5,7 +5,11 @@ import { describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { resolveClientConfigTransactionPath } from "../../../src/application/ClientConfigPath.js"; -import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js"; +import { readConfigurationFile } from "../../../src/application/ClientConfigurationFiles.js"; +import { + configureClientConfiguration, + inspectClientConfiguration, +} from "../../../src/application/SetupClientConfiguration.js"; import { PRODUCT_IDENTITY } from "../../../src/identity.js"; const pinnedPackage = PRODUCT_IDENTITY.registrationPackageSpecifier; @@ -330,3 +334,87 @@ describe("JSON client configuration migration and provider paths", () => { ).rejects.toThrow(); }); }); + +describe("JSON client configuration shape diagnostics", () => { + it.each([ + { + format: "json" as const, + original: + '{"__proto__":{"mcpServers":{"rea":{"command":"rea","args":["mcp"]}}}}', + serversKey: "mcpServers", + }, + { + format: "opencode" as const, + original: + '{"mcp":{"__proto__":{"servers":{"rea":{"type":"local","command":["rea","mcp"]}}}}}', + serversKey: "mcp", + }, + ])( + "registers an own server instead of trusting inherited $format settings", + async ({ format, original, serversKey }) => { + const directory = await createTestTempDirectory("rea-setup-prototype-"); + const configPath = join(directory, "mcp.json"); + const client = { + name: format === "json" ? "cursor" : "opencode", + configPath, + format, + }; + await writeFile(configPath, original); + expect( + await inspectClientConfiguration(client, {}, ["rea", "mcp"]), + ).toMatchObject({ status: "update" }); + expect( + await configureClientConfiguration(client, {}, ["rea", "mcp"]), + ).toMatchObject({ status: "configured" }); + const document = JSON.parse(await readFile(configPath, "utf8")); + expect(Object.hasOwn(document, serversKey)).toBe(true); + expect(Object.hasOwn(document[serversKey], "rea")).toBe(true); + expect(document[serversKey].rea.command).toEqual( + format === "json" ? "rea" : ["rea", "mcp"], + ); + const originalDocument = JSON.parse(original); + expect( + format === "json" ? document.__proto__ : document[serversKey].__proto__, + ).toEqual( + format === "json" + ? originalDocument.__proto__ + : originalDocument[serversKey].__proto__, + ); + expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original); + expect( + await inspectClientConfiguration(client, {}, ["rea", "mcp"]), + ).toMatchObject({ status: "already_current" }); + }, + ); + + it.each([ + ['{"mcpServers":null,"keep":1}', '"mcpServers" must be an object'], + ["[]", "The configuration root must be an object"], + ['{"mcpServers":"x"}', '"mcpServers" must be an object'], + ] as const)( + "names structural shape problems in setup remediation: %s", + async (original, problem) => { + const directory = await createTestTempDirectory("rea-setup-shape-"); + const configPath = join(directory, "mcp.json"); + const client = { + name: "cursor", + configPath, + format: "json" as const, + }; + await writeFile(configPath, original); + const read = await readConfigurationFile(configPath, "json"); + expect(read.ok).toBe(false); + if (read.ok) throw new Error("expected malformed configuration"); + expect(read.error.detail).toBe( + `Configuration ${configPath} is malformed: ${problem}`, + ); + expect( + await inspectClientConfiguration(client, {}, ["rea", "mcp"]), + ).toEqual({ + status: "invalid", + remediation: `Configuration ${configPath} is malformed: ${problem}`, + }); + expect(await readFile(configPath, "utf8")).toBe(original); + }, + ); +});