From ee35e75e2165b0fa4a93b4b206f929cceb33b0c5 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 16:29:29 -0400 Subject: [PATCH 01/13] fix(audit): bind MCP remediation commit --- src/audit/fix.test.ts | 363 +++++++++-- src/audit/fix.ts | 282 +++++++- src/audit/read-only.test.ts | 2 +- .../report-persistence-adversarial.test.ts | 31 +- src/audit/report-persistence.ts | 324 +++++++++- src/audit/safe-openat.ts | 602 ++++++++++++++++++ src/audit/static.ts | 13 + 7 files changed, 1536 insertions(+), 81 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 18a65cd..ded66e5 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -1,24 +1,29 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; -import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + rename, + rm, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { buildCompiledCliFixture, type CompiledCliFixture, } from "../../test/compiled-cli-fixture"; -import { LEGACY_MANAGED_MUTATION_ENV } from "../legacy-mutation-policy"; import { saveManagedState } from "../manage"; -import { facultStateDir } from "../paths"; import { runAuditFix } from "./fix"; import { persistAuditReport } from "./report-persistence"; import { evaluateStaticAudit } from "./static"; const ORIGINAL_HOME = process.env.HOME; -const ORIGINAL_LEGACY_MUTATION_ENV = process.env[LEGACY_MANAGED_MUTATION_ENV]; let tempHome: string | null = null; let tempReportRoot: string | null = null; let evaluation: Awaited> | null = null; -let legacyPath: string | null = null; let managedHome: string | null = null; let managedReportPath: string | null = null; let managedReportRoot: string | null = null; @@ -116,9 +121,47 @@ async function makeFixBase() { }, }, }); +} - legacyPath = join(facultStateDir(tempHome), "audit", "static-latest.json"); - await writeJson(legacyPath, { legacy: true }); +async function makeAuthorizedFixture(marker = "fixture_secret_1234567890") { + const home = await makeTempHome(); + const root = join(home, ".ai"); + const trackedPath = join(root, "mcp", "servers.json"); + const localPath = join(root, "mcp", "servers.local.json"); + await writeJson(trackedPath, { + servers: { + github: { + command: "fixture-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, + }, + }, + }); + const audit = await evaluateStaticAudit({ + argv: [], + cwd: home, + from: [root], + homeDir: home, + includeConfigFrom: false, + minSeverity: "high", + }); + const reportRoot = await mkdtemp(join(tmpdir(), "fclt-audit-fix-exact-")); + const exactReportPath = await persistAuditReport({ + ...audit, + mode: "static", + reportRoot, + }); + return { + cleanup: async () => { + await rm(home, { force: true, recursive: true }); + await rm(reportRoot, { force: true, recursive: true }); + }, + exactReportPath, + home, + localPath, + reportRoot, + root, + trackedPath, + }; } beforeAll(async () => { @@ -201,11 +244,9 @@ afterAll(async () => { managedReportPath = null; managedRoot = null; evaluation = null; - legacyPath = null; reportPath = null; rootDir = null; process.env.HOME = ORIGINAL_HOME; - process.env[LEGACY_MANAGED_MUTATION_ENV] = ORIGINAL_LEGACY_MUTATION_ENV; }); afterAll(async () => { @@ -230,22 +271,20 @@ describe("audit fix", () => { expect(await Bun.file(reportPath!).exists()).toBe(true); }); - it("fails closed before deprecated managed-output sync", async () => { - process.env[LEGACY_MANAGED_MUTATION_ENV] = undefined; + it("remediates only canonical MCP files and preserves managed tool bytes", async () => { const managedToolPath = join(managedHome!, ".codex", "mcp.json"); const managedToolBefore = await Bun.file(managedToolPath).text(); - await expect( - runAuditFix({ - argv: ["mcp:github", "--report", managedReportPath!, "--yes"], - cwd: managedHome!, - homeDir: managedHome!, - }) - ).rejects.toThrow("audit fix mutation is temporarily disabled"); + const result = await runAuditFix({ + argv: ["mcp:github", "--report", managedReportPath!, "--yes"], + cwd: managedHome!, + homeDir: managedHome!, + }); + expect(result.fixed).toBe(1); expect(await Bun.file(managedToolPath).text()).toBe(managedToolBefore); expect( await Bun.file(join(managedRoot!, "mcp", "servers.local.json")).exists() - ).toBe(false); + ).toBe(true); }); it("reports exact matches in dry-run mode without changing MCP state", async () => { @@ -264,23 +303,81 @@ describe("audit fix", () => { expect(await Bun.file(localPath).exists()).toBe(false); }); - it("disables inline MCP mutation with zero writes", async () => { - process.env[LEGACY_MANAGED_MUTATION_ENV] = undefined; - const trackedPath = join(rootDir!, "mcp", "servers.json"); - const localPath = join(rootDir!, "mcp", "servers.local.json"); - const trackedBefore = await Bun.file(trackedPath).text(); - - await expect( - runAuditFix({ - argv: ["mcp:github", "--report", reportPath!, "--yes"], - cwd: tempHome!, - homeDir: tempHome!, - }) - ).rejects.toThrow("audit fix mutation is temporarily disabled"); + it("rejects a receipt binding that does not match its exact finding", async () => { + const fixture = await makeAuthorizedFixture(); + try { + const envelope = (await Bun.file(fixture.exactReportPath).json()) as { + receipt: { + remediationBindings: Array<{ envKey: string }>; + }; + }; + const binding = envelope.receipt.remediationBindings[0]; + expect(binding).toBeDefined(); + if (!binding) { + throw new Error("Expected an exact remediation binding fixture"); + } + binding.envKey = "DIFFERENT_TOKEN"; + await Bun.write( + fixture.exactReportPath, + `${JSON.stringify(envelope, null, 2)}\n` + ); + await chmod(fixture.exactReportPath, 0o600); + await expect( + runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow("do not match the report"); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + } finally { + await fixture.cleanup(); + } + }); - expect(await Bun.file(trackedPath).text()).toBe(trackedBefore); - expect(await Bun.file(localPath).exists()).toBe(false); - expect(await Bun.file(legacyPath!).json()).toEqual({ legacy: true }); + it("moves the exact secret, preserves reports, and leaves a clean future audit", async () => { + const fixture = await makeAuthorizedFixture(); + try { + const reportBefore = await readFile(fixture.exactReportPath, "utf8"); + const result = await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + expect(result.trackedPath).toBe(fixture.trackedPath); + expect(result.localPath).toBe(fixture.localPath); + expect(await Bun.file(fixture.trackedPath).json()).toEqual({ + servers: { github: { command: "fixture-command" } }, + }); + expect(await Bun.file(fixture.localPath).json()).toEqual({ + servers: { + github: { + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }); + expect(await readFile(fixture.exactReportPath, "utf8")).toBe( + reportBefore + ); + const future = await evaluateStaticAudit({ + argv: [], + cwd: fixture.home, + from: [fixture.root], + homeDir: fixture.home, + includeConfigFrom: false, + minSeverity: "high", + }); + expect( + future.report.results.flatMap((entry) => entry.findings) + ).not.toContainEqual( + expect.objectContaining({ ruleId: "mcp-env-inline-secret" }) + ); + } finally { + await fixture.cleanup(); + } }); it("keeps the CLI dry-run path zero-write", async () => { @@ -300,11 +397,10 @@ describe("audit fix", () => { expect(await Bun.file(localPath).exists()).toBe(false); }); - it("makes the CLI --yes path reject before any MCP or tool write", async () => { + it("rejects replay of a report after its authorized source changed", async () => { const trackedPath = join(managedRoot!, "mcp", "servers.json"); const localPath = join(managedRoot!, "mcp", "servers.local.json"); const managedToolPath = join(managedHome!, ".codex", "mcp.json"); - const trackedBefore = await Bun.file(trackedPath).text(); const managedToolBefore = await Bun.file(managedToolPath).text(); const result = await runFixCli( ["mcp:github", "--report", managedReportPath!, "--yes"], @@ -313,11 +409,8 @@ describe("audit fix", () => { ); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain( - "audit fix mutation is temporarily disabled" - ); - expect(await Bun.file(trackedPath).text()).toBe(trackedBefore); - expect(await Bun.file(localPath).exists()).toBe(false); + expect(result.stderr).toContain("Audit evaluated context changed"); + expect(await Bun.file(localPath).exists()).toBe(true); expect(await Bun.file(managedToolPath).text()).toBe(managedToolBefore); }); @@ -366,7 +459,7 @@ describe("audit fix", () => { cwd: targetHome, homeDir: targetHome, }) - ).rejects.toThrow("audit fix mutation is temporarily disabled"); + ).rejects.toThrow("does not match the report-authorized canonical root"); expect(await Bun.file(targetTrackedPath).text()).toBe(targetBefore); expect( await Bun.file(join(targetRoot, "mcp", "servers.local.json")).exists() @@ -382,25 +475,27 @@ describe("audit fix", () => { const trackedPath = join(rootDir!, "mcp", "servers.json"); const original = await Bun.file(trackedPath).text(); try { - await writeJson(trackedPath, { - servers: { - github: { - command: "drifted-command", - env: { - GITHUB_PERSONAL_ACCESS_TOKEN: "drifted_fixture_value_1234567890", - }, - }, - }, - }); - const drifted = await Bun.file(trackedPath).text(); - await expect( runAuditFix({ argv: ["mcp:github", "--report", reportPath!, "--yes"], + beforeSourceValidation: async () => { + await writeJson(trackedPath, { + servers: { + github: { + command: "drifted-command", + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: + "drifted_fixture_value_1234567890", + }, + }, + }, + }); + }, cwd: tempHome!, homeDir: tempHome!, }) ).rejects.toThrow("Audit evaluated context changed"); + const drifted = await Bun.file(trackedPath).text(); expect(await Bun.file(trackedPath).text()).toBe(drifted); expect( @@ -410,4 +505,162 @@ describe("audit fix", () => { await Bun.write(trackedPath, original); } }); + + it("does not rewrite an unchanged replacement at the before-open seam", async () => { + const fixture = await makeAuthorizedFixture(); + const movedPath = `${fixture.trackedPath}.moved`; + try { + const replacement = `${JSON.stringify( + { + servers: { + github: { + command: "replacement-command", + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "replacement_secret_1234567890", + }, + }, + }, + }, + null, + 2 + )}\n`; + await expect( + runAuditFix({ + afterReportValidation: async () => { + await rename(fixture.trackedPath, movedPath); + await Bun.write(fixture.trackedPath, replacement); + }, + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow(); + expect(await Bun.file(fixture.trackedPath).text()).toBe(replacement); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + + it("fails closed on an ancestor swap after the directory is bound", async () => { + const fixture = await makeAuthorizedFixture(); + const mcpRoot = join(fixture.root, "mcp"); + const movedRoot = join(fixture.root, "mcp.moved"); + try { + await expect( + runAuditFix({ + afterBoundOpen: async () => { + await rename(mcpRoot, movedRoot); + await writeJson(join(mcpRoot, "servers.json"), { + servers: { + github: { + command: "replacement-command", + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: + "replacement_secret_1234567890", + }, + }, + }, + }); + }, + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow(); + expect(await Bun.file(join(mcpRoot, "servers.json")).json()).toEqual( + expect.objectContaining({ + servers: expect.objectContaining({ + github: expect.objectContaining({ + command: "replacement-command", + }), + }), + }) + ); + expect( + await Bun.file(join(movedRoot, "servers.local.json")).exists() + ).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + + it("rolls back the destination when the final source commit is interrupted", async () => { + const fixture = await makeAuthorizedFixture(); + try { + const trackedBefore = await Bun.file(fixture.trackedPath).text(); + await expect( + runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + beforeSourceCommit: () => { + throw new Error("injected final commit interruption"); + }, + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow("injected final commit interruption"); + expect(await Bun.file(fixture.trackedPath).text()).toBe(trackedBefore); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + expect( + (await readdir(join(fixture.root, "mcp"))).some((name) => + name.endsWith(".tmp") + ) + ).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + + it("fails closed on source permission drift after bound open", async () => { + const fixture = await makeAuthorizedFixture(); + try { + const trackedBefore = await Bun.file(fixture.trackedPath).text(); + await expect( + runAuditFix({ + afterBoundOpen: async () => { + await chmod(fixture.trackedPath, 0o666); + }, + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow(); + expect(await Bun.file(fixture.trackedPath).text()).toBe(trackedBefore); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + + it("fails closed without blocking when a FIFO replaces the bound source", async () => { + const fixture = await makeAuthorizedFixture(); + const movedPath = `${fixture.trackedPath}.moved`; + try { + const attempt = runAuditFix({ + afterReportValidation: async () => { + await rename(fixture.trackedPath, movedPath); + const proc = Bun.spawn(["mkfifo", fixture.trackedPath], { + stderr: "pipe", + stdout: "pipe", + }); + expect(await proc.exited).toBe(0); + }, + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + await expect( + Promise.race([ + attempt, + Bun.sleep(1000).then(() => { + throw new Error("audit fix blocked on FIFO replacement"); + }), + ]) + ).rejects.toThrow(); + expect((await lstat(fixture.trackedPath)).isFIFO()).toBe(true); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + } finally { + await fixture.cleanup(); + } + }); }); diff --git a/src/audit/fix.ts b/src/audit/fix.ts index 301e776..d37b049 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -1,12 +1,28 @@ -import { basename } from "node:path"; +import { closeSync } from "node:fs"; +import { homedir } from "node:os"; +import { basename, dirname, join, normalize } from "node:path"; +import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; +import { facultContextRootDir } from "../paths"; +import { parseJsonLenient } from "../util/json"; import type { AgentAuditReport } from "./agent"; -import { loadVerifiedAuditReport } from "./report-persistence"; +import { + type AuditMcpRemediationBinding, + auditFindingIdentity, + loadVerifiedAuditReportEnvelope, + type VerifiedAuditReportEnvelope, +} from "./report-persistence"; +import { + openBoundPrivateSubdirectory, + replaceBoundPrivateFilePairAt, +} from "./safe-openat"; +import { validateAuditSourceSnapshot } from "./source-provenance"; import type { AuditFinding, AuditItemResult, StaticAuditReport } from "./types"; type AuditFixSource = "static" | "agent" | "combined"; const RULE_ID_PREFIX_RE = /^(static|agent):/; const INLINE_SECRET_RULE_ID = "mcp-env-inline-secret"; const ARG_VALUE_SPLIT_RE = /=(.*)/s; +const MAX_MCP_CONFIG_BYTES = 2 * 1024 * 1024; interface AuditFixArgs { all: boolean; @@ -337,11 +353,144 @@ function selectFixableFindings(args: { ); } -export const AUDIT_FIX_MUTATION_DISABLED_MESSAGE = - "audit fix mutation is temporarily disabled until exact reports bind the MCP source and destination through the mutation commit; use --dry-run to inspect matches and remediate manually"; +function isPlainObject(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +function transformBoundMcpConfigs(args: { + bindings: AuditMcpRemediationBinding[]; + destinationContents: string | null; + sourceContents: string; +}): { destinationContents: string; sourceContents: string } { + const sourceRoot = parseJsonLenient(args.sourceContents); + if (!isPlainObject(sourceRoot)) { + throw new Error("Bound MCP source is not a JSON object"); + } + const sourceServers = extractServersObject(sourceRoot); + if (!sourceServers) { + throw new Error("Bound MCP source has no servers object"); + } + const destinationRoot: Record = args.destinationContents + ? (() => { + const parsed = parseJsonLenient(args.destinationContents); + if (!(isPlainObject(parsed) && extractServersObject(parsed))) { + throw new Error("Bound MCP destination has no servers object"); + } + return parsed; + })() + : { servers: {} }; + const destinationServers = extractServersObject(destinationRoot)!; + + for (const binding of args.bindings) { + const sourceServer = sourceServers[binding.serverName]; + if (!isPlainObject(sourceServer)) { + throw new Error( + `Bound MCP source server is missing: ${binding.serverName}` + ); + } + const sourceEnv = sourceServer.env; + if (!isPlainObject(sourceEnv)) { + throw new Error(`Bound MCP source env is missing: ${binding.serverName}`); + } + const secret = sourceEnv[binding.envKey]; + if (!isInlineMcpSecretValue(secret)) { + throw new Error( + `Bound MCP source secret changed: ${binding.serverName}:${binding.envKey}` + ); + } + + const currentDestinationServer = destinationServers[binding.serverName]; + if ( + currentDestinationServer !== undefined && + !isPlainObject(currentDestinationServer) + ) { + throw new Error( + `Bound MCP destination server is invalid: ${binding.serverName}` + ); + } + const destinationServer = currentDestinationServer ?? {}; + const currentDestinationEnv = destinationServer.env; + if ( + currentDestinationEnv !== undefined && + !isPlainObject(currentDestinationEnv) + ) { + throw new Error( + `Bound MCP destination env is invalid: ${binding.serverName}` + ); + } + if ( + currentDestinationEnv && + Object.hasOwn(currentDestinationEnv, binding.envKey) && + currentDestinationEnv[binding.envKey] !== secret + ) { + throw new Error( + `Bound MCP destination secret conflicts: ${binding.serverName}:${binding.envKey}` + ); + } + destinationServer.env = { + ...(currentDestinationEnv ?? {}), + [binding.envKey]: secret, + }; + destinationServers[binding.serverName] = destinationServer; + Reflect.deleteProperty(sourceEnv, binding.envKey); + if (Object.keys(sourceEnv).length === 0) { + Reflect.deleteProperty(sourceServer, "env"); + } + } + + return { + destinationContents: `${JSON.stringify(destinationRoot, null, 2)}\n`, + sourceContents: `${JSON.stringify(sourceRoot, null, 2)}\n`, + }; +} + +function exactStaticBindings(args: { + envelope: VerifiedAuditReportEnvelope; + selections: FindingSelection[]; +}): AuditMcpRemediationBinding[] { + const byIdentity = new Map( + args.envelope.receipt.remediationBindings.map((binding) => [ + binding.findingIdentity, + binding, + ]) + ); + const bindings = args.selections.map((selection) => { + const identity = auditFindingIdentity(selection); + const binding = byIdentity.get(identity); + if (!binding) { + throw new Error( + "Selected finding has no exact report-authorized remediation binding" + ); + } + return binding; + }); + const first = bindings[0]; + if ( + !first || + bindings.some( + (binding) => + binding.canonicalRootPath !== first.canonicalRootPath || + binding.sourcePath !== first.sourcePath || + binding.destinationPath !== first.destinationPath + ) + ) { + throw new Error( + "Selected findings do not share one exact bound MCP transaction" + ); + } + return bindings; +} export async function runAuditFix(args: { + /** @internal Adversarial test hook; production callers must not set this. */ + afterBoundOpen?: () => Promise; + /** @internal Adversarial test hook; production callers must not set this. */ + afterReportValidation?: () => Promise; argv: string[]; + /** @internal Adversarial test hook; production callers must not set this. */ + beforeSourceCommit?: () => Promise; + /** @internal Adversarial test hook; production callers must not set this. */ + beforeSourceValidation?: () => Promise; homeDir?: string; cwd?: string; }): Promise<{ @@ -358,15 +507,23 @@ export async function runAuditFix(args: { let staticReport: StaticAuditReport | null = null; let agentReport: AgentAuditReport | null = null; + let staticEnvelope: VerifiedAuditReportEnvelope | null = + null; for (const reportPath of parsed.reportPaths) { - const report = await loadVerifiedAuditReport< + const envelope = await loadVerifiedAuditReportEnvelope< StaticAuditReport | AgentAuditReport - >({ reportPath }); + >({ + beforeSourceValidation: args.beforeSourceValidation, + reportPath, + }); + const report = envelope.report; if (report.mode === "static") { if (staticReport) { throw new Error("Only one exact static audit report may be supplied"); } staticReport = report; + staticEnvelope = + envelope as VerifiedAuditReportEnvelope; } else { if (agentReport) { throw new Error("Only one exact agent audit report may be supplied"); @@ -422,21 +579,122 @@ export async function runAuditFix(args: { if (!parsed.yes) { throw new Error("audit fix mutation requires explicit --yes approval"); } - throw new Error(AUDIT_FIX_MUTATION_DISABLED_MESSAGE); + if (source === "agent" || !staticReport || !staticEnvelope) { + throw new Error( + "Audit fix mutation requires an exact static report with remediation bindings" + ); + } + + const staticSelections = selectFixableFindings({ + results: staticReport.results, + filters: parsed, + }); + if (staticSelections.length === 0) { + throw new Error( + "No exact static finding authorizes the requested MCP mutation" + ); + } + const bindings = exactStaticBindings({ + envelope: staticEnvelope, + selections: staticSelections, + }); + const firstBinding = bindings[0]!; + const currentRoot = normalize( + facultContextRootDir({ + home: args.homeDir ?? homedir(), + cwd: args.cwd, + }) + ); + if (currentRoot !== firstBinding.canonicalRootPath) { + throw new Error( + "Current audit fix scope does not match the report-authorized canonical root" + ); + } + + const snapshot = staticEnvelope.receipt.sourceSnapshot; + const rootIdentity = snapshot.evaluatedDirectories.find( + (identity) => identity.path === firstBinding.canonicalRootPath + ); + const mcpRoot = dirname(firstBinding.sourcePath); + const directoryIdentity = snapshot.evaluatedDirectories.find( + (identity) => identity.path === mcpRoot + ); + const sourceIdentity = snapshot.evaluatedFiles.find( + (identity) => identity.path === firstBinding.sourcePath + ); + const destinationIdentity = + snapshot.evaluatedFiles.find( + (identity) => identity.path === firstBinding.destinationPath + ) ?? null; + if ( + !(rootIdentity && directoryIdentity && sourceIdentity) || + mcpRoot !== join(firstBinding.canonicalRootPath, "mcp") + ) { + throw new Error("Audit remediation binding has incomplete object identity"); + } + + if (args.afterReportValidation) { + await args.afterReportValidation(); + } + const bound = openBoundPrivateSubdirectory({ + directoryIdentity, + directoryName: "mcp", + rootIdentity, + rootPath: firstBinding.canonicalRootPath, + }); + try { + if (args.afterBoundOpen) { + await args.afterBoundOpen(); + } + await validateAuditSourceSnapshot(snapshot); + await replaceBoundPrivateFilePairAt({ + beforeSourceCommit: args.beforeSourceCommit, + destinationIdentity, + destinationName: basename(firstBinding.destinationPath), + directoryFd: bound.directoryFd, + directoryIdentity, + maxBytes: MAX_MCP_CONFIG_BYTES, + rootFd: bound.rootFd, + rootIdentity, + rootPath: firstBinding.canonicalRootPath, + sourceIdentity, + sourceName: basename(firstBinding.sourcePath), + transform: (sourceContents, destinationContents) => + transformBoundMcpConfigs({ + bindings, + destinationContents, + sourceContents, + }), + }); + } finally { + closeSync(bound.directoryFd); + closeSync(bound.rootFd); + } + + return { + fixed: bindings.length, + localPath: firstBinding.destinationPath, + matched: selections.length, + riskyManagedOutputs: [], + skipped: [], + source, + syncedTools: [], + trackedPath: firstBinding.sourcePath, + }; } function printHelp() { - console.log(`fclt audit fix — inspect fixable audit findings + console.log(`fclt audit fix — remediate exact report-authorized findings Usage: fclt audit fix --report --dry-run - fclt audit fix --item --report [--path ] [--source ] --dry-run - fclt audit fix --all --report [--report ] [--source ] --dry-run + fclt audit fix --item --report [--path ] [--source ] --yes + fclt audit fix --all --report [--report ] [--source ] --yes Notes: - Dry-run inspection remains available for exact persisted reports. - - Automated MCP mutation is temporarily disabled and --yes fails closed. - - Mutation remains disabled until exact source/destination objects stay descriptor-bound through commit. + - Mutation requires explicit --yes approval and an exact static remediation binding. + - Source and destination objects remain descriptor-bound and are revalidated at commit. - Requires a fresh, content-hashed report-and-receipt envelope created by --report-root. - Legacy static-latest.json and agent-latest.json files never authorize mutation. `); diff --git a/src/audit/read-only.test.ts b/src/audit/read-only.test.ts index 8c93782..cbf55a5 100644 --- a/src/audit/read-only.test.ts +++ b/src/audit/read-only.test.ts @@ -252,7 +252,7 @@ describe("read-only audit boundary", () => { const reportPath = await exactReportPath(reportRoot); const envelope = JSON.parse(await readFile(reportPath, "utf8")); expect(envelope.report).toEqual(report); - expect(envelope.receipt.reportRevision).toBe(10); + expect(envelope.receipt.reportRevision).toBe(11); await expect(loadVerifiedAuditReport({ reportPath })).resolves.toEqual( report ); diff --git a/src/audit/report-persistence-adversarial.test.ts b/src/audit/report-persistence-adversarial.test.ts index 0424aaf..a1bec99 100644 --- a/src/audit/report-persistence-adversarial.test.ts +++ b/src/audit/report-persistence-adversarial.test.ts @@ -17,7 +17,9 @@ import { import { tmpdir } from "node:os"; import { basename, join } from "node:path"; import { + AUDIT_READ_ONLY_CAPABILITY, AUDIT_REPORT_MAX_ENVELOPE_BYTES, + AUDIT_REPORT_REVISION, auditReportRootPermissionsAreSafe, loadVerifiedAuditReport, persistAuditReport, @@ -59,14 +61,35 @@ async function fixture() { ).toISOString(); const report = { mode: "static" as const, results: [], timestamp }; const reportContents = `${JSON.stringify(report, null, 2)}\n`; + const sourceSnapshot = tracker.snapshot(); + const receipt = { + schemaVersion: 6, + capability: AUDIT_READ_ONLY_CAPABILITY, + reportRevision: AUDIT_REPORT_REVISION, + mode: "static", + persistedAt: timestamp, + reportTimestamp: timestamp, + reportSha256: createHash("sha256").update(reportContents).digest("hex"), + sourceIdentitySha256: createHash("sha256") + .update(stableJson(sourceSnapshot)) + .digest("hex"), + sourceSnapshot, + findingIdentities: [], + remediationBindings: [], + } as const; + const envelopeContents = `${JSON.stringify( + { schemaVersion: 1, receipt, report }, + null, + 2 + )}\n`; const reportFileName = `static-${createHash("sha256") - .update(reportContents) + .update(envelopeContents) .digest("hex")}.json`; return { evaluation: { auditedRoots: [sourceRoot], report, - sourceSnapshot: tracker.snapshot(), + sourceSnapshot, }, reportFileName, sourceRoot, @@ -472,8 +495,8 @@ describe("adversarial audit report persistence", () => { null, 2 ).replace( - '"receipt": {\n "schemaVersion": 5,', - '"receipt": {\n "schemaVersion": 0,\n "schemaVersion": 5,' + '"receipt": {\n "schemaVersion": 6,', + '"receipt": {\n "schemaVersion": 0,\n "schemaVersion": 6,' )}\n`; await writeFile(reportPath, duplicateReceiptKey); await chmod(reportPath, 0o600); diff --git a/src/audit/report-persistence.ts b/src/audit/report-persistence.ts index e61a5c4..806680a 100644 --- a/src/audit/report-persistence.ts +++ b/src/audit/report-persistence.ts @@ -32,12 +32,13 @@ export type AuditReportMode = "agent" | "static"; export interface AuditEvaluation { auditedRoots: string[]; + remediationBindings?: AuditMcpRemediationBinding[]; report: TReport; sourceSnapshot: AuditSourceSnapshot; } export const AUDIT_READ_ONLY_CAPABILITY = "audit-read-only-v1"; -export const AUDIT_REPORT_REVISION = 10; +export const AUDIT_REPORT_REVISION = 11; export const AUDIT_REPORT_MAX_AGE_MS = 15 * 60 * 1000; export const AUDIT_REPORT_MAX_ENVELOPE_BYTES = 16 * 1024 * 1024; @@ -54,6 +55,7 @@ const RECEIPT_KEYS = [ "sourceIdentitySha256", "sourceSnapshot", "findingIdentities", + "remediationBindings", ] as const; function permissionBits(mode: number): number { @@ -83,7 +85,7 @@ interface PathSemantics { } export interface AuditReportReceipt { - schemaVersion: 5; + schemaVersion: 6; capability: typeof AUDIT_READ_ONLY_CAPABILITY; reportRevision: typeof AUDIT_REPORT_REVISION; mode: AuditReportMode; @@ -93,6 +95,17 @@ export interface AuditReportReceipt { sourceIdentitySha256: string; sourceSnapshot: AuditSourceSnapshot; findingIdentities: string[]; + remediationBindings: AuditMcpRemediationBinding[]; +} + +export interface AuditMcpRemediationBinding { + canonicalRootPath: string; + destinationPath: string; + envKey: string; + findingIdentity: string; + kind: "mcp-inline-secret"; + serverName: string; + sourcePath: string; } interface PersistedAuditEnvelope { @@ -156,6 +169,259 @@ export function auditFindingIdentity(args: { ); } +const REMEDIATION_BINDING_KEYS = [ + "canonicalRootPath", + "destinationPath", + "envKey", + "findingIdentity", + "kind", + "serverName", + "sourcePath", +] as const; + +function parseInlineSecretLocation(location: string): { + configPath: string; + envKey: string; + serverName: string; +} | null { + const envMarker = location.lastIndexOf(":env:"); + if (envMarker <= 0) { + return null; + } + const envKey = location.slice(envMarker + ":env:".length).trim(); + const left = location.slice(0, envMarker); + const serverMarker = left.lastIndexOf(":"); + if (serverMarker <= 0 || !envKey) { + return null; + } + const configPath = left.slice(0, serverMarker); + const serverName = left.slice(serverMarker + 1).trim(); + return configPath && serverName ? { configPath, envKey, serverName } : null; +} + +function isSingleSafeSegment(value: string): boolean { + return ( + !!value && + value !== "__proto__" && + value !== "constructor" && + value !== "prototype" && + value !== "." && + value !== ".." && + !value.includes("/") && + !value.includes("\\") && + !value.includes("\0") + ); +} + +function assertRemediationBinding( + binding: unknown, + receipt: Pick +): asserts binding is AuditMcpRemediationBinding { + const value = binding as Partial | null; + if ( + !value || + Object.keys(value).join("\0") !== REMEDIATION_BINDING_KEYS.join("\0") || + value.kind !== "mcp-inline-secret" || + typeof value.canonicalRootPath !== "string" || + typeof value.sourcePath !== "string" || + typeof value.destinationPath !== "string" || + typeof value.serverName !== "string" || + typeof value.envKey !== "string" || + typeof value.findingIdentity !== "string" || + !SHA256_RE.test(value.findingIdentity) || + !receipt.findingIdentities.includes(value.findingIdentity) || + !isSingleSafeSegment(value.serverName) || + !isSingleSafeSegment(value.envKey) + ) { + throw new Error("Audit remediation binding is unsupported"); + } + const root = value.canonicalRootPath; + const mcpRoot = join(root, "mcp"); + const sourceNames = new Set(["servers.json", "mcp.json"]); + const destinationNames = new Set(["servers.local.json", "mcp.local.json"]); + if ( + !isAbsolute(root) || + normalize(root) !== root || + dirname(value.sourcePath) !== mcpRoot || + dirname(value.destinationPath) !== mcpRoot || + !sourceNames.has(basename(value.sourcePath)) || + !destinationNames.has(basename(value.destinationPath)) || + value.sourcePath === value.destinationPath || + !receipt.sourceSnapshot.protectedRoots.some( + (identity) => identity.kind === "directory" && identity.path === root + ) || + !receipt.sourceSnapshot.evaluatedDirectories.some( + (identity) => identity.path === mcpRoot + ) || + !receipt.sourceSnapshot.evaluatedFiles.some( + (identity) => identity.path === value.sourcePath + ) || + !( + receipt.sourceSnapshot.evaluatedFiles.some( + (identity) => identity.path === value.destinationPath + ) || + receipt.sourceSnapshot.absentPaths.some( + (identity) => identity.path === value.destinationPath + ) + ) + ) { + throw new Error("Audit remediation binding is not source-bound"); + } +} + +function canonicalRemediationBindings( + bindings: readonly AuditMcpRemediationBinding[] +): AuditMcpRemediationBinding[] { + return bindings + .map((binding) => ({ ...binding })) + .sort((left, right) => + [ + left.findingIdentity, + left.sourcePath, + left.destinationPath, + left.serverName, + left.envKey, + ] + .join("\0") + .localeCompare( + [ + right.findingIdentity, + right.sourcePath, + right.destinationPath, + right.serverName, + right.envKey, + ].join("\0") + ) + ); +} + +function assertRemediationBindingsMatchReport(args: { + bindings: readonly AuditMcpRemediationBinding[]; + mode: AuditReportMode; + report: unknown; +}): void { + if (args.bindings.length === 0) { + return; + } + if ( + args.mode !== "static" || + !(args.report && typeof args.report === "object") || + !Array.isArray((args.report as { results?: unknown }).results) + ) { + throw new Error("Audit remediation bindings do not match the report"); + } + const expected = new Map< + string, + { envKey: string; serverName: string; sourcePath: string } + >(); + for (const candidate of (args.report as { results: unknown[] }).results) { + if (!(candidate && typeof candidate === "object")) { + continue; + } + const result = candidate as AuditItemResult; + if (result.type !== "mcp" || !Array.isArray(result.findings)) { + continue; + } + for (const findingCandidate of result.findings) { + if (!(findingCandidate && typeof findingCandidate === "object")) { + continue; + } + const finding = findingCandidate as AuditFinding; + const location = finding.location + ? parseInlineSecretLocation(finding.location) + : null; + if ( + finding.ruleId === "mcp-env-inline-secret" && + location && + location.configPath === result.path + ) { + expected.set(auditFindingIdentity({ finding, result }), { + envKey: location.envKey, + serverName: location.serverName, + sourcePath: result.path, + }); + } + } + } + for (const binding of args.bindings) { + const finding = expected.get(binding.findingIdentity); + if ( + !finding || + finding.envKey !== binding.envKey || + finding.serverName !== binding.serverName || + finding.sourcePath !== binding.sourcePath + ) { + throw new Error("Audit remediation bindings do not match the report"); + } + } +} + +export function buildMcpRemediationBindings(args: { + canonicalRootPath: string; + report: { results: AuditItemResult[] }; + sourceSnapshot: AuditSourceSnapshot; +}): AuditMcpRemediationBinding[] { + const canonicalRootPath = normalize(args.canonicalRootPath); + const mcpRoot = join(canonicalRootPath, "mcp"); + const canonicalDestination = join(mcpRoot, "servers.local.json"); + const legacyDestination = join(mcpRoot, "mcp.local.json"); + const existingPaths = new Set( + args.sourceSnapshot.evaluatedFiles.map((identity) => identity.path) + ); + const absentPaths = new Set( + args.sourceSnapshot.absentPaths.map((identity) => identity.path) + ); + const destinationPath = existingPaths.has(canonicalDestination) + ? canonicalDestination + : existingPaths.has(legacyDestination) + ? legacyDestination + : absentPaths.has(canonicalDestination) + ? canonicalDestination + : null; + if (!destinationPath) { + return []; + } + + const bindings = args.report.results.flatMap((result) => + result.findings.flatMap((finding) => { + const location = finding.location + ? parseInlineSecretLocation(finding.location) + : null; + if ( + result.type !== "mcp" || + finding.ruleId !== "mcp-env-inline-secret" || + !location || + result.path !== location.configPath || + dirname(result.path) !== mcpRoot || + !["servers.json", "mcp.json"].includes(basename(result.path)) || + !existingPaths.has(result.path) + ) { + return []; + } + return [ + { + canonicalRootPath, + destinationPath, + envKey: location.envKey, + findingIdentity: auditFindingIdentity({ finding, result }), + kind: "mcp-inline-secret" as const, + serverName: location.serverName, + sourcePath: result.path, + }, + ]; + }) + ); + const canonical = canonicalRemediationBindings(bindings); + const receiptShape = { + findingIdentities: canonicalFindingIdentities(args.report), + sourceSnapshot: args.sourceSnapshot, + }; + for (const binding of canonical) { + assertRemediationBinding(binding, receiptShape); + } + return canonical; +} + function canonicalFindingIdentities(report: unknown): string[] { if (!(report && typeof report === "object" && !Array.isArray(report))) { throw new Error("Audit report schema is unsupported"); @@ -500,6 +766,7 @@ export async function persistAuditReport(args: { /** @internal Adversarial test hook; production callers must not set this. */ beforeReportRootOpen?: () => Promise; mode: AuditReportMode; + remediationBindings?: AuditMcpRemediationBinding[]; report: unknown; reportRoot: string; sourceSnapshot: AuditSourceSnapshot; @@ -541,7 +808,6 @@ export async function persistAuditReport(args: { } const reportContents = `${JSON.stringify(args.report, null, 2)}\n`; const reportSha256 = sha256(reportContents); - const reportFileName = `${args.mode}-${reportSha256}.json`; const requestedRoot = normalize(args.reportRoot); const requestedMetadata = await lstat(requestedRoot).catch(() => null); @@ -605,13 +871,26 @@ export async function persistAuditReport(args: { ); } - const reportPath = join(reportRoot, reportFileName); - const canonicalSourceSnapshot = canonicalAuditSourceSnapshot( args.sourceSnapshot ); + const remediationBindings = canonicalRemediationBindings( + args.remediationBindings ?? [] + ); + const receiptBindingShape = { + findingIdentities: canonicalFindingIdentityList, + sourceSnapshot: canonicalSourceSnapshot, + }; + for (const binding of remediationBindings) { + assertRemediationBinding(binding, receiptBindingShape); + } + assertRemediationBindingsMatchReport({ + bindings: remediationBindings, + mode: args.mode, + report: args.report, + }); const receipt: AuditReportReceipt = { - schemaVersion: 5, + schemaVersion: 6, capability: AUDIT_READ_ONLY_CAPABILITY, reportRevision: AUDIT_REPORT_REVISION, mode: args.mode, @@ -621,6 +900,7 @@ export async function persistAuditReport(args: { sourceIdentitySha256: sha256(stableJson(canonicalSourceSnapshot)), sourceSnapshot: canonicalSourceSnapshot, findingIdentities: canonicalFindingIdentityList, + remediationBindings, }; const envelope: PersistedAuditEnvelope = { schemaVersion: 1, @@ -628,6 +908,8 @@ export async function persistAuditReport(args: { report: args.report, }; const envelopeContents = `${JSON.stringify(envelope, null, 2)}\n`; + const reportFileName = `${args.mode}-${sha256(envelopeContents)}.json`; + const reportPath = join(reportRoot, reportFileName); if (Buffer.byteLength(envelopeContents) > AUDIT_REPORT_MAX_ENVELOPE_BYTES) { throw new Error( `Audit report envelope exceeds ${AUDIT_REPORT_MAX_ENVELOPE_BYTES} bytes` @@ -702,7 +984,7 @@ function assertReceipt(value: unknown): asserts value is AuditReportReceipt { if ( !receipt || Object.keys(receipt).join("\0") !== RECEIPT_KEYS.join("\0") || - receipt.schemaVersion !== 5 || + receipt.schemaVersion !== 6 || receipt.capability !== AUDIT_READ_ONLY_CAPABILITY || receipt.reportRevision !== AUDIT_REPORT_REVISION || (receipt.mode !== "static" && receipt.mode !== "agent") || @@ -712,6 +994,7 @@ function assertReceipt(value: unknown): asserts value is AuditReportReceipt { !SHA256_RE.test(receipt.sourceIdentitySha256 ?? "") || !receipt.sourceSnapshot || !Array.isArray(receipt.findingIdentities) || + !Array.isArray(receipt.remediationBindings) || receipt.findingIdentities.some( (identity) => typeof identity !== "string" || !SHA256_RE.test(identity) ) @@ -732,11 +1015,26 @@ function assertReceipt(value: unknown): asserts value is AuditReportReceipt { throw new Error("Audit report receipt schema or revision is unsupported"); } assertAuditSourceSnapshot(receipt.sourceSnapshot); + for (const binding of receipt.remediationBindings) { + assertRemediationBinding(binding, receipt as AuditReportReceipt); + } + const canonicalBindings = canonicalRemediationBindings( + receipt.remediationBindings + ); + if ( + JSON.stringify(canonicalBindings) !== + JSON.stringify(receipt.remediationBindings) || + new Set( + receipt.remediationBindings.map((binding) => binding.findingIdentity) + ).size !== receipt.remediationBindings.length + ) { + throw new Error("Audit report receipt schema or revision is unsupported"); + } } function canonicalReceipt(receipt: AuditReportReceipt): AuditReportReceipt { return { - schemaVersion: 5, + schemaVersion: 6, capability: receipt.capability, reportRevision: receipt.reportRevision, mode: receipt.mode, @@ -746,6 +1044,9 @@ function canonicalReceipt(receipt: AuditReportReceipt): AuditReportReceipt { sourceIdentitySha256: receipt.sourceIdentitySha256, sourceSnapshot: canonicalAuditSourceSnapshot(receipt.sourceSnapshot), findingIdentities: [...receipt.findingIdentities], + remediationBindings: canonicalRemediationBindings( + receipt.remediationBindings + ), }; } @@ -762,6 +1063,11 @@ function assertEnvelope( throw new Error("Audit report receipt schema or revision is unsupported"); } assertReceipt(envelope.receipt); + assertRemediationBindingsMatchReport({ + bindings: envelope.receipt.remediationBindings, + mode: envelope.receipt.mode, + report: envelope.report, + }); } function canonicalEnvelopeContents(envelope: PersistedAuditEnvelope): string { @@ -864,7 +1170,7 @@ export async function loadVerifiedAuditReportEnvelope< if (sha256(canonicalReportContents) !== receipt.reportSha256) { throw new Error("Audit report content hash does not match its receipt"); } - if (fileName !== `${receipt.mode}-${receipt.reportSha256}.json`) { + if (fileName !== `${receipt.mode}-${sha256(contents)}.json`) { throw new Error("Audit report path does not match its content identity"); } if (args.expectedMode && receipt.mode !== args.expectedMode) { diff --git a/src/audit/safe-openat.ts b/src/audit/safe-openat.ts index e2d294a..5c88fb5 100644 --- a/src/audit/safe-openat.ts +++ b/src/audit/safe-openat.ts @@ -15,6 +15,7 @@ import { fstatSync, lstatSync, openSync, + readSync, realpathSync, type Stats, } from "node:fs"; @@ -240,6 +241,13 @@ interface PrivateFileMutationSymbols extends DirectoryMutationSymbols { fchmod: (fd: number, mode: number) => number; flock: (fd: number, operation: number) => number; fsync: (fd: number) => number; + linkat: ( + oldDirectoryFd: number, + oldName: Pointer, + newDirectoryFd: number, + newName: Pointer, + flags: number + ) => number; read: (fd: number, buffer: Pointer, length: number) => number | bigint; renameat: ( oldDirectoryFd: number, @@ -247,6 +255,20 @@ interface PrivateFileMutationSymbols extends DirectoryMutationSymbols { newDirectoryFd: number, newName: Pointer ) => number; + renameat2?: ( + oldDirectoryFd: number, + oldName: Pointer, + newDirectoryFd: number, + newName: Pointer, + flags: number + ) => number; + renameatx_np?: ( + oldDirectoryFd: number, + oldName: Pointer, + newDirectoryFd: number, + newName: Pointer, + flags: number + ) => number; unlinkat: (directoryFd: number, name: Pointer, flags: number) => number; write: (fd: number, buffer: Pointer, length: number) => number | bigint; } @@ -737,6 +759,94 @@ export interface PrivateDirectoryReceiptBinding { directorySegments: string[]; } +export interface PrivateDirectoryIdentity { + dev: number; + ino: number; + mode: number; +} + +export interface BoundPrivateSubdirectory { + directoryFd: number; + rootFd: number; +} + +export function openBoundPrivateSubdirectory(args: { + directoryIdentity: PrivateDirectoryIdentity; + directoryName: string; + rootIdentity: PrivateDirectoryIdentity; + rootPath: string; +}): BoundPrivateSubdirectory { + if ( + !isAbsolute(args.rootPath) || + normalize(args.rootPath) !== args.rootPath || + !args.directoryName || + args.directoryName === "." || + args.directoryName === ".." || + args.directoryName.includes(sep) || + !auditReportPersistenceSupported() + ) { + throw new Error("Bound private directory path is unsupported"); + } + const configuration = platformConfiguration(); + const libc = openSystemLibc(configuration, { + openat: { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.i32], + returns: FFIType.i32, + }, + }); + const directoryFlags = + constants.O_RDONLY + + (constants.O_DIRECTORY ?? 0) + + (constants.O_NOFOLLOW ?? 0) + + (constants.O_NONBLOCK ?? 0) + + ((constants as typeof constants & { O_CLOEXEC?: number }).O_CLOEXEC ?? 0); + let rootFd = -1; + let directoryFd = -1; + try { + rootFd = openSync(args.rootPath, directoryFlags); + const rootMetadata = fstatSync(rootFd); + if ( + !privateDirectoryComponentIsSafe(rootMetadata) || + rootMetadata.dev !== args.rootIdentity.dev || + rootMetadata.ino !== args.rootIdentity.ino || + rootMetadata.mode !== args.rootIdentity.mode || + realpathSync(args.rootPath) !== args.rootPath + ) { + throw new Error("Bound private root changed before open"); + } + directoryFd = libc.symbols.openat( + rootFd, + ptr(Buffer.from(`${args.directoryName}\0`)), + directoryFlags, + 0 + ); + if (directoryFd < 0) { + throw new Error("Bound private directory could not be opened"); + } + const directoryMetadata = fstatSync(directoryFd); + if ( + !privateDirectoryComponentIsSafe(directoryMetadata) || + directoryMetadata.dev !== args.directoryIdentity.dev || + directoryMetadata.ino !== args.directoryIdentity.ino || + directoryMetadata.mode !== args.directoryIdentity.mode + ) { + throw new Error("Bound private directory changed before open"); + } + const result = { directoryFd, rootFd }; + directoryFd = -1; + rootFd = -1; + return result; + } finally { + if (directoryFd >= 0) { + closeSync(directoryFd); + } + if (rootFd >= 0) { + closeSync(rootFd); + } + libc.close(); + } +} + export function openOrCreatePrivateDirectory( binding: PrivateDirectoryReceiptBinding ): number { @@ -824,6 +934,498 @@ export function openOrCreatePrivateDirectory( } } +export interface PrivateFileReceiptIdentity { + ctimeMs: number; + dev: number; + ino: number; + mode: number; + mtimeMs: number; + sha256: string; + size: number; +} + +function sameReceiptFileIdentity( + metadata: Stats, + expected: PrivateFileReceiptIdentity +): boolean { + return ( + privateMutableFileIsSafe(metadata, Math.max(expected.size, 0)) && + metadata.dev === expected.dev && + metadata.ino === expected.ino && + metadata.mode === expected.mode && + metadata.size === expected.size && + metadata.ctimeMs === expected.ctimeMs && + metadata.mtimeMs === expected.mtimeMs + ); +} + +function sameObjectIdentity(left: Stats, right: Stats): boolean { + return ( + left.isFile() === right.isFile() && + left.isDirectory() === right.isDirectory() && + left.dev === right.dev && + left.ino === right.ino && + left.uid === right.uid + ); +} + +export async function replaceBoundPrivateFilePairAt(args: { + /** @internal Adversarial test hook; production callers must not set this. */ + beforeSourceCommit?: () => Promise; + destinationIdentity: PrivateFileReceiptIdentity | null; + destinationName: string; + directoryFd: number; + directoryIdentity: PrivateDirectoryIdentity; + maxBytes: number; + rootFd: number; + rootIdentity: PrivateDirectoryIdentity; + rootPath: string; + sourceIdentity: PrivateFileReceiptIdentity; + sourceName: string; + transform: ( + sourceContents: string, + destinationContents: string | null + ) => { destinationContents: string; sourceContents: string }; +}): Promise<{ destinationContents: string; sourceContents: string }> { + for (const name of [args.sourceName, args.destinationName]) { + if ( + !name || + name === "." || + name === ".." || + name.includes("/") || + name.includes("\\") + ) { + throw new Error("Bound private file names must be one segment"); + } + } + if (args.sourceName === args.destinationName) { + throw new Error("Bound private source and destination must be distinct"); + } + const configuration = platformConfiguration(); + const definitions: Record = { + close: { args: [FFIType.i32], returns: FFIType.i32 }, + fchmod: { args: [FFIType.i32, FFIType.i32], returns: FFIType.i32 }, + flock: { args: [FFIType.i32, FFIType.i32], returns: FFIType.i32 }, + fsync: { args: [FFIType.i32], returns: FFIType.i32 }, + linkat: { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.ptr, FFIType.i32], + returns: FFIType.i32, + }, + openat: { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.i32], + returns: FFIType.i32, + }, + unlinkat: { + args: [FFIType.i32, FFIType.ptr, FFIType.i32], + returns: FFIType.i32, + }, + write: { + args: [FFIType.i32, FFIType.ptr, FFIType.u64], + returns: FFIType.i64, + }, + }; + if (process.platform === "darwin") { + definitions.renameatx_np = { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.ptr, FFIType.u32], + returns: FFIType.i32, + }; + definitions.__error = { args: [], returns: FFIType.ptr }; + } else { + definitions.renameat2 = { + args: [FFIType.i32, FFIType.ptr, FFIType.i32, FFIType.ptr, FFIType.u32], + returns: FFIType.i32, + }; + definitions.__errno_location = { args: [], returns: FFIType.ptr }; + } + const libc = openSystemLibc(configuration, definitions); + const symbols = libc.symbols as unknown as PrivateFileMutationSymbols; + const errnoPointer = + process.platform === "darwin" + ? symbols.__error!() + : symbols.__errno_location!(); + const errno = new DataView(toArrayBuffer(errnoPointer!, 0, 4)); + const sourceName = Buffer.from(`${args.sourceName}\0`); + const destinationName = Buffer.from(`${args.destinationName}\0`); + const sourceTemporaryName = Buffer.from( + `.${args.sourceName}.${randomUUID()}.tmp\0` + ); + const destinationTemporaryName = Buffer.from( + `.${args.destinationName}.${randomUUID()}.tmp\0` + ); + let sourceFd = -1; + let destinationFd = -1; + let sourceTemporaryFd = -1; + let destinationTemporaryFd = -1; + let destinationCommitted = false; + let sourceCommitted = false; + let sourceBefore: Stats | null = null; + let destinationBefore: Stats | null = null; + let sourceTemporaryMetadata: Stats | null = null; + let destinationTemporaryMetadata: Stats | null = null; + + const exchange = (left: Buffer, right: Buffer): void => { + const result = + process.platform === "darwin" + ? symbols.renameatx_np!( + args.directoryFd, + ptr(left), + args.directoryFd, + ptr(right), + 2 + ) + : symbols.renameat2!( + args.directoryFd, + ptr(left), + args.directoryFd, + ptr(right), + 2 + ); + if (result !== 0) { + throw new Error("Atomic private file exchange failed closed"); + } + }; + const openExisting = (name: Buffer): number | null => { + errno.setInt32(0, 0, true); + const fd = symbols.openat( + args.directoryFd, + ptr(name), + safeExistingOpenFlags(), + 0 + ); + if (fd >= 0) { + return fd; + } + if (errno.getInt32(0, true) === 2) { + return null; + } + throw new Error("Bound private file open failed closed"); + }; + const assertMappedObject = (name: Buffer, expected: Stats): void => { + const fd = openExisting(name); + if (fd === null) { + throw new Error("Bound private file mapping disappeared"); + } + try { + if (!sameObjectIdentity(fstatSync(fd), expected)) { + throw new Error("Bound private file mapping changed"); + } + } finally { + symbols.close(fd); + } + }; + const assertAbsent = (name: Buffer): void => { + const fd = openExisting(name); + if (fd !== null) { + symbols.close(fd); + throw new Error("Bound private destination appeared"); + } + }; + const unlinkIfMapped = (name: Buffer, expected: Stats | null): boolean => { + if (!expected) { + return false; + } + const fd = openExisting(name); + if (fd === null) { + return true; + } + let matches = false; + try { + matches = sameObjectIdentity(fstatSync(fd), expected); + } finally { + symbols.close(fd); + } + return matches && symbols.unlinkat(args.directoryFd, ptr(name), 0) === 0; + }; + const readHeld = ( + fd: number, + expected: PrivateFileReceiptIdentity, + label: string + ): { contents: string; metadata: Stats } => { + const before = fstatSync(fd); + if (!sameReceiptFileIdentity(before, expected)) { + throw new Error(`${label} no longer matches its audit receipt`); + } + const bytes = Buffer.alloc(before.size); + let offset = 0; + while (offset < bytes.length) { + const count = readSync(fd, bytes, offset, bytes.length - offset, offset); + if (count <= 0) { + throw new Error(`${label} changed while reading`); + } + offset += count; + } + const after = fstatSync(fd); + if (!sameFileIdentity(before, after)) { + throw new Error(`${label} changed while reading`); + } + if (createHash("sha256").update(bytes).digest("hex") !== expected.sha256) { + throw new Error(`${label} bytes no longer match their audit receipt`); + } + try { + return { contents: UTF8_DECODER.decode(bytes), metadata: before }; + } catch { + throw new Error(`${label} is not UTF-8`); + } + }; + const stage = ( + name: Buffer, + bytes: Buffer, + mode: number + ): { fd: number; metadata: Stats } => { + const fd = symbols.openat( + args.directoryFd, + ptr(name), + configuration.createExclusive, + mode + ); + if (fd < 0 || symbols.fchmod(fd, mode) !== 0) { + if (fd >= 0) { + symbols.close(fd); + symbols.unlinkat(args.directoryFd, ptr(name), 0); + } + throw new Error("Bound private temporary creation failed closed"); + } + let offset = 0; + while (offset < bytes.length) { + const count = Number( + symbols.write(fd, ptr(bytes, offset), bytes.length - offset) + ); + if (count <= 0) { + symbols.close(fd); + symbols.unlinkat(args.directoryFd, ptr(name), 0); + throw new Error("Bound private temporary write failed closed"); + } + offset += count; + } + if (symbols.fsync(fd) !== 0) { + symbols.close(fd); + symbols.unlinkat(args.directoryFd, ptr(name), 0); + throw new Error("Bound private temporary sync failed closed"); + } + const metadata = fstatSync(fd); + if ( + !privateMutableFileIsSafe(metadata, args.maxBytes) || + permissionBits(metadata.mode) !== mode || + metadata.size !== bytes.length + ) { + symbols.close(fd); + symbols.unlinkat(args.directoryFd, ptr(name), 0); + throw new Error("Bound private temporary is unsafe"); + } + return { fd, metadata }; + }; + const assertDirectoryBinding = (): void => { + const root = fstatSync(args.rootFd); + const directory = fstatSync(args.directoryFd); + const rootPathMetadata = lstatSync(args.rootPath); + const directoryPath = join(args.rootPath, "mcp"); + const directoryPathMetadata = lstatSync(directoryPath); + if ( + !privateDirectoryComponentIsSafe(root) || + root.dev !== args.rootIdentity.dev || + root.ino !== args.rootIdentity.ino || + root.mode !== args.rootIdentity.mode || + !privateDirectoryComponentIsSafe(directory) || + directory.dev !== args.directoryIdentity.dev || + directory.ino !== args.directoryIdentity.ino || + directory.mode !== args.directoryIdentity.mode || + rootPathMetadata.isSymbolicLink() || + !sameObjectIdentity(rootPathMetadata, root) || + directoryPathMetadata.isSymbolicLink() || + !sameObjectIdentity(directoryPathMetadata, directory) || + realpathSync(args.rootPath) !== args.rootPath || + realpathSync(directoryPath) !== directoryPath + ) { + throw new Error("Bound private directory identity changed"); + } + const reboundFd = symbols.openat( + args.rootFd, + ptr(Buffer.from("mcp\0")), + constants.O_RDONLY + + (constants.O_DIRECTORY ?? 0) + + (constants.O_NOFOLLOW ?? 0) + + (constants.O_NONBLOCK ?? 0), + 0 + ); + if (reboundFd < 0) { + throw new Error("Bound private directory mapping changed"); + } + try { + if (!sameObjectIdentity(fstatSync(reboundFd), directory)) { + throw new Error("Bound private directory mapping changed"); + } + } finally { + symbols.close(reboundFd); + } + }; + const rollback = (): void => { + if (sourceCommitted) { + assertMappedObject(sourceName, sourceTemporaryMetadata!); + assertMappedObject(sourceTemporaryName, sourceBefore!); + exchange(sourceTemporaryName, sourceName); + sourceCommitted = false; + } + if (destinationCommitted) { + if (destinationBefore) { + assertMappedObject(destinationName, destinationTemporaryMetadata!); + assertMappedObject(destinationTemporaryName, destinationBefore); + exchange(destinationTemporaryName, destinationName); + } else { + assertMappedObject(destinationName, destinationTemporaryMetadata!); + if (symbols.unlinkat(args.directoryFd, ptr(destinationName), 0) !== 0) { + throw new Error("Bound private destination rollback failed closed"); + } + } + destinationCommitted = false; + } + symbols.fsync(args.directoryFd); + }; + + try { + if (symbols.flock(args.directoryFd, 6) !== 0) { + throw new Error("Bound private mutation is already active"); + } + assertDirectoryBinding(); + sourceFd = openExisting(sourceName) ?? -1; + if (sourceFd < 0) { + throw new Error("Bound private source disappeared"); + } + const sourceSnapshot = readHeld( + sourceFd, + args.sourceIdentity, + "Bound private source" + ); + sourceBefore = sourceSnapshot.metadata; + destinationFd = openExisting(destinationName) ?? -1; + let destinationContents: string | null = null; + if (args.destinationIdentity) { + if (destinationFd < 0) { + throw new Error("Bound private destination disappeared"); + } + const destinationSnapshot = readHeld( + destinationFd, + args.destinationIdentity, + "Bound private destination" + ); + destinationBefore = destinationSnapshot.metadata; + destinationContents = destinationSnapshot.contents; + } else if (destinationFd >= 0) { + throw new Error("Bound private destination appeared"); + } + + const committedContents = args.transform( + sourceSnapshot.contents, + destinationContents + ); + const sourceBytes = Buffer.from(committedContents.sourceContents); + const destinationBytes = Buffer.from(committedContents.destinationContents); + if ( + sourceBytes.byteLength > args.maxBytes || + destinationBytes.byteLength > args.maxBytes + ) { + throw new Error("Bound private file exceeds byte limit"); + } + + let staged = stage( + sourceTemporaryName, + sourceBytes, + permissionBits(args.sourceIdentity.mode) + ); + sourceTemporaryFd = staged.fd; + sourceTemporaryMetadata = staged.metadata; + staged = stage( + destinationTemporaryName, + destinationBytes, + args.destinationIdentity + ? permissionBits(args.destinationIdentity.mode) + : 0o600 + ); + destinationTemporaryFd = staged.fd; + destinationTemporaryMetadata = staged.metadata; + + assertDirectoryBinding(); + if (!sameFileIdentity(sourceBefore, fstatSync(sourceFd))) { + throw new Error("Bound private source changed before commit"); + } + assertMappedObject(sourceName, sourceBefore); + if (destinationBefore) { + if (!sameFileIdentity(destinationBefore, fstatSync(destinationFd))) { + throw new Error("Bound private destination changed before commit"); + } + assertMappedObject(destinationName, destinationBefore); + exchange(destinationTemporaryName, destinationName); + } else { + assertAbsent(destinationName); + if ( + symbols.linkat( + args.directoryFd, + ptr(destinationTemporaryName), + args.directoryFd, + ptr(destinationName), + 0 + ) !== 0 + ) { + throw new Error("Bound private destination create failed closed"); + } + } + destinationCommitted = true; + + if (args.beforeSourceCommit) { + await args.beforeSourceCommit(); + } + assertDirectoryBinding(); + if (!sameFileIdentity(sourceBefore, fstatSync(sourceFd))) { + throw new Error("Bound private source changed before final commit"); + } + assertMappedObject(sourceName, sourceBefore); + assertMappedObject(destinationName, destinationTemporaryMetadata); + exchange(sourceTemporaryName, sourceName); + sourceCommitted = true; + assertMappedObject(sourceName, sourceTemporaryMetadata); + assertMappedObject(sourceTemporaryName, sourceBefore); + + // Both exchanges are now the irrevocable commit. Cleanup cannot be + // reported as a failed transaction because either rollback object may + // already have been unlinked; doing so would turn a committed mutation + // into a false failure and invite an unsafe pathname retry. + sourceCommitted = false; + destinationCommitted = false; + unlinkIfMapped(sourceTemporaryName, sourceBefore); + unlinkIfMapped( + destinationTemporaryName, + destinationBefore ?? destinationTemporaryMetadata + ); + symbols.fsync(args.directoryFd); + return committedContents; + } catch (error) { + try { + rollback(); + } catch (rollbackError) { + throw new Error( + `Bound private mutation rollback failed: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + { cause: error } + ); + } + throw error; + } finally { + if (sourceFd >= 0) { + symbols.close(sourceFd); + } + if (destinationFd >= 0) { + symbols.close(destinationFd); + } + if (sourceTemporaryFd >= 0) { + symbols.close(sourceTemporaryFd); + } + if (destinationTemporaryFd >= 0) { + symbols.close(destinationTemporaryFd); + } + unlinkIfMapped(sourceTemporaryName, sourceTemporaryMetadata); + unlinkIfMapped(destinationTemporaryName, destinationTemporaryMetadata); + libc.close(); + } +} + export async function replacePrivateFileAt(args: { /** @internal Adversarial test hook; production callers must not set this. */ beforeCommit?: () => Promise; diff --git a/src/audit/static.ts b/src/audit/static.ts index ced2f8e..83c28a8 100644 --- a/src/audit/static.ts +++ b/src/audit/static.ts @@ -29,6 +29,7 @@ import { parseJsonLenient } from "../util/json"; import { type AuditEvaluation, auditedRootsFromScan, + buildMcpRemediationBindings, parseReportRootFlag, persistAuditReport, } from "./report-persistence"; @@ -820,6 +821,11 @@ export async function evaluateStaticAudit(opts?: { auditedRoots.push(rulesPath, dirname(rulesPath)); } await sourceTracker.protect(Array.from(new Set(auditedRoots)).sort()); + const canonicalMcpRoot = join(canonicalRoot, "mcp"); + await sourceTracker.capture(canonicalRoot); + await sourceTracker.capture(canonicalMcpRoot); + await sourceTracker.capture(join(canonicalMcpRoot, "servers.local.json")); + await sourceTracker.capture(join(canonicalMcpRoot, "mcp.local.json")); for (const source of res.sources) { for (const pathValue of [ ...source.evidence, @@ -1225,8 +1231,14 @@ export async function evaluateStaticAudit(opts?: { } const sourceSnapshot = sourceTracker.snapshot(); await validateAuditSourceSnapshot(sourceSnapshot); + const remediationBindings = buildMcpRemediationBindings({ + canonicalRootPath: canonicalRoot, + report, + sourceSnapshot, + }); return { auditedRoots: Array.from(new Set(auditedRoots)).sort(), + remediationBindings, report, sourceSnapshot, }; @@ -1306,6 +1318,7 @@ export async function staticAuditCommand(argv: string[]) { reportPath = await persistAuditReport({ auditedRoots: evaluation.auditedRoots, mode: "static", + remediationBindings: evaluation.remediationBindings, report: evaluation.report, reportRoot, sourceSnapshot: evaluation.sourceSnapshot, From 135a1b89bf68450b25f97d1add2ce67fd4679430 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 16:33:29 -0400 Subject: [PATCH 02/13] test(audit): align envelope authorization proofs --- scripts/verify-binary.ts | 4 +-- src/audit/fix.test.ts | 4 +-- .../report-persistence-adversarial.test.ts | 30 +++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/scripts/verify-binary.ts b/scripts/verify-binary.ts index 0e824ef..aca1235 100644 --- a/scripts/verify-binary.ts +++ b/scripts/verify-binary.ts @@ -154,8 +154,8 @@ if (auditPersistenceContract(process.platform) === "fail-closed") { !reportName || reportNames.length !== 1 || envelope?.schemaVersion !== 1 || - envelope.receipt?.schemaVersion !== 5 || - envelope.receipt.reportRevision !== 10 || + envelope.receipt?.schemaVersion !== 6 || + envelope.receipt.reportRevision !== 11 || JSON.stringify(envelope.report) !== JSON.stringify(JSON.parse(persistedAuditText)) || (await Bun.file(auditSkill).text()) !== auditSourceBefore diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index ded66e5..7832cb8 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -328,7 +328,7 @@ describe("audit fix", () => { cwd: fixture.home, homeDir: fixture.home, }) - ).rejects.toThrow("do not match the report"); + ).rejects.toThrow("schema or revision is unsupported"); expect(await Bun.file(fixture.localPath).exists()).toBe(false); } finally { await fixture.cleanup(); @@ -409,7 +409,7 @@ describe("audit fix", () => { ); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Audit evaluated context changed"); + expect(result.stderr).toContain("Audit requested path changed"); expect(await Bun.file(localPath).exists()).toBe(true); expect(await Bun.file(managedToolPath).text()).toBe(managedToolBefore); }); diff --git a/src/audit/report-persistence-adversarial.test.ts b/src/audit/report-persistence-adversarial.test.ts index a1bec99..ac2d510 100644 --- a/src/audit/report-persistence-adversarial.test.ts +++ b/src/audit/report-persistence-adversarial.test.ts @@ -378,7 +378,7 @@ describe("adversarial audit report persistence", () => { ).resolves.toEqual(evaluation.report); }); - test("same-content concurrency is idempotent and source conflicts preserve the winner", async () => { + test("same-content concurrency is idempotent and distinct source envelopes coexist", async () => { const { evaluation } = await fixture(); const reportRoot = await mkdtemp( join(tmpdir(), "fclt-report-adversarial-idempotent-") @@ -396,14 +396,14 @@ describe("adversarial audit report persistence", () => { expect(await readdir(reportRoot)).toHaveLength(1); const other = await fixture(); - await expect( - persistAuditReport({ - ...other.evaluation, - mode: "static", - report: evaluation.report, - reportRoot, - }) - ).rejects.toThrow("collision"); + const otherPath = await persistAuditReport({ + ...other.evaluation, + mode: "static", + report: evaluation.report, + reportRoot, + }); + expect(otherPath).not.toBe(paths[0]); + expect(await readdir(reportRoot)).toHaveLength(2); await expect( loadVerifiedAuditReport({ reportPath: paths[0]! }) ).resolves.toEqual(evaluation.report); @@ -693,10 +693,16 @@ describe("adversarial audit report persistence", () => { .update(stableJson(sourceSnapshot)) .digest("hex"); await writePrivateJson(reportPath, envelope); - - await expect(loadVerifiedAuditReport({ reportPath })).rejects.toThrow( - "overlaps an audited source" + const modifiedContents = await readFile(reportPath, "utf8"); + const modifiedPath = join( + placedRoot, + `static-${createHash("sha256").update(modifiedContents).digest("hex")}.json` ); + await rename(reportPath, modifiedPath); + + await expect( + loadVerifiedAuditReport({ reportPath: modifiedPath }) + ).rejects.toThrow("overlaps an audited source"); }); test("loader binds the exact parent descriptor, permissions, and final child name", async () => { From 42027f14e00fec610b6f89d1960d091e77f54101 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 16:43:31 -0400 Subject: [PATCH 03/13] fix(audit): secure local remediation identity --- src/audit/fix.test.ts | 78 ++++++++++++++++++++++++++++++++- src/audit/fix.ts | 34 +++++++------- src/audit/report-persistence.ts | 39 +++++++++++------ src/audit/safe-openat.ts | 8 +--- 4 files changed, 120 insertions(+), 39 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 7832cb8..87055a6 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -123,19 +123,35 @@ async function makeFixBase() { }); } -async function makeAuthorizedFixture(marker = "fixture_secret_1234567890") { +async function makeAuthorizedFixture( + marker = "fixture_secret_1234567890", + options?: { + localMode?: number; + localServer?: Record; + serverName?: string; + } +) { const home = await makeTempHome(); const root = join(home, ".ai"); const trackedPath = join(root, "mcp", "servers.json"); const localPath = join(root, "mcp", "servers.local.json"); + const serverName = options?.serverName ?? "github"; await writeJson(trackedPath, { servers: { - github: { + [serverName]: { command: "fixture-command", env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, }, }, }); + if (options?.localServer) { + await writeJson(localPath, { + servers: { [serverName]: options.localServer }, + }); + if (options.localMode !== undefined) { + await chmod(localPath, options.localMode); + } + } const audit = await evaluateStaticAudit({ argv: [], cwd: home, @@ -160,6 +176,7 @@ async function makeAuthorizedFixture(marker = "fixture_secret_1234567890") { localPath, reportRoot, root, + serverName, trackedPath, }; } @@ -380,6 +397,63 @@ describe("audit fix", () => { } }); + it("secures an existing readable local destination to owner-only mode", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + localMode: 0o644, + localServer: { env: { NON_SECRET_SETTING: "preserve" } }, + }); + try { + await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect((await lstat(fixture.localPath)).mode % 0o1000).toBe(0o600); + expect(await Bun.file(fixture.localPath).json()).toEqual({ + servers: { + github: { + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + NON_SECRET_SETTING: "preserve", + }, + }, + }, + }); + } finally { + await fixture.cleanup(); + } + }); + + it("binds colon-bearing server names without location ambiguity", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + serverName: "team:github", + }); + try { + const result = await runAuditFix({ + argv: [ + "--item", + "team:github", + "--report", + fixture.exactReportPath, + "--yes", + ], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + const local = (await Bun.file(fixture.localPath).json()) as { + servers: Record; + }; + expect(local.servers["team:github"]).toEqual({ + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }); + } finally { + await fixture.cleanup(); + } + }); + it("keeps the CLI dry-run path zero-write", async () => { const trackedPath = join(rootDir!, "mcp", "servers.json"); const localPath = join(rootDir!, "mcp", "servers.local.json"); diff --git a/src/audit/fix.ts b/src/audit/fix.ts index d37b049..17d1b7e 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -154,27 +154,26 @@ function parseAuditFixArgs(argv: string[]): AuditFixArgs { return args; } -function parseInlineSecretLocation(location: string): { +function parseInlineSecretLocation(args: { + location: string; + result: AuditItemResult; +}): { configPath: string; serverName: string; envKey: string; } | null { - const envMarker = location.lastIndexOf(":env:"); - if (envMarker <= 0) { - return null; - } - const envKey = location.slice(envMarker + ":env:".length).trim(); - const left = location.slice(0, envMarker); - const serverMarker = left.lastIndexOf(":"); - if (serverMarker <= 0 || !envKey) { + const prefix = `${args.result.path}:${args.result.item}:env:`; + if (!args.location.startsWith(prefix)) { return null; } - const configPath = left.slice(0, serverMarker); - const serverName = left.slice(serverMarker + 1).trim(); - if (!(configPath && serverName)) { - return null; - } - return { configPath, serverName, envKey }; + const envKey = args.location.slice(prefix.length).trim(); + return envKey + ? { + configPath: args.result.path, + envKey, + serverName: args.result.item, + } + : null; } function findingKey(args: { @@ -182,7 +181,10 @@ function findingKey(args: { finding: AuditFinding; }): string { const parsed = args.finding.location - ? parseInlineSecretLocation(args.finding.location) + ? parseInlineSecretLocation({ + location: args.finding.location, + result: args.result, + }) : null; return [ args.result.type, diff --git a/src/audit/report-persistence.ts b/src/audit/report-persistence.ts index 806680a..bc51c10 100644 --- a/src/audit/report-persistence.ts +++ b/src/audit/report-persistence.ts @@ -179,24 +179,27 @@ const REMEDIATION_BINDING_KEYS = [ "sourcePath", ] as const; -function parseInlineSecretLocation(location: string): { +function parseInlineSecretLocation(args: { + configPath: string; + location: string; + serverName: string; +}): { configPath: string; envKey: string; serverName: string; } | null { - const envMarker = location.lastIndexOf(":env:"); - if (envMarker <= 0) { + const prefix = `${args.configPath}:${args.serverName}:env:`; + if (!args.location.startsWith(prefix)) { return null; } - const envKey = location.slice(envMarker + ":env:".length).trim(); - const left = location.slice(0, envMarker); - const serverMarker = left.lastIndexOf(":"); - if (serverMarker <= 0 || !envKey) { - return null; - } - const configPath = left.slice(0, serverMarker); - const serverName = left.slice(serverMarker + 1).trim(); - return configPath && serverName ? { configPath, envKey, serverName } : null; + const envKey = args.location.slice(prefix.length).trim(); + return envKey + ? { + configPath: args.configPath, + envKey, + serverName: args.serverName, + } + : null; } function isSingleSafeSegment(value: string): boolean { @@ -328,7 +331,11 @@ function assertRemediationBindingsMatchReport(args: { } const finding = findingCandidate as AuditFinding; const location = finding.location - ? parseInlineSecretLocation(finding.location) + ? parseInlineSecretLocation({ + configPath: result.path, + location: finding.location, + serverName: result.item, + }) : null; if ( finding.ruleId === "mcp-env-inline-secret" && @@ -385,7 +392,11 @@ export function buildMcpRemediationBindings(args: { const bindings = args.report.results.flatMap((result) => result.findings.flatMap((finding) => { const location = finding.location - ? parseInlineSecretLocation(finding.location) + ? parseInlineSecretLocation({ + configPath: result.path, + location: finding.location, + serverName: result.item, + }) : null; if ( result.type !== "mcp" || diff --git a/src/audit/safe-openat.ts b/src/audit/safe-openat.ts index 5c88fb5..f3f2169 100644 --- a/src/audit/safe-openat.ts +++ b/src/audit/safe-openat.ts @@ -1333,13 +1333,7 @@ export async function replaceBoundPrivateFilePairAt(args: { ); sourceTemporaryFd = staged.fd; sourceTemporaryMetadata = staged.metadata; - staged = stage( - destinationTemporaryName, - destinationBytes, - args.destinationIdentity - ? permissionBits(args.destinationIdentity.mode) - : 0o600 - ); + staged = stage(destinationTemporaryName, destinationBytes, 0o600); destinationTemporaryFd = staged.fd; destinationTemporaryMetadata = staged.metadata; From 2aa69d2ebf6d1b8fc854266781179675fe68723a Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 16:52:53 -0400 Subject: [PATCH 04/13] fix(audit): support dotted MCP server containers --- src/audit/fix.test.ts | 32 +++++++++++++++++++++++++++++++- src/mcp-config.ts | 1 + 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 87055a6..ae8737f 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -129,6 +129,7 @@ async function makeAuthorizedFixture( localMode?: number; localServer?: Record; serverName?: string; + sourceContainer?: "mcp.servers" | "servers"; } ) { const home = await makeTempHome(); @@ -136,8 +137,9 @@ async function makeAuthorizedFixture( const trackedPath = join(root, "mcp", "servers.json"); const localPath = join(root, "mcp", "servers.local.json"); const serverName = options?.serverName ?? "github"; + const sourceContainer = options?.sourceContainer ?? "servers"; await writeJson(trackedPath, { - servers: { + [sourceContainer]: { [serverName]: { command: "fixture-command", env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, @@ -454,6 +456,34 @@ describe("audit fix", () => { } }); + it("remediates the auditor-supported dotted MCP server container", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + sourceContainer: "mcp.servers", + }); + try { + const result = await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + expect(await Bun.file(fixture.trackedPath).json()).toEqual({ + "mcp.servers": { github: { command: "fixture-command" } }, + }); + expect(await Bun.file(fixture.localPath).json()).toEqual({ + servers: { + github: { + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }); + } finally { + await fixture.cleanup(); + } + }); + it("keeps the CLI dry-run path zero-write", async () => { const trackedPath = join(rootDir!, "mcp", "servers.json"); const localPath = join(rootDir!, "mcp", "servers.local.json"); diff --git a/src/mcp-config.ts b/src/mcp-config.ts index e465c95..cad9e0d 100644 --- a/src/mcp-config.ts +++ b/src/mcp-config.ts @@ -40,6 +40,7 @@ export function extractServersObject( const servers = (raw.servers as Record | undefined) ?? (raw.mcpServers as Record | undefined) ?? + (raw["mcp.servers"] as Record | undefined) ?? ((raw.mcp as Record | undefined)?.servers as | Record | undefined) ?? From c84aa06a5b21f11b1df73a300a9c7979ff49b8ae Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:01:58 -0400 Subject: [PATCH 05/13] fix(audit): bind remediation container selection --- src/audit/fix.test.ts | 69 ++++++++++++++++++++++++++++++++++++++----- src/audit/fix.ts | 11 ++++--- src/audit/static.ts | 37 ++++------------------- src/mcp-config.ts | 29 +++++++++++++++++- 4 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index ae8737f..29e523f 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -130,6 +130,7 @@ async function makeAuthorizedFixture( localServer?: Record; serverName?: string; sourceContainer?: "mcp.servers" | "servers"; + sourceRoot?: Record; } ) { const home = await makeTempHome(); @@ -138,14 +139,17 @@ async function makeAuthorizedFixture( const localPath = join(root, "mcp", "servers.local.json"); const serverName = options?.serverName ?? "github"; const sourceContainer = options?.sourceContainer ?? "servers"; - await writeJson(trackedPath, { - [sourceContainer]: { - [serverName]: { - command: "fixture-command", - env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, + await writeJson( + trackedPath, + options?.sourceRoot ?? { + [sourceContainer]: { + [serverName]: { + command: "fixture-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, + }, }, - }, - }); + } + ); if (options?.localServer) { await writeJson(localPath, { servers: { [serverName]: options.localServer }, @@ -484,6 +488,57 @@ describe("audit fix", () => { } }); + it("remediates the exact container selected by the static auditor", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + sourceRoot: { + servers: { + github: { + command: "decoy-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, + }, + }, + "mcp.servers": { + github: { + command: "fixture-command", + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }, + }); + try { + const result = await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + expect(await Bun.file(fixture.trackedPath).json()).toEqual({ + servers: { + github: { + command: "decoy-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, + }, + }, + "mcp.servers": { + github: { command: "fixture-command" }, + }, + }); + expect(await Bun.file(fixture.localPath).json()).toEqual({ + servers: { + github: { + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }); + } finally { + await fixture.cleanup(); + } + }); + it("keeps the CLI dry-run path zero-write", async () => { const trackedPath = join(rootDir!, "mcp", "servers.json"); const localPath = join(rootDir!, "mcp", "servers.local.json"); diff --git a/src/audit/fix.ts b/src/audit/fix.ts index 17d1b7e..1af0a48 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -1,7 +1,10 @@ import { closeSync } from "node:fs"; import { homedir } from "node:os"; import { basename, dirname, join, normalize } from "node:path"; -import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; +import { + extractAuditMcpServersObject, + isInlineMcpSecretValue, +} from "../mcp-config"; import { facultContextRootDir } from "../paths"; import { parseJsonLenient } from "../util/json"; import type { AgentAuditReport } from "./agent"; @@ -368,20 +371,20 @@ function transformBoundMcpConfigs(args: { if (!isPlainObject(sourceRoot)) { throw new Error("Bound MCP source is not a JSON object"); } - const sourceServers = extractServersObject(sourceRoot); + const sourceServers = extractAuditMcpServersObject(sourceRoot); if (!sourceServers) { throw new Error("Bound MCP source has no servers object"); } const destinationRoot: Record = args.destinationContents ? (() => { const parsed = parseJsonLenient(args.destinationContents); - if (!(isPlainObject(parsed) && extractServersObject(parsed))) { + if (!(isPlainObject(parsed) && extractAuditMcpServersObject(parsed))) { throw new Error("Bound MCP destination has no servers object"); } return parsed; })() : { servers: {} }; - const destinationServers = extractServersObject(destinationRoot)!; + const destinationServers = extractAuditMcpServersObject(destinationRoot)!; for (const binding of args.bindings) { const sourceServer = sourceServers[binding.serverName]; diff --git a/src/audit/static.ts b/src/audit/static.ts index 83c28a8..b6bed9f 100644 --- a/src/audit/static.ts +++ b/src/audit/static.ts @@ -9,7 +9,10 @@ import { } from "node:path"; import { parse as parseYaml } from "yaml"; import { loadManagedState } from "../manage"; -import { isInlineMcpSecretValue } from "../mcp-config"; +import { + extractAuditMcpServersObject, + isInlineMcpSecretValue, +} from "../mcp-config"; import { facultConfigPath, facultContextRootDir, @@ -400,36 +403,6 @@ function shouldApplyRule( return t === target; } -function extractMcpServersObject( - parsed: unknown -): Record | null { - if (!isPlainObject(parsed)) { - return null; - } - const obj = parsed as Record; - if (isPlainObject(obj.mcpServers)) { - return obj.mcpServers as Record; - } - for (const [k, v] of Object.entries(obj)) { - if (k.endsWith(".mcpServers") && isPlainObject(v)) { - return v as Record; - } - } - if (isPlainObject(obj["mcp.servers"])) { - return obj["mcp.servers"] as Record; - } - if (isPlainObject(obj.servers)) { - return obj.servers as Record; - } - if (isPlainObject(obj.mcp)) { - const mcp = obj.mcp as Record; - if (isPlainObject(mcp.servers)) { - return mcp.servers as Record; - } - } - return null; -} - function mcpSafeAuditText(definition: unknown): string { if (!isPlainObject(definition)) { return String(definition); @@ -1134,7 +1107,7 @@ export async function evaluateStaticAudit(opts?: { continue; } - const serversObj = extractMcpServersObject(parsed); + const serversObj = extractAuditMcpServersObject(parsed); if (!serversObj) { continue; } diff --git a/src/mcp-config.ts b/src/mcp-config.ts index cad9e0d..90e3e37 100644 --- a/src/mcp-config.ts +++ b/src/mcp-config.ts @@ -25,6 +25,33 @@ export function isInlineMcpSecretValue(value: unknown): value is string { return true; } +export function extractAuditMcpServersObject( + parsed: unknown +): Record | null { + if (!isPlainObject(parsed)) { + return null; + } + const raw = parsed as Record; + if (isPlainObject(raw.mcpServers)) { + return raw.mcpServers; + } + for (const [key, value] of Object.entries(raw)) { + if (key.endsWith(".mcpServers") && isPlainObject(value)) { + return value; + } + } + if (isPlainObject(raw["mcp.servers"])) { + return raw["mcp.servers"]; + } + if (isPlainObject(raw.servers)) { + return raw.servers; + } + if (isPlainObject(raw.mcp) && isPlainObject(raw.mcp.servers)) { + return raw.mcp.servers; + } + return null; +} + export function extractServersObject( parsed: unknown ): Record | null { @@ -34,7 +61,7 @@ export function extractServersObject( const raw = parsed as Record; for (const [key, value] of Object.entries(raw)) { if (key.endsWith(".mcpServers") && isPlainObject(value)) { - return value as Record; + return value; } } const servers = From c9a2160152674a506ec57e16e3912667722e84a1 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:08:40 -0400 Subject: [PATCH 06/13] fix(audit): remediate the active MCP container --- src/audit/fix.test.ts | 22 ++++++++++++---------- src/audit/fix.ts | 11 ++++------- src/audit/static.ts | 7 ++----- src/mcp-config.ts | 27 --------------------------- 4 files changed, 18 insertions(+), 49 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 29e523f..c832700 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -488,16 +488,10 @@ describe("audit fix", () => { } }); - it("remediates the exact container selected by the static auditor", async () => { + it("remediates the active container shared by runtime and static audit", async () => { const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { sourceRoot: { servers: { - github: { - command: "decoy-command", - env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, - }, - }, - "mcp.servers": { github: { command: "fixture-command", env: { @@ -505,6 +499,12 @@ describe("audit fix", () => { }, }, }, + "mcp.servers": { + github: { + command: "decoy-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, + }, + }, }, }); try { @@ -517,12 +517,14 @@ describe("audit fix", () => { expect(await Bun.file(fixture.trackedPath).json()).toEqual({ servers: { github: { - command: "decoy-command", - env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, + command: "fixture-command", }, }, "mcp.servers": { - github: { command: "fixture-command" }, + github: { + command: "decoy-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: "decoy_secret_1234567890" }, + }, }, }); expect(await Bun.file(fixture.localPath).json()).toEqual({ diff --git a/src/audit/fix.ts b/src/audit/fix.ts index 1af0a48..17d1b7e 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -1,10 +1,7 @@ import { closeSync } from "node:fs"; import { homedir } from "node:os"; import { basename, dirname, join, normalize } from "node:path"; -import { - extractAuditMcpServersObject, - isInlineMcpSecretValue, -} from "../mcp-config"; +import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; import { facultContextRootDir } from "../paths"; import { parseJsonLenient } from "../util/json"; import type { AgentAuditReport } from "./agent"; @@ -371,20 +368,20 @@ function transformBoundMcpConfigs(args: { if (!isPlainObject(sourceRoot)) { throw new Error("Bound MCP source is not a JSON object"); } - const sourceServers = extractAuditMcpServersObject(sourceRoot); + const sourceServers = extractServersObject(sourceRoot); if (!sourceServers) { throw new Error("Bound MCP source has no servers object"); } const destinationRoot: Record = args.destinationContents ? (() => { const parsed = parseJsonLenient(args.destinationContents); - if (!(isPlainObject(parsed) && extractAuditMcpServersObject(parsed))) { + if (!(isPlainObject(parsed) && extractServersObject(parsed))) { throw new Error("Bound MCP destination has no servers object"); } return parsed; })() : { servers: {} }; - const destinationServers = extractAuditMcpServersObject(destinationRoot)!; + const destinationServers = extractServersObject(destinationRoot)!; for (const binding of args.bindings) { const sourceServer = sourceServers[binding.serverName]; diff --git a/src/audit/static.ts b/src/audit/static.ts index b6bed9f..4b83732 100644 --- a/src/audit/static.ts +++ b/src/audit/static.ts @@ -9,10 +9,7 @@ import { } from "node:path"; import { parse as parseYaml } from "yaml"; import { loadManagedState } from "../manage"; -import { - extractAuditMcpServersObject, - isInlineMcpSecretValue, -} from "../mcp-config"; +import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; import { facultConfigPath, facultContextRootDir, @@ -1107,7 +1104,7 @@ export async function evaluateStaticAudit(opts?: { continue; } - const serversObj = extractAuditMcpServersObject(parsed); + const serversObj = extractServersObject(parsed); if (!serversObj) { continue; } diff --git a/src/mcp-config.ts b/src/mcp-config.ts index 90e3e37..83b8e9f 100644 --- a/src/mcp-config.ts +++ b/src/mcp-config.ts @@ -25,33 +25,6 @@ export function isInlineMcpSecretValue(value: unknown): value is string { return true; } -export function extractAuditMcpServersObject( - parsed: unknown -): Record | null { - if (!isPlainObject(parsed)) { - return null; - } - const raw = parsed as Record; - if (isPlainObject(raw.mcpServers)) { - return raw.mcpServers; - } - for (const [key, value] of Object.entries(raw)) { - if (key.endsWith(".mcpServers") && isPlainObject(value)) { - return value; - } - } - if (isPlainObject(raw["mcp.servers"])) { - return raw["mcp.servers"]; - } - if (isPlainObject(raw.servers)) { - return raw.servers; - } - if (isPlainObject(raw.mcp) && isPlainObject(raw.mcp.servers)) { - return raw.mcp.servers; - } - return null; -} - export function extractServersObject( parsed: unknown ): Record | null { From c241452c7424023130b42b34ea461ecce0b18259 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:21:38 -0400 Subject: [PATCH 07/13] fix(audit): align canonical MCP scope --- src/audit/agent.ts | 33 ++------------------------------- src/audit/fix.test.ts | 22 ++++++++++++++++++++++ src/audit/fix.ts | 9 ++------- src/scan.test.ts | 26 ++++++++++++++++++++++++++ src/scan.ts | 36 +++--------------------------------- 5 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/audit/agent.ts b/src/audit/agent.ts index 540b526..9586f0d 100644 --- a/src/audit/agent.ts +++ b/src/audit/agent.ts @@ -2,6 +2,7 @@ import { constants } from "node:fs"; import { lstat, mkdir, mkdtemp, open, realpath, rm } from "node:fs/promises"; import { homedir, tmpdir } from "node:os"; import { basename, join } from "node:path"; +import { extractServersObject } from "../mcp-config"; import { facultConfigPath, facultRootDir, @@ -208,36 +209,6 @@ function parseMaxItemsFlag(argv: string[]): number | null { return null; } -function extractMcpServersObject( - parsed: unknown -): Record | null { - if (!isPlainObject(parsed)) { - return null; - } - const obj = parsed as Record; - if (isPlainObject(obj.mcpServers)) { - return obj.mcpServers as Record; - } - for (const [k, v] of Object.entries(obj)) { - if (k.endsWith(".mcpServers") && isPlainObject(v)) { - return v as Record; - } - } - if (isPlainObject(obj["mcp.servers"])) { - return obj["mcp.servers"] as Record; - } - if (isPlainObject(obj.servers)) { - return obj.servers as Record; - } - if (isPlainObject(obj.mcp)) { - const mcp = obj.mcp as Record; - if (isPlainObject(mcp.servers)) { - return mcp.servers as Record; - } - } - return null; -} - function mcpSafeText(definition: unknown): string { if (!isPlainObject(definition)) { return redactPossibleSecrets(String(definition)); @@ -1402,7 +1373,7 @@ export async function evaluateAgentAudit(opts?: { } catch { continue; } - const serversObj = extractMcpServersObject(parsed); + const serversObj = extractServersObject(parsed); if (!serversObj) { continue; } diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index c832700..ad7e2f7 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -541,6 +541,28 @@ describe("audit fix", () => { } }); + it("uses the report's global canonical root from a project working directory", async () => { + const fixture = await makeAuthorizedFixture(); + const project = join(fixture.home, "project"); + try { + await mkdir(join(project, ".ai", "mcp"), { recursive: true }); + const result = await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: project, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + expect(await Bun.file(fixture.localPath).exists()).toBe(true); + expect( + await Bun.file( + join(project, ".ai", "mcp", "servers.local.json") + ).exists() + ).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + it("keeps the CLI dry-run path zero-write", async () => { const trackedPath = join(rootDir!, "mcp", "servers.json"); const localPath = join(rootDir!, "mcp", "servers.local.json"); diff --git a/src/audit/fix.ts b/src/audit/fix.ts index 17d1b7e..e78a773 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -2,7 +2,7 @@ import { closeSync } from "node:fs"; import { homedir } from "node:os"; import { basename, dirname, join, normalize } from "node:path"; import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; -import { facultContextRootDir } from "../paths"; +import { facultRootDir } from "../paths"; import { parseJsonLenient } from "../util/json"; import type { AgentAuditReport } from "./agent"; import { @@ -601,12 +601,7 @@ export async function runAuditFix(args: { selections: staticSelections, }); const firstBinding = bindings[0]!; - const currentRoot = normalize( - facultContextRootDir({ - home: args.homeDir ?? homedir(), - cwd: args.cwd, - }) - ); + const currentRoot = normalize(facultRootDir(args.homeDir ?? homedir())); if (currentRoot !== firstBinding.canonicalRootPath) { throw new Error( "Current audit fix scope does not match the report-authorized canonical root" diff --git a/src/scan.test.ts b/src/scan.test.ts index 6daa804..e37dfeb 100644 --- a/src/scan.test.ts +++ b/src/scan.test.ts @@ -245,6 +245,32 @@ test("scan --from detects Factory tool surfaces", async () => { expect(allMcpPaths).toContain(join(toolDir, "mcp.json")); }); +test("scan uses the runtime-active MCP container for mixed JSON configs", async () => { + const dir = await mkdtemp(join(tmpdir(), "facult-scan-")); + const home = join(dir, "home"); + const toolDir = join(dir, ".factory"); + const configPath = join(toolDir, "mcp.json"); + await mkdir(toolDir, { recursive: true }); + await Bun.write( + configPath, + `${JSON.stringify({ + servers: { active: { command: "active-command" } }, + "mcp.servers": { inactive: { command: "inactive-command" } }, + })}\n` + ); + + const res = await scan([], { + cwd: dir, + homeDir: home, + includeConfigFrom: false, + from: [dir], + }); + const config = res.sources + .flatMap((source) => source.mcp.configs) + .find((candidate) => candidate.path === configPath); + expect(config?.servers).toEqual(["active"]); +}); + test("scan --from discovers per-project .vscode/settings.json MCP servers (JSONC)", async () => { const dir = await mkdtemp(join(tmpdir(), "facult-scan-")); const home = join(dir, "home"); diff --git a/src/scan.ts b/src/scan.ts index e365a93..1991e6d 100644 --- a/src/scan.ts +++ b/src/scan.ts @@ -3,6 +3,7 @@ import { createHash } from "node:crypto"; import { lstat, mkdir, readdir, realpath } from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { extractServersObject } from "./mcp-config"; import { type FacultConfig, facultRootDir, @@ -352,7 +353,7 @@ async function discoverMcpConfig( try { const text = readText ? await readText(p) : await Bun.file(p).text(); const parsed = parseJsonLenient(text); - const serversObj = extractMcpServersObject(parsed); + const serversObj = extractServersObject(parsed); if (serversObj) { cfg.servers = uniqueSorted(Object.keys(serversObj)); } @@ -2017,37 +2018,6 @@ function sha256Hex(text: string): string { return createHash("sha256").update(text).digest("hex"); } -function extractMcpServersObject( - parsed: unknown -): Record | null { - if (!isPlainObject(parsed)) { - return null; - } - const obj = parsed as Record; - if (isPlainObject(obj.mcpServers)) { - return obj.mcpServers as Record; - } - // Some VS Code-like settings store this under a tool-prefixed key. - for (const [k, v] of Object.entries(obj)) { - if (k.endsWith(".mcpServers") && isPlainObject(v)) { - return v as Record; - } - } - if (isPlainObject(obj["mcp.servers"])) { - return obj["mcp.servers"] as Record; - } - if (isPlainObject(obj.servers)) { - return obj.servers as Record; - } - if (isPlainObject(obj.mcp)) { - const mcp = obj.mcp as Record; - if (isPlainObject(mcp.servers)) { - return mcp.servers as Record; - } - } - return null; -} - function mcpSafeDefinitionText(definition: unknown): string { // Best-effort sanitization: include structural fields and env keys, not env values. if (!isPlainObject(definition)) { @@ -2115,7 +2085,7 @@ async function computeMcpDefinitionVariantCounts( continue; } - const serversObj = extractMcpServersObject(parsed); + const serversObj = extractServersObject(parsed); if (!serversObj) { continue; } From 1cc3ca633a51ea8634392cd529337965ff29cb01 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:31:08 -0400 Subject: [PATCH 08/13] fix(audit): skip invalid MCP containers --- src/audit/fix.test.ts | 17 +++++++++++++---- src/mcp-config.ts | 20 ++++++++++---------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index ad7e2f7..b1ce335 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -129,7 +129,6 @@ async function makeAuthorizedFixture( localMode?: number; localServer?: Record; serverName?: string; - sourceContainer?: "mcp.servers" | "servers"; sourceRoot?: Record; } ) { @@ -138,11 +137,10 @@ async function makeAuthorizedFixture( const trackedPath = join(root, "mcp", "servers.json"); const localPath = join(root, "mcp", "servers.local.json"); const serverName = options?.serverName ?? "github"; - const sourceContainer = options?.sourceContainer ?? "servers"; await writeJson( trackedPath, options?.sourceRoot ?? { - [sourceContainer]: { + servers: { [serverName]: { command: "fixture-command", env: { GITHUB_PERSONAL_ACCESS_TOKEN: marker }, @@ -462,7 +460,17 @@ describe("audit fix", () => { it("remediates the auditor-supported dotted MCP server container", async () => { const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { - sourceContainer: "mcp.servers", + sourceRoot: { + servers: "unrelated metadata", + "mcp.servers": { + github: { + command: "fixture-command", + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }, }); try { const result = await runAuditFix({ @@ -472,6 +480,7 @@ describe("audit fix", () => { }); expect(result.fixed).toBe(1); expect(await Bun.file(fixture.trackedPath).json()).toEqual({ + servers: "unrelated metadata", "mcp.servers": { github: { command: "fixture-command" } }, }); expect(await Bun.file(fixture.localPath).json()).toEqual({ diff --git a/src/mcp-config.ts b/src/mcp-config.ts index 83b8e9f..0ae23ab 100644 --- a/src/mcp-config.ts +++ b/src/mcp-config.ts @@ -37,16 +37,16 @@ export function extractServersObject( return value; } } - const servers = - (raw.servers as Record | undefined) ?? - (raw.mcpServers as Record | undefined) ?? - (raw["mcp.servers"] as Record | undefined) ?? - ((raw.mcp as Record | undefined)?.servers as - | Record - | undefined) ?? - null; - if (servers && isPlainObject(servers)) { - return servers; + const nestedMcpServers = isPlainObject(raw.mcp) ? raw.mcp.servers : undefined; + for (const candidate of [ + raw.servers, + raw.mcpServers, + raw["mcp.servers"], + nestedMcpServers, + ]) { + if (isPlainObject(candidate)) { + return candidate; + } } return null; } From 7ec99022b3b93cb9e51327201b82e906a266bf3f Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:42:10 -0400 Subject: [PATCH 09/13] fix(audit): preserve unfixable MCP findings --- src/audit/fix.test.ts | 70 +++++++++++++++++++++++++++++++-- src/audit/fix.ts | 11 +++++- src/audit/report-persistence.ts | 4 +- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index b1ce335..1a28eb3 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -127,6 +127,7 @@ async function makeAuthorizedFixture( marker = "fixture_secret_1234567890", options?: { localMode?: number; + localRoot?: Record; localServer?: Record; serverName?: string; sourceRoot?: Record; @@ -148,10 +149,13 @@ async function makeAuthorizedFixture( }, } ); - if (options?.localServer) { - await writeJson(localPath, { - servers: { [serverName]: options.localServer }, - }); + if (options?.localRoot || options?.localServer) { + await writeJson( + localPath, + options.localRoot ?? { + servers: { [serverName]: options.localServer }, + } + ); if (options.localMode !== undefined) { await chmod(localPath, options.localMode); } @@ -428,6 +432,64 @@ describe("audit fix", () => { } }); + it("initializes an exact-bound empty local overlay", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + localRoot: {}, + }); + try { + const result = await runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(result.fixed).toBe(1); + expect(await Bun.file(fixture.localPath).json()).toEqual({ + servers: { + github: { + env: { + GITHUB_PERSONAL_ACCESS_TOKEN: "fixture_secret_1234567890", + }, + }, + }, + }); + } finally { + await fixture.cleanup(); + } + }); + + it("reports unsupported remediation names without creating a binding", async () => { + const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { + serverName: "team/github", + }); + try { + const envelope = (await Bun.file(fixture.exactReportPath).json()) as { + receipt: { remediationBindings: unknown[] }; + report: { results: Array<{ findings: Array<{ ruleId: string }> }> }; + }; + expect(envelope.receipt.remediationBindings).toEqual([]); + expect( + envelope.report.results.flatMap((result) => result.findings) + ).toContainEqual( + expect.objectContaining({ ruleId: "mcp-env-inline-secret" }) + ); + const dryRun = await runAuditFix({ + argv: [ + "--item", + "team/github", + "--report", + fixture.exactReportPath, + "--dry-run", + ], + cwd: fixture.home, + homeDir: fixture.home, + }); + expect(dryRun.matched).toBe(1); + expect(dryRun.fixed).toBe(0); + } finally { + await fixture.cleanup(); + } + }); + it("binds colon-bearing server names without location ambiguity", async () => { const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { serverName: "team:github", diff --git a/src/audit/fix.ts b/src/audit/fix.ts index e78a773..e2497b9 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -375,8 +375,15 @@ function transformBoundMcpConfigs(args: { const destinationRoot: Record = args.destinationContents ? (() => { const parsed = parseJsonLenient(args.destinationContents); - if (!(isPlainObject(parsed) && extractServersObject(parsed))) { - throw new Error("Bound MCP destination has no servers object"); + if (!isPlainObject(parsed)) { + throw new Error("Bound MCP destination is not a JSON object"); + } + if (!extractServersObject(parsed)) { + if (Object.keys(parsed).length === 0) { + parsed.servers = {}; + } else { + throw new Error("Bound MCP destination has no servers object"); + } } return parsed; })() diff --git a/src/audit/report-persistence.ts b/src/audit/report-persistence.ts index bc51c10..271afe7 100644 --- a/src/audit/report-persistence.ts +++ b/src/audit/report-persistence.ts @@ -405,7 +405,9 @@ export function buildMcpRemediationBindings(args: { result.path !== location.configPath || dirname(result.path) !== mcpRoot || !["servers.json", "mcp.json"].includes(basename(result.path)) || - !existingPaths.has(result.path) + !existingPaths.has(result.path) || + !isSingleSafeSegment(location.serverName) || + !isSingleSafeSegment(location.envKey) ) { return []; } From bbfea6e37c85d06416ec29547541825182dd1721 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:53:29 -0400 Subject: [PATCH 10/13] fix(audit): refuse Git-exposed secret overlays --- src/audit/fix.test.ts | 19 +++++++++++++++++++ src/audit/fix.ts | 29 +++++++++++++++++++++++++++-- src/audit/safe-openat.ts | 3 +++ src/audit/static.ts | 14 +++++++++----- 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 1a28eb3..4682072 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -490,6 +490,25 @@ describe("audit fix", () => { } }); + it("refuses a destination that enters a Git worktree after the report", async () => { + const fixture = await makeAuthorizedFixture(); + try { + const trackedBefore = await Bun.file(fixture.trackedPath).text(); + await mkdir(join(fixture.home, ".git")); + await expect( + runAuditFix({ + argv: ["mcp:github", "--report", fixture.exactReportPath, "--yes"], + cwd: fixture.home, + homeDir: fixture.home, + }) + ).rejects.toThrow("outside a Git worktree"); + expect(await Bun.file(fixture.trackedPath).text()).toBe(trackedBefore); + expect(await Bun.file(fixture.localPath).exists()).toBe(false); + } finally { + await fixture.cleanup(); + } + }); + it("binds colon-bearing server names without location ambiguity", async () => { const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { serverName: "team:github", diff --git a/src/audit/fix.ts b/src/audit/fix.ts index e2497b9..1f0e9f0 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -1,6 +1,6 @@ -import { closeSync } from "node:fs"; +import { closeSync, lstatSync } from "node:fs"; import { homedir } from "node:os"; -import { basename, dirname, join, normalize } from "node:path"; +import { basename, dirname, join, normalize, resolve } from "node:path"; import { extractServersObject, isInlineMcpSecretValue } from "../mcp-config"; import { facultRootDir } from "../paths"; import { parseJsonLenient } from "../util/json"; @@ -359,6 +359,28 @@ function isPlainObject(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); } +function assertOutsideGitWorktree(pathValue: string): void { + let current = dirname(resolve(pathValue)); + while (true) { + const marker = join(current, ".git"); + try { + lstatSync(marker); + throw new Error( + "Audit fix destination must remain outside a Git worktree" + ); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } + const parent = dirname(current); + if (parent === current) { + return; + } + current = parent; + } +} + function transformBoundMcpConfigs(args: { bindings: AuditMcpRemediationBinding[]; destinationContents: string | null; @@ -608,6 +630,7 @@ export async function runAuditFix(args: { selections: staticSelections, }); const firstBinding = bindings[0]!; + assertOutsideGitWorktree(firstBinding.destinationPath); const currentRoot = normalize(facultRootDir(args.homeDir ?? homedir())); if (currentRoot !== firstBinding.canonicalRootPath) { throw new Error( @@ -652,6 +675,8 @@ export async function runAuditFix(args: { } await validateAuditSourceSnapshot(snapshot); await replaceBoundPrivateFilePairAt({ + assertCommitPolicy: () => + assertOutsideGitWorktree(firstBinding.destinationPath), beforeSourceCommit: args.beforeSourceCommit, destinationIdentity, destinationName: basename(firstBinding.destinationPath), diff --git a/src/audit/safe-openat.ts b/src/audit/safe-openat.ts index f3f2169..ba1231f 100644 --- a/src/audit/safe-openat.ts +++ b/src/audit/safe-openat.ts @@ -970,6 +970,7 @@ function sameObjectIdentity(left: Stats, right: Stats): boolean { } export async function replaceBoundPrivateFilePairAt(args: { + assertCommitPolicy?: () => void; /** @internal Adversarial test hook; production callers must not set this. */ beforeSourceCommit?: () => Promise; destinationIdentity: PrivateFileReceiptIdentity | null; @@ -1342,6 +1343,7 @@ export async function replaceBoundPrivateFilePairAt(args: { throw new Error("Bound private source changed before commit"); } assertMappedObject(sourceName, sourceBefore); + args.assertCommitPolicy?.(); if (destinationBefore) { if (!sameFileIdentity(destinationBefore, fstatSync(destinationFd))) { throw new Error("Bound private destination changed before commit"); @@ -1373,6 +1375,7 @@ export async function replaceBoundPrivateFilePairAt(args: { } assertMappedObject(sourceName, sourceBefore); assertMappedObject(destinationName, destinationTemporaryMetadata); + args.assertCommitPolicy?.(); exchange(sourceTemporaryName, sourceName); sourceCommitted = true; assertMappedObject(sourceName, sourceTemporaryMetadata); diff --git a/src/audit/static.ts b/src/audit/static.ts index 4b83732..645e4df 100644 --- a/src/audit/static.ts +++ b/src/audit/static.ts @@ -796,6 +796,8 @@ export async function evaluateStaticAudit(opts?: { await sourceTracker.capture(canonicalMcpRoot); await sourceTracker.capture(join(canonicalMcpRoot, "servers.local.json")); await sourceTracker.capture(join(canonicalMcpRoot, "mcp.local.json")); + const canonicalMcpExposure = + await sourceTracker.recordGitPathExposure(canonicalMcpRoot); for (const source of res.sources) { for (const pathValue of [ ...source.evidence, @@ -1201,11 +1203,13 @@ export async function evaluateStaticAudit(opts?: { } const sourceSnapshot = sourceTracker.snapshot(); await validateAuditSourceSnapshot(sourceSnapshot); - const remediationBindings = buildMcpRemediationBindings({ - canonicalRootPath: canonicalRoot, - report, - sourceSnapshot, - }); + const remediationBindings = canonicalMcpExposure.insideRepo + ? [] + : buildMcpRemediationBindings({ + canonicalRootPath: canonicalRoot, + report, + sourceSnapshot, + }); return { auditedRoots: Array.from(new Set(auditedRoots)).sort(), remediationBindings, From 336b2b51acedbf873039475b975765603c324988 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 17:58:02 -0400 Subject: [PATCH 11/13] fix(audit): handle absent canonical MCP roots --- src/audit/static.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/audit/static.ts b/src/audit/static.ts index 645e4df..8d2f719 100644 --- a/src/audit/static.ts +++ b/src/audit/static.ts @@ -796,8 +796,14 @@ export async function evaluateStaticAudit(opts?: { await sourceTracker.capture(canonicalMcpRoot); await sourceTracker.capture(join(canonicalMcpRoot, "servers.local.json")); await sourceTracker.capture(join(canonicalMcpRoot, "mcp.local.json")); - const canonicalMcpExposure = - await sourceTracker.recordGitPathExposure(canonicalMcpRoot); + const canonicalMcpExists = sourceTracker + .snapshot() + .evaluatedDirectories.some( + (identity) => identity.path === canonicalMcpRoot + ); + const canonicalMcpExposure = canonicalMcpExists + ? await sourceTracker.recordGitPathExposure(canonicalMcpRoot) + : null; for (const source of res.sources) { for (const pathValue of [ ...source.evidence, @@ -1203,13 +1209,14 @@ export async function evaluateStaticAudit(opts?: { } const sourceSnapshot = sourceTracker.snapshot(); await validateAuditSourceSnapshot(sourceSnapshot); - const remediationBindings = canonicalMcpExposure.insideRepo - ? [] - : buildMcpRemediationBindings({ - canonicalRootPath: canonicalRoot, - report, - sourceSnapshot, - }); + const remediationBindings = + canonicalMcpExposure && !canonicalMcpExposure.insideRepo + ? buildMcpRemediationBindings({ + canonicalRootPath: canonicalRoot, + report, + sourceSnapshot, + }) + : []; return { auditedRoots: Array.from(new Set(auditedRoots)).sort(), remediationBindings, From 3dc73ab364eed5c02db542bf17bcdbdbe7073926 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 18:11:53 -0400 Subject: [PATCH 12/13] docs(audit): document bound MCP remediation --- README.md | 12 ++++++++---- docs/reference.md | 7 +++++-- docs/security-trust.md | 20 +++++++++++++------- src/audit/index.ts | 3 ++- src/audit/tui.test.ts | 6 ++++-- src/audit/tui.ts | 4 ++-- 6 files changed, 34 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index bb10f18..86292ca 100644 --- a/README.md +++ b/README.md @@ -499,6 +499,7 @@ fclt audit fclt audit --non-interactive --severity high fclt audit --non-interactive --report-root /absolute/isolated/reports --json fclt audit fix mcp:github --report /absolute/isolated/reports/static-.json --dry-run +fclt audit fix mcp:github --report /absolute/isolated/reports/static-.json --yes ``` Audit evaluation is read-only by default: it does not refresh saved reports or @@ -511,10 +512,13 @@ Persisted reports are content-addressed authorization envelopes containing the report payload and a receipt that binds its bytes, evaluated source revisions, and finding identities in one atomic file. `audit safe` requires the exact fresh envelope path and `--yes`; legacy `*-latest.json` files and pre-revision-9 -detached report/receipt pairs cannot authorize mutation. `audit fix` currently -supports exact-report `--dry-run` inspection only. Automated MCP mutation fails -closed pending descriptor-bound source and destination authorization through -the final commit boundary. +detached report/receipt pairs cannot authorize mutation. `audit fix --dry-run` +inspects exact matches without writing. For a supported inline MCP secret, +`audit fix --yes` moves only the report-authorized value from the bound +canonical source into a bound owner-only local overlay. The descriptor-relative +transaction revalidates both objects and their ancestors at the final commit +boundary, refuses Git-worktree destinations, and fails closed on drift or +replacement without writing external tool homes. Keep tracked MCP config secret-free. Use local overlays such as `mcp/servers.local.json` for machine-specific secrets. diff --git a/docs/reference.md b/docs/reference.md index 06b8994..6d0f921 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -205,8 +205,11 @@ content-addressed report-and-receipt envelope only to a pre-existing, non-symlinked root that does not overlap any evaluated source. `audit safe` requires `--report --yes`; legacy latest reports and detached pre-revision-9 pairs are never trusted for mutation. `audit fix` -supports exact-report `--dry-run` inspection only and automated MCP mutation -fails closed pending a descriptor-bound exact-source/destination commit design. +uses `--dry-run` for zero-write inspection. With explicit `--yes`, supported +inline MCP secrets are moved only from the exact report-bound canonical source +to its bound owner-only local overlay. The descriptor-relative transaction +revalidates source, destination, ancestors, permissions, and identity at the +final commit boundary, and refuses Git-worktree destinations. `--update-index` is a separate explicit canonical generated-state mutation. `self-update` detects release-script, npm/Bun, and mise-managed npm installs. diff --git a/docs/security-trust.md b/docs/security-trust.md index 61c77a2..042798b 100644 --- a/docs/security-trust.md +++ b/docs/security-trust.md @@ -115,8 +115,8 @@ as a separate explicit mutation. Verified-envelope loading is descriptor-bound and bounded before allocation. Oversize, sparse, growing, multiply linked, non-private, or identity-changing -envelopes fail closed before they can authorize `audit safe` mutation or an -`audit fix --dry-run` preview. +envelopes fail closed before they can authorize `audit safe`, an `audit fix` +mutation, or a zero-write fix preview. Persistence currently requires native descriptor-relative `openat`/`linkat` support (macOS or Linux). Other platforms fail closed rather than falling back @@ -152,10 +152,14 @@ Compatibility note: older releases refreshed `.ai/.facult/audit/*-latest.json` and generated index audit annotations during every audit. Those implicit writes are removed. Legacy saved `*-latest.json` reports remain inspection artifacts only; they do not authorize `audit safe` -or the former automated `audit fix` path. `audit safe` mutations require an +or `audit fix`. `audit safe` mutations require an exact fresh content-addressed report, its receipt, and explicit `--yes` -approval. `audit fix` is limited to zero-write `--dry-run` inspection pending -mutation-time descriptor binding. +approval. `audit fix --dry-run` remains zero-write. Supported `audit fix --yes` +remediation holds the exact report-authorized canonical MCP source and local +destination open with no-follow descriptors, stages owner-only bytes, and +revalidates object identity, permissions, ancestors, source bytes, and the +outside-Git policy at the atomic commit boundary. Drift, replacement, or +cross-root redirection fails closed with rollback and no external artifacts. Root cause of the old behavior: the static and agent library runners wrote their latest reports before returning; both non-interactive CLI wrappers then @@ -164,19 +168,21 @@ runners; and the typed MCP audit routed to the non-interactive CLI. Read-only entry points therefore shared persistence code instead of merely evaluating the scanned source. -Suppress or preview reviewed findings: +Suppress, preview, or remediate reviewed findings: ```bash fclt audit safe mcp:github --rule static:mcp-env-inline-secret --note "reviewed" \ --report /absolute/isolated/audit-reports/static-.json --yes fclt audit fix mcp:github \ --report /absolute/isolated/audit-reports/static-.json --dry-run +fclt audit fix mcp:github \ + --report /absolute/isolated/audit-reports/static-.json --yes ``` Receipts fail closed when their schema/capability revision, report hash, finding identities, source path identity or content revision, or 15-minute freshness window does not match. Supply both exact reports with repeated -`--report` for a combined static/agent safe action or fix dry-run. +`--report` for a combined static/agent safe action or fix selection. ## Secrets diff --git a/src/audit/index.ts b/src/audit/index.ts index 22db8d0..f2e925b 100644 --- a/src/audit/index.ts +++ b/src/audit/index.ts @@ -10,6 +10,7 @@ function printHelp() { Usage: fclt audit [--from ] [--no-config-from] fclt audit fix --report --dry-run [--path ] [--source ] + fclt audit fix --report --yes [--path ] [--source ] fclt audit safe --report --yes [--rule ] [--location ] [--message ] fclt audit --non-interactive [name|mcp:] [--severity ] [--rules ] [--from ] [--json] fclt audit --non-interactive ... --report-root @@ -20,7 +21,7 @@ Safety: - Audit evaluation is read-only by default and writes no report or index state. - --report-root writes one content-addressed -.json report-and-receipt envelope outside every audited source. - safe requires that exact fresh envelope; legacy *-latest.json and detached older pairs never authorize mutation. - - fix supports exact-report dry-run inspection only; automated MCP mutation fails closed pending a descriptor-bound exact-source/destination commit design. + - fix dry-runs without writing; --yes moves supported inline MCP secrets only through an exact-report-bound descriptor-relative commit. - --update-index is a separate explicit mutation of canonical generated state. - Report roots that overlap, traverse, alias, or symlink are rejected. diff --git a/src/audit/tui.test.ts b/src/audit/tui.test.ts index fc0dff4..a729854 100644 --- a/src/audit/tui.test.ts +++ b/src/audit/tui.test.ts @@ -48,8 +48,10 @@ describe("audit TUI arguments", () => { reviewMode: "static", }); - expect(prompt).toContain("--dry-run` only to inspect exact matches"); - expect(prompt).toContain("then propose the manual remediation"); + expect(prompt).toContain( + "--dry-run`; use the same exact report with `--yes`" + ); + expect(prompt).toContain("only after explicit user approval"); expect(prompt).toContain( "Do not mutate MCP config or secrets without explicit user approval" ); diff --git a/src/audit/tui.ts b/src/audit/tui.ts index 1199fe5..75a95d2 100644 --- a/src/audit/tui.ts +++ b/src/audit/tui.ts @@ -317,12 +317,12 @@ export function buildReviewerPrompt(args: { "- Propose the safest order to handle them.", "- If a fix is straightforward and does not change MCP config or secrets, suggest or implement it in this session.", "- Prefer fixing the canonical `.ai` source once when the same MCP issue appears in multiple tool configs.", - "- If an MCP secret needs remediation, use `fclt audit fix ... --dry-run` only to inspect exact matches, then propose the manual remediation.", + "- If an inline MCP secret needs remediation, inspect exact matches with `fclt audit fix ... --dry-run`; use the same exact report with `--yes` only after explicit user approval.", "- Do not mutate MCP config or secrets without explicit user approval.", "", "Useful `fclt` commands in this repo:", "- `fclt show mcp:` to inspect the canonical MCP entry.", - "- `fclt audit fix --report --dry-run` to inspect inline MCP secret matches without writing.", + "- `fclt audit fix --report --dry-run` to inspect inline MCP secret matches, or the same command with `--yes` after explicit approval.", "- `fclt audit safe ...` to suppress a reviewed false positive.", "- `fclt manage --dry-run` or `fclt sync [tool] --dry-run` to inspect deprecated managed rendering without changing tool state.", "", From 216d6331c48d8b16207955a292344515679449f3 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Wed, 15 Jul 2026 18:28:08 -0400 Subject: [PATCH 13/13] fix(audit): distinguish duplicate MCP sources --- src/audit/fix.test.ts | 55 +++++++++++++++++++++++++++++++++++++++++++ src/audit/fix.ts | 1 + 2 files changed, 56 insertions(+) diff --git a/src/audit/fix.test.ts b/src/audit/fix.test.ts index 4682072..61fe72c 100644 --- a/src/audit/fix.test.ts +++ b/src/audit/fix.test.ts @@ -405,6 +405,61 @@ describe("audit fix", () => { } }); + it("refuses duplicate findings that require different source transactions", async () => { + const home = await makeTempHome(); + const root = join(home, ".ai"); + const mcpRoot = join(root, "mcp"); + const canonicalPath = join(mcpRoot, "servers.json"); + const legacyPath = join(mcpRoot, "mcp.json"); + const localPath = join(mcpRoot, "servers.local.json"); + const reportRoot = await mkdtemp( + join(tmpdir(), "fclt-audit-fix-duplicate-") + ); + try { + const server = (secret: string) => ({ + servers: { + github: { + command: "fixture-command", + env: { GITHUB_PERSONAL_ACCESS_TOKEN: secret }, + }, + }, + }); + await writeJson(canonicalPath, server("canonical_secret_1234567890")); + await writeJson(legacyPath, server("legacy_secret_1234567890")); + const audit = await evaluateStaticAudit({ + argv: [], + cwd: home, + from: [root], + homeDir: home, + includeConfigFrom: false, + minSeverity: "high", + }); + const exactReportPath = await persistAuditReport({ + ...audit, + mode: "static", + reportRoot, + }); + const canonicalBefore = await Bun.file(canonicalPath).text(); + const legacyBefore = await Bun.file(legacyPath).text(); + + await expect( + runAuditFix({ + argv: ["mcp:github", "--report", exactReportPath, "--yes"], + cwd: home, + homeDir: home, + }) + ).rejects.toThrow( + "Selected findings do not share one exact bound MCP transaction" + ); + expect(await Bun.file(canonicalPath).text()).toBe(canonicalBefore); + expect(await Bun.file(legacyPath).text()).toBe(legacyBefore); + expect(await Bun.file(localPath).exists()).toBe(false); + } finally { + await rm(home, { force: true, recursive: true }); + await rm(reportRoot, { force: true, recursive: true }); + } + }); + it("secures an existing readable local destination to owner-only mode", async () => { const fixture = await makeAuthorizedFixture("fixture_secret_1234567890", { localMode: 0o644, diff --git a/src/audit/fix.ts b/src/audit/fix.ts index 1f0e9f0..3a72e71 100644 --- a/src/audit/fix.ts +++ b/src/audit/fix.ts @@ -189,6 +189,7 @@ function findingKey(args: { return [ args.result.type, args.result.item, + args.result.path, parsed?.serverName ?? "", parsed?.envKey ?? "", normalizeRuleId(args.finding.ruleId),