shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(setup): name invalid client config shape errors clearly (#2157) 

* fix(setup): name invalid client config shape errors clearly

Setup was dumping raw Zod issues for non-object roots or server tables,
and uninstall always claimed the file was not valid JSON. Report the
structural problem instead, and reserve parse failures for real syntax errors.

Fixes #2152

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(setup): preserve client configuration record validation

Keep descriptive shape errors while retaining Zod record validation and its own-property copy. Reject tagged YAML objects before setup or uninstall writes and prevent inherited JSONC properties from fabricating client registrations.

Add production-boundary regressions for tagged YAML roots and server tables, plus inherited Cursor and OpenCode settings. All 91 focused tests, npm run check, verify:test-discovery, verify:package, and npm pack --dry-run passed.

---------

Co-authored-by: Beqa Ksovreli <beqa@Beqas-Macbook-Pro-2.local>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
BeqaKs 2026-10-12 19:24:24 +04:00 committed by GitHub
parent 7711b00005
commit b1bf4a70d4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 213 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/application/ClientConfigurationDocument.shape.test.ts Normal file
View File

@ -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,
);
});
});

26
src/application/ClientConfigurationDocument.ts
View File

@ -107,18 +107,21 @@ const parseDocument = (
format: ClientConfigurationFormat,
): Record<string, unknown> => {
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<string, unknown>;
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 =

5
tests/boundary/filesystem/clientLifecycle.test.ts
View File

@ -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();
},
);

89
tests/boundary/filesystem/hermesClientConfiguration.test.ts
View File

@ -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(

90
tests/boundary/filesystem/setupConfig.test.ts
View File

@ -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);
},
);
});