diff --git a/src/trust/project-trust.test.ts b/src/trust/project-trust.test.ts new file mode 100644 index 000000000..3cb4718bb --- /dev/null +++ b/src/trust/project-trust.test.ts @@ -0,0 +1,102 @@ +import { describe, test, expect } from "bun:test"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + loadProjectTrust, + projectTrustPath, + trustMcpServer, + trustPlugin, + type ProjectTrustStore, +} from "./project-trust.js"; +import type { MCPServerConfig } from "../config/settings.js"; + +async function withTempHome(fn: (home: string, cwd: string) => Promise): Promise { + const home = await mkdtemp(join(tmpdir(), "project-trust-test-")); + try { + await fn(home, "/repo/under/test"); + } finally { + await rm(home, { recursive: true, force: true }); + } +} + +const mcpServer = (name: string): MCPServerConfig => ({ name, command: "node", args: [name] }); + +describe("project trust store", () => { + test("concurrent plugin trust grants both survive without a corrupt file", async () => { + await withTempHome(async (home, cwd) => { + await Promise.all([ + trustPlugin(cwd, "/plugins/a", home), + trustPlugin(cwd, "/plugins/b", home), + ]); + + const store = await loadProjectTrust(cwd, home); + expect(store.trustedPluginPaths.sort()).toEqual(["/plugins/a", "/plugins/b"]); + + // File on disk must be complete, valid JSON — not truncated by an + // interleaved write. + const raw = await readFile(projectTrustPath(cwd, home), "utf8"); + expect(() => JSON.parse(raw)).not.toThrow(); + }); + }); + + test("concurrent plugin-trust and MCP-trust updates both survive", async () => { + await withTempHome(async (home, cwd) => { + const server1 = mcpServer("server-one"); + const server2 = mcpServer("server-two"); + + await Promise.all([ + trustPlugin(cwd, "/plugins/a", home), + trustMcpServer(cwd, server1, home), + trustPlugin(cwd, "/plugins/b", home), + trustMcpServer(cwd, server2, home), + ]); + + const store: ProjectTrustStore = await loadProjectTrust(cwd, home); + expect(store.trustedPluginPaths.sort()).toEqual(["/plugins/a", "/plugins/b"]); + expect(store.trustedMcpFingerprints).toHaveLength(2); + + const raw = await readFile(projectTrustPath(cwd, home), "utf8"); + expect(() => JSON.parse(raw)).not.toThrow(); + }); + }); + + test("many concurrent writers never drop a grant", async () => { + await withTempHome(async (home, cwd) => { + const pluginPaths = Array.from({ length: 20 }, (_, i) => `/plugins/p${i}`); + const servers = Array.from({ length: 20 }, (_, i) => mcpServer(`server-${i}`)); + + await Promise.all([ + ...pluginPaths.map((p) => trustPlugin(cwd, p, home)), + ...servers.map((s) => trustMcpServer(cwd, s, home)), + ]); + + const store = await loadProjectTrust(cwd, home); + expect(store.trustedPluginPaths).toHaveLength(pluginPaths.length); + expect(store.trustedMcpFingerprints).toHaveLength(servers.length); + + const raw = await readFile(projectTrustPath(cwd, home), "utf8"); + const parsed = JSON.parse(raw); + expect(Array.isArray(parsed.trustedPluginPaths)).toBe(true); + expect(Array.isArray(parsed.trustedMcpFingerprints)).toBe(true); + }); + }); + + test("no leftover .tmp file remains after concurrent writes settle", async () => { + await withTempHome(async (home, cwd) => { + await Promise.all([ + trustPlugin(cwd, "/plugins/a", home), + trustPlugin(cwd, "/plugins/b", home), + trustMcpServer(cwd, mcpServer("s"), home), + ]); + const path = projectTrustPath(cwd, home); + const tmp = `${path}.${process.pid}.tmp`; + const exists = await readFile(tmp, "utf8").then( + () => true, + () => false, + ); + expect(exists).toBe(false); + }); + }); +}); diff --git a/src/trust/project-trust.ts b/src/trust/project-trust.ts index 9449074e8..ee0911271 100644 --- a/src/trust/project-trust.ts +++ b/src/trust/project-trust.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { createHash } from "node:crypto"; @@ -148,11 +148,34 @@ export async function loadProjectTrust(cwd: string, home: string = homedir()): P return (await readProjectTrustStore(cwd, home)).store; } +// Written via temp-file + rename (same pattern as path-trust.ts / saveGlobalSettings) +// so a concurrent reader never sees a truncated or half-written store — a torn +// read would be indistinguishable from a corrupt file and wipe consent. async function saveProjectTrust(cwd: string, store: ProjectTrustStore, home: string = homedir()): Promise { const path = projectTrustPath(cwd, home); await mkdir(dirname(path), { recursive: true, mode: 0o700 }); const record = { repo: resolve(cwd), ...store }; - await writeFile(path, `${JSON.stringify(record, null, 2)}\n`, { mode: 0o600 }); + const tmp = `${path}.${process.pid}.tmp`; + await writeFile(tmp, `${JSON.stringify(record, null, 2)}\n`, { mode: 0o600 }); + await rename(tmp, path); +} + +// The grant helpers re-read the store immediately before writing, but two +// in-process mutations interleaving between that read and the write would +// still drop grants (e.g. a plugin-trust and an MCP-trust update landing at +// the same time). Chain them per trust-store path so each mutation sees the +// previous one's result. (Cross-process writers remain last-writer-wins of a +// complete file, same as path-trust.ts.) +const mutationQueues = new Map>(); + +function enqueueMutation(key: string, run: () => Promise): Promise { + const prior = mutationQueues.get(key) ?? Promise.resolve(); + const next = prior.then(run, run); + mutationQueues.set( + key, + next.catch(() => undefined), + ); + return next; } export function isPluginTrusted(store: ProjectTrustStore, pluginPath: string): boolean { @@ -165,13 +188,15 @@ export async function trustPlugin( pluginPath: string, home: string = homedir(), ): Promise { - const store = await loadProjectTrust(cwd, home); const abs = resolve(pluginPath); - if (!store.trustedPluginPaths.includes(abs)) { - store.trustedPluginPaths = [...store.trustedPluginPaths, abs]; - await saveProjectTrust(cwd, store, home); - } - return store; + return enqueueMutation(projectTrustPath(cwd, home), async () => { + const store = await loadProjectTrust(cwd, home); + if (!store.trustedPluginPaths.includes(abs)) { + store.trustedPluginPaths = [...store.trustedPluginPaths, abs]; + await saveProjectTrust(cwd, store, home); + } + return store; + }); } /** @@ -200,13 +225,15 @@ export async function trustMcpServer( server: MCPServerConfig, home: string = homedir(), ): Promise { - const store = await loadProjectTrust(cwd, home); const fp = mcpServerFingerprint(server); - if (!store.trustedMcpFingerprints.includes(fp)) { - store.trustedMcpFingerprints = [...store.trustedMcpFingerprints, fp]; - await saveProjectTrust(cwd, store, home); - } - return store; + return enqueueMutation(projectTrustPath(cwd, home), async () => { + const store = await loadProjectTrust(cwd, home); + if (!store.trustedMcpFingerprints.includes(fp)) { + store.trustedMcpFingerprints = [...store.trustedMcpFingerprints, fp]; + await saveProjectTrust(cwd, store, home); + } + return store; + }); } /**