From 75a0bfdf085db755ac0163ede31b790cefb66d55 Mon Sep 17 00:00:00 2001 From: Conal <33135619+Conalh@users.noreply.github.com> Date: Fri, 22 May 2026 16:20:34 -0700 Subject: [PATCH] Detect active remote MCP endpoints with scheme-aware severity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the active-config gap I noted in the previous HTTP severity commit (4082373): mcp_sample_remote_endpoint fired on sample configs but the active .mcp.json / .codex/config.toml server loops weren't scrutinizing remote endpoints at all. A `serverUrl` change on an active MCP server got nothing — even though active configs are the ones actually carrying traffic, where unencrypted transport is a real-time exfiltration risk, not just a copy-paste foot-gun. Adds two new findings, severity calibrated higher than the sample equivalents because these endpoints are live: - scope_trail.mcp_remote_endpoint http:// → critical (live unencrypted MCP transport) https:// → high (any active remote endpoint warrants review) - scope_trail.codex_mcp_remote_endpoint Same shape for `[mcp_servers.NAME]` in .codex/config.toml. Supports both `serverUrl` and `server_url` (Codex uses snake_case). Refactor: `remoteEndpoint`, `isRemoteEndpoint`, `isUnencryptedEndpoint` extracted to mcp-risk.ts so both detectors share one source of truth for the endpoint policy. Severity ladder across surfaces (lowest to highest): - sample https:// → medium (copy-paste risk only) - sample http:// → high (sample silently teaches MitM) - active https:// → high (live remote transport, encrypted) - active http:// → critical (live remote transport, unencrypted) Test coverage: dedicated regression tests in mcp-drift.test.mjs and codex-mcp-drift.test.mjs pin the http/https split for both detectors. Existing Windsurf active-drift test updated to include the new mcp_remote_endpoint finding on the team-registry serverUrl change. --- dist/detectors/codex-config.js | 26 +++++++++++++++-- dist/detectors/mcp.js | 42 ++++++++++++---------------- dist/mcp-risk.js | 23 +++++++++++++++ src/detectors/codex-config.ts | 33 ++++++++++++++++++++-- src/detectors/mcp.ts | 51 +++++++++++++++++----------------- src/mcp-risk.ts | 26 +++++++++++++++++ test/codex-mcp-drift.test.mjs | 37 ++++++++++++++++++++++++ test/mcp-drift.test.mjs | 39 ++++++++++++++++++++++++++ 8 files changed, 222 insertions(+), 55 deletions(-) diff --git a/dist/detectors/codex-config.js b/dist/detectors/codex-config.js index 925e939..6cccb4c 100644 --- a/dist/detectors/codex-config.js +++ b/dist/detectors/codex-config.js @@ -1,7 +1,7 @@ import { readFile } from 'node:fs/promises'; import { lineOfTomlKey, parseToml } from 'agent-gov-core'; import { configPath } from '../discovery.js'; -import { isUnpinnedCommand, serverCommand } from '../mcp-risk.js'; +import { isUnpinnedCommand, serverCommand, remoteEndpoint, isUnencryptedEndpoint } from '../mcp-risk.js'; export const CODEX_CONFIG_FILE = '.codex/config.toml'; export const CODEX_TARGET_PATHS = [CODEX_CONFIG_FILE]; export async function detectCodexConfigDrift(oldRoot, newRoot) { @@ -114,6 +114,23 @@ async function detectCodexMcpDrift(oldRoot, newRoot) { recommendation: 'Pin executable packages to an exact version and avoid pipe-to-shell installation commands.' }); } + const endpoint = remoteEndpoint(newServer); + if ((!oldServer || commandChanged) && endpoint) { + const unencrypted = isUnencryptedEndpoint(endpoint); + findings.push({ + kind: 'scope_trail.codex_mcp_remote_endpoint', + severity: unencrypted ? 'critical' : 'high', + file: CODEX_CONFIG_FILE, + line: lineForServer(newServer), + subject: name, + message: unencrypted + ? `Codex MCP server "${name}" points at an unencrypted remote endpoint: ${endpoint}.` + : `Codex MCP server "${name}" points at remote endpoint: ${endpoint}.`, + recommendation: unencrypted + ? 'Use https:// for remote MCP endpoints — prompt data and tool executions must not go over unencrypted transport.' + : 'Confirm the endpoint is trusted and does not expose unexpected data or tools to external hosts.' + }); + } } return findings; } @@ -152,7 +169,10 @@ async function readCodexMcpServers(root) { args: Array.isArray(entry.args) ? entry.args.filter((arg) => typeof arg === 'string') : undefined, - url: typeof entry.url === 'string' ? entry.url : undefined + url: typeof entry.url === 'string' ? entry.url : undefined, + serverUrl: typeof entry.serverUrl === 'string' + ? entry.serverUrl + : (typeof entry.server_url === 'string' ? entry.server_url : undefined) }); } return servers; @@ -161,7 +181,7 @@ function lineForServer(server) { // Point at the leaf the reviewer most needs to see — `command` // first, then any of the args/url keys. Fall back to file-level // when nothing matches so the finding still surfaces. - for (const leaf of ['command', 'args', 'url']) { + for (const leaf of ['command', 'args', 'url', 'serverUrl', 'server_url']) { const line = lineOfTomlKey(server.text, `mcp_servers.${server.name}.${leaf}`); if (line) { return line; diff --git a/dist/detectors/mcp.js b/dist/detectors/mcp.js index 9823048..9929d01 100644 --- a/dist/detectors/mcp.js +++ b/dist/detectors/mcp.js @@ -1,6 +1,6 @@ import { readdir } from 'node:fs/promises'; import { configPath, isRecord, lineOfJsonKey, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js'; -import { isPipeToShellCommand, isUnpinnedCommand, serverCommand } from '../mcp-risk.js'; +import { isPipeToShellCommand, isUnpinnedCommand, serverCommand, remoteEndpoint, isRemoteEndpoint, isUnencryptedEndpoint } from '../mcp-risk.js'; const MCP_CONFIGS = [ { path: '.mcp.json', serverKeys: ['mcpServers'] }, { path: '.cursor/mcp.json', serverKeys: ['mcpServers', 'servers'] }, @@ -87,6 +87,23 @@ export async function detectMcpDrift(oldRoot, newRoot) { recommendation: 'Pin executable packages to an exact version and avoid pipe-to-shell installation commands.' }); } + const endpoint = remoteEndpoint(newServer); + if ((!oldServer || serverCommand(newServer) !== serverCommand(oldServer)) && endpoint) { + const unencrypted = isUnencryptedEndpoint(endpoint); + findings.push({ + kind: 'scope_trail.mcp_remote_endpoint', + severity: unencrypted ? 'critical' : 'high', + file: config.path, + line: lineForRemoteEndpoint(newServer) ?? newServer.line, + subject: name, + message: unencrypted + ? `MCP server "${name}" points at an unencrypted remote endpoint: ${endpoint}.` + : `MCP server "${name}" points at remote endpoint: ${endpoint}.`, + recommendation: unencrypted + ? 'Use https:// for remote MCP endpoints — prompt data and tool executions must not go over unencrypted transport.' + : 'Confirm the endpoint is trusted and does not expose unexpected data or tools to external hosts.' + }); + } } } for (const path of await listMcpSampleConfigPaths(oldRoot, newRoot)) { @@ -284,29 +301,6 @@ function lineForUnpinnedCommand(server) { function lineForRemoteEndpoint(server) { return firstLineForValues(server, [server.url, server.serverUrl], isRemoteEndpoint); } -function remoteEndpoint(server) { - return [server.url, server.serverUrl].find((value) => Boolean(value && isRemoteEndpoint(value))); -} -function isRemoteEndpoint(value) { - try { - const url = new URL(value); - if (url.protocol !== 'http:' && url.protocol !== 'https:') { - return false; - } - return !['localhost', '127.0.0.1', '::1'].includes(url.hostname); - } - catch { - return false; - } -} -function isUnencryptedEndpoint(value) { - try { - return new URL(value).protocol === 'http:'; - } - catch { - return false; - } -} function firstLineForValues(server, values, predicate = () => true) { const source = getSourceText(server); for (const value of values) { diff --git a/dist/mcp-risk.js b/dist/mcp-risk.js index 56390da..a90320a 100644 --- a/dist/mcp-risk.js +++ b/dist/mcp-risk.js @@ -59,3 +59,26 @@ function hasExactVersion(value) { const version = value.slice(packageVersion + 1); return /^\d+\.\d+\.\d+/.test(version); } +export function remoteEndpoint(spec) { + return [spec.url, spec.serverUrl].find((value) => Boolean(value && isRemoteEndpoint(value))); +} +export function isRemoteEndpoint(value) { + try { + const url = new URL(value); + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + return false; + } + return !['localhost', '127.0.0.1', '::1'].includes(url.hostname); + } + catch { + return false; + } +} +export function isUnencryptedEndpoint(value) { + try { + return new URL(value).protocol === 'http:'; + } + catch { + return false; + } +} diff --git a/src/detectors/codex-config.ts b/src/detectors/codex-config.ts index 282450b..0f59499 100644 --- a/src/detectors/codex-config.ts +++ b/src/detectors/codex-config.ts @@ -1,7 +1,13 @@ import { readFile } from 'node:fs/promises'; import { lineOfTomlKey, parseToml } from 'agent-gov-core'; import { configPath } from '../discovery.js'; -import { isUnpinnedCommand, serverCommand, type McpCommandShape } from '../mcp-risk.js'; +import { + isUnpinnedCommand, + serverCommand, + remoteEndpoint, + isUnencryptedEndpoint, + type McpCommandShape +} from '../mcp-risk.js'; import type { Finding } from '../types.js'; export const CODEX_CONFIG_FILE = '.codex/config.toml'; @@ -136,6 +142,24 @@ async function detectCodexMcpDrift(oldRoot: string, newRoot: string): Promise typeof arg === 'string') : undefined, - url: typeof entry.url === 'string' ? entry.url : undefined + url: typeof entry.url === 'string' ? entry.url : undefined, + serverUrl: typeof entry.serverUrl === 'string' + ? entry.serverUrl + : (typeof entry.server_url === 'string' ? entry.server_url : undefined) }); } @@ -189,7 +216,7 @@ function lineForServer(server: CodexMcpServer): number | undefined { // Point at the leaf the reviewer most needs to see — `command` // first, then any of the args/url keys. Fall back to file-level // when nothing matches so the finding still surfaces. - for (const leaf of ['command', 'args', 'url']) { + for (const leaf of ['command', 'args', 'url', 'serverUrl', 'server_url']) { const line = lineOfTomlKey(server.text, `mcp_servers.${server.name}.${leaf}`); if (line) { return line; diff --git a/src/detectors/mcp.ts b/src/detectors/mcp.ts index 018af2b..ee023fa 100644 --- a/src/detectors/mcp.ts +++ b/src/detectors/mcp.ts @@ -1,6 +1,13 @@ import { readdir } from 'node:fs/promises'; import { configPath, isRecord, lineOfJsonKey, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js'; -import { isPipeToShellCommand, isUnpinnedCommand, serverCommand } from '../mcp-risk.js'; +import { + isPipeToShellCommand, + isUnpinnedCommand, + serverCommand, + remoteEndpoint, + isRemoteEndpoint, + isUnencryptedEndpoint +} from '../mcp-risk.js'; import type { Finding, McpServerConfig, Severity } from '../types.js'; const MCP_CONFIGS = [ @@ -106,6 +113,24 @@ export async function detectMcpDrift(oldRoot: string, newRoot: string): Promise< recommendation: 'Pin executable packages to an exact version and avoid pipe-to-shell installation commands.' }); } + + const endpoint = remoteEndpoint(newServer); + if ((!oldServer || serverCommand(newServer) !== serverCommand(oldServer)) && endpoint) { + const unencrypted = isUnencryptedEndpoint(endpoint); + findings.push({ + kind: 'scope_trail.mcp_remote_endpoint', + severity: unencrypted ? 'critical' : 'high', + file: config.path, + line: lineForRemoteEndpoint(newServer) ?? newServer.line, + subject: name, + message: unencrypted + ? `MCP server "${name}" points at an unencrypted remote endpoint: ${endpoint}.` + : `MCP server "${name}" points at remote endpoint: ${endpoint}.`, + recommendation: unencrypted + ? 'Use https:// for remote MCP endpoints — prompt data and tool executions must not go over unencrypted transport.' + : 'Confirm the endpoint is trusted and does not expose unexpected data or tools to external hosts.' + }); + } } } @@ -345,30 +370,6 @@ function lineForRemoteEndpoint(server: McpServerModel): number | undefined { return firstLineForValues(server, [server.url, server.serverUrl], isRemoteEndpoint); } -function remoteEndpoint(server: McpServerModel): string | undefined { - return [server.url, server.serverUrl].find((value): value is string => Boolean(value && isRemoteEndpoint(value))); -} - -function isRemoteEndpoint(value: string): boolean { - try { - const url = new URL(value); - if (url.protocol !== 'http:' && url.protocol !== 'https:') { - return false; - } - - return !['localhost', '127.0.0.1', '::1'].includes(url.hostname); - } catch { - return false; - } -} - -function isUnencryptedEndpoint(value: string): boolean { - try { - return new URL(value).protocol === 'http:'; - } catch { - return false; - } -} function firstLineForValues( server: McpServerModel, diff --git a/src/mcp-risk.ts b/src/mcp-risk.ts index 7fc8265..9f1e161 100644 --- a/src/mcp-risk.ts +++ b/src/mcp-risk.ts @@ -79,3 +79,29 @@ function hasExactVersion(value: string): boolean { const version = value.slice(packageVersion + 1); return /^\d+\.\d+\.\d+/.test(version); } + +export function remoteEndpoint(spec: McpCommandShape): string | undefined { + return [spec.url, spec.serverUrl].find((value): value is string => Boolean(value && isRemoteEndpoint(value))); +} + +export function isRemoteEndpoint(value: string): boolean { + try { + const url = new URL(value); + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + return false; + } + + return !['localhost', '127.0.0.1', '::1'].includes(url.hostname); + } catch { + return false; + } +} + +export function isUnencryptedEndpoint(value: string): boolean { + try { + return new URL(value).protocol === 'http:'; + } catch { + return false; + } +} + diff --git a/test/codex-mcp-drift.test.mjs b/test/codex-mcp-drift.test.mjs index 071f5ed..1b41a60 100644 --- a/test/codex-mcp-drift.test.mjs +++ b/test/codex-mcp-drift.test.mjs @@ -81,3 +81,40 @@ test('codex_mcp_server_command_changed fires when an existing server changes its rmSync(root, { recursive: true, force: true }); } }); + +test('codex_mcp_remote_endpoint: http:// fires critical severity, https:// fires high', async () => { + const { mkdtempSync, writeFileSync, mkdirSync, rmSync } = await import('node:fs'); + const { tmpdir } = await import('node:os'); + + const root = mkdtempSync(join(tmpdir(), 'scopetrail-codex-http-')); + try { + const oldDir = join(root, 'old'); + const newDir = join(root, 'new'); + mkdirSync(join(oldDir, '.codex'), { recursive: true }); + mkdirSync(join(newDir, '.codex'), { recursive: true }); + + writeFileSync( + join(oldDir, '.codex', 'config.toml'), + '[mcp_servers]\n' + ); + writeFileSync( + join(newDir, '.codex', 'config.toml'), + '[mcp_servers.plain-remote]\nserverUrl = "http://mcp.example.com/insecure"\n\n[mcp_servers.tls-remote]\nserverUrl = "https://mcp.example.com/safe"\n' + ); + + const findings = await detectCodexConfigDrift(oldDir, newDir); + const remoteEndpoint = findings.filter((f) => f.kind === 'scope_trail.codex_mcp_remote_endpoint'); + const bySubject = Object.fromEntries(remoteEndpoint.map((f) => [f.subject, f])); + + assert.ok(bySubject['plain-remote'], 'expected active codex remote_endpoint finding for plain-remote'); + assert.equal(bySubject['plain-remote'].severity, 'critical'); + assert.match(bySubject['plain-remote'].message, /unencrypted/); + + assert.ok(bySubject['tls-remote'], 'expected active codex remote_endpoint finding for tls-remote'); + assert.equal(bySubject['tls-remote'].severity, 'high'); + assert.doesNotMatch(bySubject['tls-remote'].message, /unencrypted/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + diff --git a/test/mcp-drift.test.mjs b/test/mcp-drift.test.mjs index bdc6b41..dc12de2 100644 --- a/test/mcp-drift.test.mjs +++ b/test/mcp-drift.test.mjs @@ -157,6 +157,7 @@ test('detects MCP drift in Windsurf config files', async () => { findings.map((finding) => [finding.file, finding.kind, finding.subject, finding.line]), [ ['.codeium/windsurf/mcp_config.json', 'scope_trail.mcp_server_command_changed', 'team-registry', 4], + ['.codeium/windsurf/mcp_config.json', 'scope_trail.mcp_remote_endpoint', 'team-registry', 4], ['.codeium/windsurf/mcp_config.json', 'scope_trail.mcp_server_added', 'browser-tools', 6], ['.codeium/windsurf/mcp_config.json', 'scope_trail.unpinned_mcp_command', 'browser-tools', 8] ] @@ -237,3 +238,41 @@ test('detects prefixed MCP config example drift without treating it as active se ] ); }); + +test('mcp_remote_endpoint: http:// fires critical severity, https:// fires high', async () => { + const { mkdtempSync, writeFileSync, mkdirSync, rmSync } = await import('node:fs'); + const { tmpdir } = await import('node:os'); + + const root = mkdtempSync(join(tmpdir(), 'scopetrail-active-http-')); + try { + const oldDir = join(root, 'old'); + const newDir = join(root, 'new'); + mkdirSync(oldDir, { recursive: true }); + mkdirSync(newDir, { recursive: true }); + writeFileSync(join(oldDir, '.mcp.json'), JSON.stringify({ mcpServers: {} })); + writeFileSync( + join(newDir, '.mcp.json'), + JSON.stringify({ + mcpServers: { + 'plain-remote': { serverUrl: 'http://mcp.example.com/insecure' }, + 'tls-remote': { serverUrl: 'https://mcp.example.com/safe' } + } + }, null, 2) + ); + + const findings = await detectMcpDrift(oldDir, newDir); + const remoteEndpoint = findings.filter((f) => f.kind === 'scope_trail.mcp_remote_endpoint'); + const bySubject = Object.fromEntries(remoteEndpoint.map((f) => [f.subject, f])); + + assert.ok(bySubject['plain-remote'], 'expected active remote_endpoint finding for plain-remote'); + assert.equal(bySubject['plain-remote'].severity, 'critical'); + assert.match(bySubject['plain-remote'].message, /unencrypted/); + + assert.ok(bySubject['tls-remote'], 'expected active remote_endpoint finding for tls-remote'); + assert.equal(bySubject['tls-remote'].severity, 'high'); + assert.doesNotMatch(bySubject['tls-remote'].message, /unencrypted/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); +