From 96e7027d75f4599442bbf0963aca0e215acac9e6 Mon Sep 17 00:00:00 2001 From: Conal <33135619+Conalh@users.noreply.github.com> Date: Thu, 21 May 2026 19:12:56 -0700 Subject: [PATCH] Wire normalizeMcpCommand into mcp_command_mismatch detector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the false-positive class flagged in the PolicyMesh audit: two surfaces that differ only in cosmetically neutral ways (`npx -y ` vs `npx `, `.cmd` vs unsuffixed, flag reordering) were being reported as high-severity command mismatches. What changed - McpServer gains a `canonicalIdentity: string` field, computed by agent-gov-core@v0.1.2's normalizeMcpCommand from (command, args, url). Both the JSON and Codex TOML parsers populate it. - `detectMcpCommandMismatch` now groups by `canonicalIdentity` instead of the raw joined `command` string. The human-readable command list in the finding message still uses `command` so the finding stays actionable. - Env is deliberately omitted from `canonicalIdentity`. Env drift has its own detector (mcp_env_mismatch); including env here would have surfaced two findings for what users perceive as one issue (and broke the mcp-env-value-mismatch fixture test). Regression test pinned `mcp-command-neutral-flag-equivalence` fixture: root MCP runs `npx -y @modelcontextprotocol/server-github@1.2.3`, Cursor runs the same without `-y`. Before this change the audit emitted a high-severity mcp_command_mismatch finding; after it emits none. Test 'CLI does not flag mcp_command_mismatch on neutral -y flag drift between surfaces' asserts the post-fix behavior — it fails on the pre-fix engine, passes here. 39 PolicyMesh tests pass. Stacked on #40 (JSONC migration); merge that one first. Co-Authored-By: Claude Opus 4.7 --- dist/mesh/engine.js | 17 +++++++++---- dist/parsers/codex.js | 6 +++++ dist/parsers/mcp.js | 6 +++++ package-lock.json | 6 ++--- package.json | 2 +- src/mesh/engine.ts | 17 +++++++++---- src/parsers/codex.ts | 6 +++++ src/parsers/mcp.ts | 6 +++++ src/types.ts | 11 +++++++++ test/cli-output.test.mjs | 24 +++++++++++++++++++ .../.cursor/mcp.json | 8 +++++++ .../.mcp.json | 8 +++++++ 12 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 test/fixtures/mcp-command-neutral-flag-equivalence/.cursor/mcp.json create mode 100644 test/fixtures/mcp-command-neutral-flag-equivalence/.mcp.json diff --git a/dist/mesh/engine.js b/dist/mesh/engine.js index d41de9e..89ecc29 100644 --- a/dist/mesh/engine.js +++ b/dist/mesh/engine.js @@ -50,16 +50,23 @@ function detectMcpCommandMismatch(policies) { const findings = []; const byName = groupMcpServersByName(policies); for (const [name, servers] of byName) { - const commands = new Map(); + // Group by canonical identity, not raw command string, so neutral + // differences (npx -y vs npx, .cmd/.exe suffix, flag reordering) + // don't produce false-positive mcp_command_mismatch findings. + const byIdentity = new Map(); for (const server of servers) { - const existing = commands.get(server.command) ?? []; + const existing = byIdentity.get(server.canonicalIdentity) ?? []; existing.push(server); - commands.set(server.command, existing); + byIdentity.set(server.canonicalIdentity, existing); } - if (commands.size <= 1) { + if (byIdentity.size <= 1) { continue; } - const commandList = [...commands.keys()].map((cmd) => `"${cmd}"`).join(' vs '); + // Message still shows the user-visible commands so the finding is + // actionable — even though grouping was on canonical identity. + const commandList = [...new Set(servers.map((s) => s.command))] + .map((cmd) => `"${cmd}"`) + .join(' vs '); const primary = servers[0]; findings.push({ kind: 'mcp_command_mismatch', diff --git a/dist/parsers/codex.js b/dist/parsers/codex.js index 5cef9ff..6e42ad1 100644 --- a/dist/parsers/codex.js +++ b/dist/parsers/codex.js @@ -1,4 +1,5 @@ import { readFile } from 'node:fs/promises'; +import { normalizeMcpCommand } from 'agent-gov-core'; import { configPath } from '../discovery.js'; import { isUnpinnedCommand, serverCommand } from './mcp.js'; import { configParseFinding } from './errors.js'; @@ -289,6 +290,11 @@ function buildCodexMcpServers(drafts) { servers.push({ name, command, + canonicalIdentity: normalizeMcpCommand({ + command: draft.command, + args: draft.args, + url: draft.url ?? draft.serverUrl, + }), enabled: draft.enabled !== false, env: draft.env, headers: draft.headers, diff --git a/dist/parsers/mcp.js b/dist/parsers/mcp.js index 83bf804..00bad3f 100644 --- a/dist/parsers/mcp.js +++ b/dist/parsers/mcp.js @@ -1,3 +1,4 @@ +import { normalizeMcpCommand } from 'agent-gov-core'; import { configPath, isRecord, lineOfJsonKey, readJsonObjectWithSource } from '../discovery.js'; import { configParseFinding } from './errors.js'; const MCP_CONFIGS = [ @@ -65,6 +66,11 @@ async function readMcpServers(root, config) { servers.push({ name, command, + canonicalIdentity: normalizeMcpCommand({ + command: raw.command, + args: raw.args, + url: raw.url ?? raw.serverUrl, + }), enabled: serverEnabled(raw), env: raw.env ?? {}, headers: raw.headers ?? {}, diff --git a/package-lock.json b/package-lock.json index ea2e9e4..d51b0f6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.1.18", "license": "MIT", "dependencies": { - "agent-gov-core": "github:Conalh/agent-gov-core#v0.1.1" + "agent-gov-core": "github:Conalh/agent-gov-core#v0.1.2" }, "bin": { "policymesh": "dist/index.js" @@ -30,8 +30,8 @@ } }, "node_modules/agent-gov-core": { - "version": "0.1.0", - "resolved": "git+ssh://git@github.com/Conalh/agent-gov-core.git#503b30f5aebf2eb0ebe6f4a60cd3cde14068670b", + "version": "0.1.2", + "resolved": "git+ssh://git@github.com/Conalh/agent-gov-core.git#db05618adbd84a503df9b475c49ad86ff161c62e", "license": "MIT", "engines": { "node": ">=20" diff --git a/package.json b/package.json index 3b0a841..64ccb70 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "test": "node --test" }, "dependencies": { - "agent-gov-core": "github:Conalh/agent-gov-core#v0.1.1" + "agent-gov-core": "github:Conalh/agent-gov-core#v0.1.2" }, "devDependencies": { "@types/node": "^24.0.0", diff --git a/src/mesh/engine.ts b/src/mesh/engine.ts index 298d428..c2a5fc0 100644 --- a/src/mesh/engine.ts +++ b/src/mesh/engine.ts @@ -62,18 +62,25 @@ function detectMcpCommandMismatch(policies: RepoPolicies): Finding[] { const byName = groupMcpServersByName(policies); for (const [name, servers] of byName) { - const commands = new Map(); + // Group by canonical identity, not raw command string, so neutral + // differences (npx -y vs npx, .cmd/.exe suffix, flag reordering) + // don't produce false-positive mcp_command_mismatch findings. + const byIdentity = new Map(); for (const server of servers) { - const existing = commands.get(server.command) ?? []; + const existing = byIdentity.get(server.canonicalIdentity) ?? []; existing.push(server); - commands.set(server.command, existing); + byIdentity.set(server.canonicalIdentity, existing); } - if (commands.size <= 1) { + if (byIdentity.size <= 1) { continue; } - const commandList = [...commands.keys()].map((cmd) => `"${cmd}"`).join(' vs '); + // Message still shows the user-visible commands so the finding is + // actionable — even though grouping was on canonical identity. + const commandList = [...new Set(servers.map((s) => s.command))] + .map((cmd) => `"${cmd}"`) + .join(' vs '); const primary = servers[0]; findings.push({ kind: 'mcp_command_mismatch', diff --git a/src/parsers/codex.ts b/src/parsers/codex.ts index d754a0d..4f9045b 100644 --- a/src/parsers/codex.ts +++ b/src/parsers/codex.ts @@ -1,4 +1,5 @@ import { readFile } from 'node:fs/promises'; +import { normalizeMcpCommand } from 'agent-gov-core'; import { configPath } from '../discovery.js'; import { isUnpinnedCommand, serverCommand } from './mcp.js'; import { configParseFinding } from './errors.js'; @@ -362,6 +363,11 @@ function buildCodexMcpServers(drafts: Map): McpServ servers.push({ name, command, + canonicalIdentity: normalizeMcpCommand({ + command: draft.command, + args: draft.args, + url: draft.url ?? draft.serverUrl, + }), enabled: draft.enabled !== false, env: draft.env, headers: draft.headers, diff --git a/src/parsers/mcp.ts b/src/parsers/mcp.ts index f0f4b8b..fe10e7a 100644 --- a/src/parsers/mcp.ts +++ b/src/parsers/mcp.ts @@ -1,3 +1,4 @@ +import { normalizeMcpCommand } from 'agent-gov-core'; import { configPath, isRecord, lineOfJsonKey, readJsonObjectWithSource } from '../discovery.js'; import { configParseFinding } from './errors.js'; import type { Finding, McpServer, McpSurface, SurfaceId } from '../types.js'; @@ -104,6 +105,11 @@ async function readMcpServers( servers.push({ name, command, + canonicalIdentity: normalizeMcpCommand({ + command: raw.command, + args: raw.args, + url: raw.url ?? raw.serverUrl, + }), enabled: serverEnabled(raw), env: raw.env ?? {}, headers: raw.headers ?? {}, diff --git a/src/types.ts b/src/types.ts index 41c4280..2b74a8b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -29,7 +29,18 @@ export interface FindingLocation { export interface McpServer { name: string; + /** Human-readable launch string. Used in messages/matrix rows only. */ command: string; + /** + * Canonical identity of the launch command from agent-gov-core's + * normalizeMcpCommand, computed *without* env. Two servers with the same + * canonicalIdentity launch the same workload, even if their raw command + * strings differ in neutral ways (flag reordering, `-y`/`--yes`, + * `.cmd`/`.exe` suffix). The mismatch detector groups by this field, + * not by `command`. Env differences are reported separately by + * mcp_env_mismatch and intentionally excluded here. + */ + canonicalIdentity: string; enabled: boolean; env: Record; headers: Record; diff --git a/test/cli-output.test.mjs b/test/cli-output.test.mjs index b5657ae..4993ad9 100644 --- a/test/cli-output.test.mjs +++ b/test/cli-output.test.mjs @@ -337,6 +337,30 @@ test('CLI reports Codex MCP server command drift against root MCP config', async assert.ok(report.matrix.some((row) => row.capability === 'MCP: github' && row.values.codex?.includes('@modelcontextprotocol/server-github@2.0.0'))); }); +test('CLI does not flag mcp_command_mismatch on neutral -y flag drift between surfaces', async () => { + // Regression for the PolicyMesh audit's false-positive class: + // root MCP uses `npx -y `, Cursor uses `npx `. `-y` only + // suppresses npx's install prompt — it doesn't change what runs. + // Pre-fix, this fixture produced a high-severity mcp_command_mismatch + // because the detector grouped by the raw joined command string. + // Post-fix, the detector groups by normalizeMcpCommand canonical + // identity, which drops `-y`/`--yes`, so the surfaces are equivalent. + const repo = join(testDir, 'fixtures', 'mcp-command-neutral-flag-equivalence'); + + const { stdout } = await execFileAsync( + process.execPath, + ['dist/index.js', 'audit', '--repo', repo, '--format', 'json'], + { cwd: packageRoot } + ); + const report = JSON.parse(stdout); + + const mismatchFindings = report.findings.filter( + (finding) => finding.kind === 'mcp_command_mismatch' + ); + assert.deepEqual(mismatchFindings, [], 'expected no mcp_command_mismatch findings'); + assert.equal(report.surfaceCount, 2); +}); + test('CLI reports Codeium plugin MCP command drift against root MCP config', async () => { const repo = join(testDir, 'fixtures', 'codeium-plugin-mcp-config'); diff --git a/test/fixtures/mcp-command-neutral-flag-equivalence/.cursor/mcp.json b/test/fixtures/mcp-command-neutral-flag-equivalence/.cursor/mcp.json new file mode 100644 index 0000000..0535813 --- /dev/null +++ b/test/fixtures/mcp-command-neutral-flag-equivalence/.cursor/mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "github": { + "command": "npx", + "args": ["@modelcontextprotocol/server-github@1.2.3"] + } + } +} diff --git a/test/fixtures/mcp-command-neutral-flag-equivalence/.mcp.json b/test/fixtures/mcp-command-neutral-flag-equivalence/.mcp.json new file mode 100644 index 0000000..6808998 --- /dev/null +++ b/test/fixtures/mcp-command-neutral-flag-equivalence/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github@1.2.3"] + } + } +}