fix(setup): preserve client config symlinks

This commit is contained in:
N0zoM1z0 2026-07-14 20:50:07 +00:00
parent 528c05e2fa
commit df87544a5d
7 changed files with 278 additions and 25 deletions

View File

@ -0,0 +1,53 @@
import { lstat, realpath } from "node:fs/promises";
interface ClientConfigPathStats {
readonly uid?: number;
isFile?(): boolean;
isSymbolicLink(): boolean;
}
interface ClientConfigPathFileSystem {
lstat(path: string): Promise<ClientConfigPathStats>;
realpath?(path: string): Promise<string>;
}
const systemFileSystem: ClientConfigPathFileSystem = { lstat, realpath };
/**
* Resolve an existing, user-owned config symlink to one stable transaction
* path. Missing regular paths remain eligible for first-run creation.
*/
export const resolveClientConfigTransactionPath = async (
requestedPath: string,
fileSystem: ClientConfigPathFileSystem = systemFileSystem,
): Promise<string | undefined> => {
let requestedStats: ClientConfigPathStats;
try {
requestedStats = await fileSystem.lstat(requestedPath);
} catch (cause: unknown) {
return isMissing(cause) ? requestedPath : undefined;
}
if (!requestedStats.isSymbolicLink()) return requestedPath;
const currentUid =
typeof process.getuid === "function" ? process.getuid() : undefined;
if (
currentUid === undefined ||
requestedStats.uid !== currentUid ||
fileSystem.realpath === undefined
)
return undefined;
try {
const canonicalPath = await fileSystem.realpath(requestedPath);
const targetStats = await fileSystem.lstat(canonicalPath);
return targetStats.isFile?.() === true && targetStats.uid === currentUid
? canonicalPath
: undefined;
} catch {
return undefined;
}
};
const isMissing = (cause: unknown): boolean =>
cause instanceof Error && "code" in cause && cause.code === "ENOENT";

View File

@ -3,7 +3,6 @@ import {
copyFile,
mkdir,
readFile,
rename,
rm,
writeFile,
} from "node:fs/promises";
@ -15,6 +14,7 @@ import { parse as parseToml, stringify as stringifyToml } from "smol-toml";
import { PRODUCT_IDENTITY } from "../identity.js";
import { supportsNodeVersion } from "../domain/runtimeVersion.js";
import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
import { runDoctor, systemDoctorHost } from "./Doctor.js";
import {
installLinuxHopper,
@ -49,7 +49,7 @@ export type ClientConfigurationResult =
}
| {
readonly status: "failed";
readonly reason: "backup" | "write" | "readback";
readonly reason: "path" | "backup" | "write" | "readback";
};
/**
* Effects required by the idempotent setup workflow.
@ -288,10 +288,15 @@ export const configureJsonClient = async (
"mcp",
],
): Promise<ClientConfigurationResult> => {
const transactionPath = await resolveClientConfigTransactionPath(
client.configPath,
);
if (transactionPath === undefined)
return { status: "failed", reason: "path" };
let document: Record<string, unknown> = {};
let original: string | undefined;
try {
original = await readFile(client.configPath, "utf8");
original = await readFile(transactionPath, "utf8");
document = parseObject(original);
} catch (cause: unknown) {
if (!isMissing(cause)) return { status: "failed", reason: "readback" };
@ -318,7 +323,7 @@ export const configureJsonClient = async (
if (original !== undefined) {
backupPath = `${client.configPath}.rea.backup`;
try {
await copyFile(client.configPath, backupPath);
await copyFile(transactionPath, backupPath);
} catch {
return { status: "failed", reason: "backup" };
}
@ -328,26 +333,26 @@ export const configureJsonClient = async (
[PRODUCT_IDENTITY.mcpServerKey]: desired,
};
const encoded = `${JSON.stringify(document, null, 2)}\n`;
const temporary = `${client.configPath}.rea.tmp`;
try {
await mkdir(dirname(client.configPath), { recursive: true });
await writeFile(temporary, encoded, { encoding: "utf8", mode: 0o600 });
await rename(temporary, client.configPath);
await writeFileAtomic(transactionPath, encoded, {
encoding: "utf8",
mode: 0o600,
});
} catch {
await rm(temporary, { force: true }).catch(() => undefined);
return { status: "failed", reason: "write" };
}
try {
const readback = parseObject(await readFile(client.configPath, "utf8"));
const readback = parseObject(await readFile(transactionPath, "utf8"));
const value = parseOptionalObject(readback.mcpServers)[
PRODUCT_IDENTITY.mcpServerKey
];
if (JSON.stringify(value) !== JSON.stringify(desired)) {
await restoreConfig(client.configPath, original);
await restoreConfig(transactionPath, original);
return { status: "failed", reason: "readback" };
}
} catch {
await restoreConfig(client.configPath, original);
await restoreConfig(transactionPath, original);
return { status: "failed", reason: "readback" };
}
return {
@ -367,10 +372,15 @@ export const configureTomlClient = async (
"mcp",
],
): Promise<ClientConfigurationResult> => {
const transactionPath = await resolveClientConfigTransactionPath(
client.configPath,
);
if (transactionPath === undefined)
return { status: "failed", reason: "path" };
let document: Record<string, unknown> = {};
let original: string | undefined;
try {
original = await readFile(client.configPath, "utf8");
original = await readFile(transactionPath, "utf8");
document = objectSchema.parse(parseToml(original));
} catch (cause: unknown) {
if (!isMissing(cause)) return { status: "failed", reason: "readback" };
@ -396,7 +406,7 @@ export const configureTomlClient = async (
const backupPath =
original === undefined ? undefined : `${client.configPath}.rea.backup`;
try {
if (backupPath !== undefined) await copyFile(client.configPath, backupPath);
if (backupPath !== undefined) await copyFile(transactionPath, backupPath);
} catch {
return { status: "failed", reason: "backup" };
}
@ -406,12 +416,12 @@ export const configureTomlClient = async (
};
try {
await mkdir(dirname(client.configPath), { recursive: true });
await writeFileAtomic(client.configPath, stringifyToml(document), {
await writeFileAtomic(transactionPath, stringifyToml(document), {
encoding: "utf8",
mode: 0o600,
});
const readback = objectSchema.parse(
parseToml(await readFile(client.configPath, "utf8")),
parseToml(await readFile(transactionPath, "utf8")),
);
if (
JSON.stringify(
@ -422,7 +432,7 @@ export const configureTomlClient = async (
)
throw new Error("TOML readback mismatch");
} catch {
await restoreConfig(client.configPath, original);
await restoreConfig(transactionPath, original);
return { status: "failed", reason: "readback" };
}
return {

View File

@ -5,8 +5,10 @@ import type {
} from "./Setup.js";
const failedConfigurationMessage = (
reason: "backup" | "write" | "readback",
reason: "path" | "backup" | "write" | "readback",
): string => {
if (reason === "path")
return "Agent configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun setup.";
if (reason === "backup")
return "Agent configuration could not be backed up, so no change was made. Check file permissions, then rerun setup.";
if (reason === "write")

View File

@ -1,4 +1,4 @@
import { copyFile, lstat, readFile, rm } from "node:fs/promises";
import { copyFile, lstat, readFile, realpath, rm } from "node:fs/promises";
import { homedir } from "node:os";
import { basename, isAbsolute, join } from "node:path";
@ -7,9 +7,12 @@ import writeFileAtomic from "write-file-atomic";
import { z } from "zod";
import { PRODUCT_IDENTITY } from "../identity.js";
import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
import { supportedClients, type SetupClient } from "./SupportedClients.js";
interface ManagedPathStats {
readonly uid?: number;
isFile?(): boolean;
isSymbolicLink(): boolean;
}
@ -19,6 +22,7 @@ export interface UninstallFileSystem {
copy(source: string, destination: string): Promise<void>;
writeText(path: string, contents: string): Promise<void>;
stat(path: string): Promise<ManagedPathStats>;
realpath?(path: string): Promise<string>;
remove(path: string): Promise<void>;
}
@ -28,6 +32,7 @@ const systemFileSystem: UninstallFileSystem = {
writeText: (path, contents) =>
writeFileAtomic(path, contents, { encoding: "utf8" }),
stat: (path) => lstat(path),
realpath: (path) => realpath(path),
remove: (path) => rm(path, { recursive: true }),
};
@ -104,9 +109,12 @@ const removeClient = async (
"skipped",
"This client has no documented local MCP configuration boundary.",
);
const resolved = await resolveUninstallConfigPath(client, fileSystem);
if (typeof resolved !== "string") return resolved;
const transactionPath = resolved;
let original: string;
try {
original = await fileSystem.readText(client.configPath);
original = await fileSystem.readText(transactionPath);
} catch (cause: unknown) {
return isMissing(cause)
? item(client.name, "skipped", "Configuration does not exist.")
@ -146,7 +154,7 @@ const removeClient = async (
document[key] = remaining;
const backupPath = `${client.configPath}.rea.backup`;
try {
await fileSystem.copy(client.configPath, backupPath);
await fileSystem.copy(transactionPath, backupPath);
} catch {
return item(
client.name,
@ -159,11 +167,11 @@ const removeClient = async (
client.format === "toml"
? stringifyToml(document)
: `${JSON.stringify(document, null, 2)}\n`;
await fileSystem.writeText(client.configPath, encoded);
await fileSystem.writeText(transactionPath, encoded);
const readback = objectSchema.parse(
client.format === "toml"
? parseToml(await fileSystem.readText(client.configPath))
: JSON.parse(await fileSystem.readText(client.configPath)),
? parseToml(await fileSystem.readText(transactionPath))
: JSON.parse(await fileSystem.readText(transactionPath)),
);
if (PRODUCT_IDENTITY.mcpServerKey in optionalObject(readback[key]))
throw new Error("registration readback mismatch");
@ -174,7 +182,7 @@ const removeClient = async (
);
} catch {
try {
await fileSystem.writeText(client.configPath, original);
await fileSystem.writeText(transactionPath, original);
} catch {
return item(
client.name,
@ -190,6 +198,26 @@ const removeClient = async (
}
};
const resolveUninstallConfigPath = async (
client: SetupClient,
fileSystem: UninstallFileSystem,
): Promise<string | UninstallItem> => {
const path = await resolveClientConfigTransactionPath(
client.configPath,
fileSystem.realpath === undefined
? { lstat: fileSystem.stat }
: { lstat: fileSystem.stat, realpath: fileSystem.realpath },
);
return (
path ??
item(
client.name,
"failed",
"Configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun uninstall.",
)
);
};
/** Ownership rule for persistent MCP registrations created by REA setup. */
const isOwnedRegistration = (value: unknown): boolean => {
const parsed = registrationSchema.safeParse(value);

View File

@ -274,6 +274,10 @@ describe("setup workflow", () => {
});
it.each([
[
"path",
"Agent configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun setup.",
],
[
"backup",
"Agent configuration could not be backed up, so no change was made. Check file permissions, then rerun setup.",

View File

@ -1,8 +1,16 @@
import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import {
lstat,
mkdtemp,
readFile,
rm,
symlink,
writeFile,
} from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { resolveClientConfigTransactionPath } from "../src/application/ClientConfigPath.js";
import { configureJsonClient } from "../src/application/Setup.js";
let directory: string | undefined;
@ -13,6 +21,25 @@ afterEach(async () => {
});
describe("JSON client configuration transaction", () => {
it("rejects a symlink target not owned by the current user", async () => {
if (typeof process.getuid !== "function") return;
const currentUid = process.getuid();
let statCalls = 0;
expect(
await resolveClientConfigTransactionPath("/config", {
lstat: () => {
statCalls += 1;
return Promise.resolve({
uid: statCalls === 1 ? currentUid : currentUid + 1,
isFile: () => statCalls > 1,
isSymbolicLink: () => statCalls === 1,
});
},
realpath: () => Promise.resolve("/target"),
}),
).toBeUndefined();
});
it("preserves existing keys, creates a backup, and reads back the MCP entry", async () => {
directory = await mkdtemp(join(tmpdir(), "rea-setup-"));
const configPath = join(directory, "mcp.json");
@ -34,6 +61,48 @@ describe("JSON client configuration transaction", () => {
});
});
it("updates a symlink target without replacing the JSON config symlink", async () => {
directory = await mkdtemp(join(tmpdir(), "rea-setup-"));
const targetPath = join(directory, "managed.json");
const configPath = join(directory, "mcp.json");
const original = '{"theme":"dark"}\n';
await writeFile(targetPath, original);
await symlink(targetPath, configPath);
expect(
await configureJsonClient({ name: "cursor", configPath }),
).toMatchObject({ status: "configured" });
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
expect(JSON.parse(await readFile(targetPath, "utf8"))).toMatchObject({
theme: "dark",
mcpServers: {
rea: {
command: "npx",
args: ["-y", "rea-agents", "mcp"],
},
},
});
expect(await configureJsonClient({ name: "cursor", configPath })).toEqual({
status: "unchanged",
});
});
it("fails before mutation when a JSON config symlink is dangling", async () => {
directory = await mkdtemp(join(tmpdir(), "rea-setup-"));
const configPath = join(directory, "mcp.json");
await symlink(join(directory, "missing.json"), configPath);
expect(await configureJsonClient({ name: "cursor", configPath })).toEqual({
status: "failed",
reason: "path",
});
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
await expect(
readFile(`${configPath}.rea.backup`, "utf8"),
).rejects.toThrow();
});
it("performs no write or second backup when configuration already matches", async () => {
directory = await mkdtemp(join(tmpdir(), "rea-setup-"));
const configPath = join(directory, "mcp.json");

View File

@ -4,7 +4,9 @@ import {
mkdtemp,
mkdir,
readFile,
realpath,
rm,
symlink,
writeFile,
} from "node:fs/promises";
import { tmpdir } from "node:os";
@ -92,6 +94,44 @@ describe("agent lifecycle", () => {
).toEqual({ status: "unchanged" });
});
it("updates a symlink target without replacing the TOML config symlink", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-toml-symlink-"));
roots.push(home);
const configPath = join(home, ".codex/config.toml");
const targetPath = join(home, "managed-config.toml");
const original = 'model = "gpt-5"\n';
await mkdir(dirname(configPath), { recursive: true });
await writeFile(targetPath, original);
await symlink(targetPath, configPath);
expect(
await configureTomlClient(
{ name: "codex", configPath, format: "toml" },
undefined,
["rea", "mcp"],
),
).toMatchObject({ status: "configured" });
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
expect(await readFile(targetPath, "utf8")).toContain("[mcp_servers.rea]");
});
it("fails before mutation when a TOML config symlink is dangling", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-toml-symlink-"));
roots.push(home);
const configPath = join(home, ".codex/config.toml");
await mkdir(dirname(configPath), { recursive: true });
await symlink(join(home, "missing-config.toml"), configPath);
expect(
await configureTomlClient({ name: "codex", configPath, format: "toml" }),
).toEqual({ status: "failed", reason: "path" });
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
await expect(
readFile(`${configPath}.rea.backup`, "utf8"),
).rejects.toThrow();
});
it("uninstalls only owned entries and refuses purge symlinks", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-uninstall-"));
roots.push(home);
@ -131,6 +171,52 @@ describe("agent lifecycle", () => {
);
});
it("removes an owned entry through a symlink without replacing it", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-uninstall-symlink-"));
roots.push(home);
const configPath = join(home, ".cursor/mcp.json");
const targetPath = join(home, "managed-mcp.json");
const original = JSON.stringify({
mcpServers: {
rea: { command: "rea", args: ["mcp"] },
other: { command: "other" },
},
});
await mkdir(dirname(configPath), { recursive: true });
await writeFile(targetPath, original);
await symlink(targetPath, configPath);
expect((await runUninstall(false, systemUninstallHost(home))).status).toBe(
"complete",
);
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
expect(JSON.parse(await readFile(targetPath, "utf8"))).toEqual({
mcpServers: { other: { command: "other" } },
});
});
it("fails before mutation when an uninstall config symlink is dangling", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-uninstall-symlink-"));
roots.push(home);
const configPath = join(home, ".cursor/mcp.json");
await mkdir(dirname(configPath), { recursive: true });
await symlink(join(home, "missing-mcp.json"), configPath);
const result = await runUninstall(false, systemUninstallHost(home));
expect(result.status).toBe("failed");
expect(result.items).toContainEqual({
name: "cursor",
status: "failed",
detail:
"Configuration path could not be safely verified. Check its permissions and, if it is a symbolic link, verify that the link resolves to a regular file owned by the current user, then rerun uninstall.",
});
expect((await lstat(configPath)).isSymbolicLink()).toBe(true);
await expect(
readFile(`${configPath}.rea.backup`, "utf8"),
).rejects.toThrow();
});
it("fails closed on malformed client configuration", async () => {
const home = await mkdtemp(join(tmpdir(), "rea-uninstall-bad-"));
roots.push(home);
@ -228,6 +314,7 @@ const testFileSystem: UninstallFileSystem = {
copy: (source, destination) => copyFile(source, destination),
writeText: (path, contents) => writeFile(path, contents),
stat: (path) => lstat(path),
realpath: (path) => realpath(path),
remove: (path) => rm(path, { recursive: true }),
};