diff --git a/docs/MCP.md b/docs/MCP.md index 90f82c4..12c1024 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -108,6 +108,30 @@ A workspace-file config names *a process to spawn*. A hostile repo shipping `.le - Servers from **user settings** start without prompting (the user typed them), but are still listed. - The consent card shows the literal command line — no summarizing. +**Shipped (S4b).** `approveMcpLaunch` (`agent.js`) gates every non-`settings` server; +`kind:'mcpLaunch'` renders the card. Trust lives in `workspaceState` under +`levelcode.ai.mcpLaunchTrust` as `{ serverName: launchFingerprint }`. + +Two details the one-line rule above does not carry, both load-bearing: + +- **Trust is keyed on the fingerprint of what would RUN, not on the server's name.** Otherwise a repo + gets consent for `npx …server-filesystem` and then swaps in `sh -c 'curl … | sh'` under the same + name. Changing the command, args, *or* env re-prompts. +- **`env` is part of that fingerprint**, because it is part of the execution surface: + `NODE_OPTIONS=--require /tmp/evil.js` is RCE without touching command or args at all. It is shown on + the card for the same reason. + +- **The fingerprint is SHA-256**, not the `shortHash` used for tool-name truncation. That helper is a + 32-bit djb2 emitted as 6 base36 chars (~2^31), and here the attacker knows the trusted value — they + authored the command that earned trust — and controls the replacement, so a second preimage *is* the + attack. Measured at ~6.8M candidate hashes/sec on one core, that is roughly five minutes of offline + work to forge a malicious command that inherits trust. Env pairs are encoded structurally + (`[[k, v]]`, sorted) rather than joined into `k=v`, which would make `{'a': 'b=c'}` and `{'a=b': 'c'}` + collide for free. + +The gate **fails closed**: with no webview there is nobody to ask, so the server does not start. A +headless or test context must never be the path that silently spawns a repo's process. + ### G2 — Per-call approval Every MCP tool call goes through `ctx.approve({ kind: 'mcp', … })` by default. The webview branches on `kind` (`chat.html:1440-1466`), so this needs a third card variant showing **server · tool · arguments**. @@ -153,8 +177,10 @@ today, `agent.js:40`, `:65`); the MCP router goes immediately before the `unknow (`agent.js:442`) — the one line every MCP call necessarily passes; an `agentTool` chip announces the servers, mirroring the project-rules chip (`agent.js:493`). -**S4 — trust + approval UX.** The `kind:'mcp'` approval card, the G1 trust-on-first-use flow, and the -autopilot policy. This is the slice that must not be skipped to "get it working." +**S4 — trust + approval UX. DONE.** The slice that must not be skipped to "get it working." +- **S4a** — the `kind:'mcp'` per-call approval card and the autopilot policy (G2, G3). +- **S4b** — the G1 trust-on-first-use launch gate, which is what finally lets a `.levelcode/mcp.json` + server start at all. With it, every gate in §4 is enforced. **S5 — visibility.** `/mcp` slash command (a near-copy of `/skills`: `chat.html:2124` → `extension.js:1297`), and an `mcp` segment in the context-usage popover (`contextUsage` already carries a diff --git a/extensions/levelcode-ai/agent.js b/extensions/levelcode-ai/agent.js index 526ea7b..0ad220a 100644 --- a/extensions/levelcode-ai/agent.js +++ b/extensions/levelcode-ai/agent.js @@ -17,7 +17,8 @@ const providers = require('./providers/index'); const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady } = require('./verify'); const { classifyCommand, dangerLabel } = require('./commandSafety'); const { loadProjectRules } = require('./projectRules'); -const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall } = require('./mcpConfig'); +const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall, + isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch } = require('./mcpConfig'); const { connectAll, getServer } = require('./mcpClient'); const SYSTEM_BASE = [ @@ -545,6 +546,56 @@ function isAgentAuthError(e) { * * Never throws: MCP is an enhancement, and no server misconfiguration may take down an agent run. */ +/** + * G1 launch gate for ONE repo-authored server. Returns true if it may be spawned. + * + * Trust is per workspace and keyed on the fingerprint of what would run, so a repo that was approved + * once cannot later swap the command, args, or env under the same server name — that reads as a new + * server and asks again. + * + * Fails CLOSED. With no webview there is nobody to ask, so the server does not start; a headless or + * test context must never be the path that spawns a repo's process silently. + */ +async function approveMcpLaunch(ctx, server, dbg) { + const store = (ctx.mcp && ctx.mcp.launchTrust) || {}; + if (isLaunchTrusted(server, store)) { + dbg('mcp.launch.trusted', { server: server.name }); + return true; + } + + const card = describeMcpLaunch(server); + if (typeof ctx.approve !== 'function') { + dbg('mcp.launch.nonInteractive', { server: server.name }); + ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · "' + server.name + '" not started — repo-defined servers need approval, and there is no prompt in this context' }); + return false; + } + + dbg('mcp.launch.prompt', { server: server.name, fingerprint: card.fingerprint }); + const approved = await ctx.approve({ + kind: 'mcpLaunch', + server: card.server, + origin: card.origin, + commandLine: card.commandLine, + envLines: card.envLines + }); + + if (!approved) { + dbg('mcp.launch.declined', { server: server.name }); + ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · "' + server.name + '" not started (declined)' }); + return false; + } + + // Remembered only on approval, and only for this workspace. Best-effort: failing to persist means + // the user is asked again next run, which is the safe direction to fail. + if (typeof ctx.rememberMcpTrust === 'function') { + try { await ctx.rememberMcpTrust(rememberLaunchTrust(server, store)); } catch (e) { + dbg('mcp.launch.rememberFailed', { server: server.name, error: String((e && e.message) || e) }); + } + } + ctx.post({ type: 'agentTool', icon: 'check', text: '🔌 mcp · trusted "' + server.name + '" for this workspace' }); + return true; +} + async function setupMcp(ctx, wsFolders, dbg) { const empty = { tools: [], routes: null }; const cfg = ctx.mcp || {}; @@ -557,11 +608,13 @@ async function setupMcp(ctx, wsFolders, dbg) { for (const p of problems) { dbg('mcp.config', p); } if (!servers.length) { return empty; } - const deferred = servers.filter((s) => s.source !== 'settings'); - if (deferred.length) { - ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · ' + deferred.length + ' workspace server(s) not started — repo-defined servers need an approval step that ships later' }); - } + // G1. Settings servers start unprompted — the user typed them. Repo-authored ones go through + // trust-on-first-use, per server, per workspace, keyed on what they would actually spawn. const trusted = servers.filter((s) => s.source === 'settings'); + for (const s of servers.filter((s) => s.source !== 'settings')) { + const ok = await approveMcpLaunch(ctx, s, dbg); + if (ok) { trusted.push(s); } + } if (!trusted.length) { return empty; } // Connecting is up-front work: the tool list must be complete before turn one, so there is no diff --git a/extensions/levelcode-ai/extension.js b/extensions/levelcode-ai/extension.js index ce1918e..e778b13 100644 --- a/extensions/levelcode-ai/extension.js +++ b/extensions/levelcode-ai/extension.js @@ -792,6 +792,21 @@ function isPlainObject(v) { return proto === Object.prototype || proto === null; } +// G1 launch trust for repo-authored MCP servers: { serverName: launchFingerprint }. +// workspaceState keeps it scoped to this workspace, so trusting a server in one repo grants nothing in +// another. safeCopy on the way out because it round-trips through stored JSON. +const MCP_TRUST_KEY = 'levelcode.ai.mcpLaunchTrust'; + +function mcpLaunchTrust() { + try { return safeCopy(ctx.workspaceState.get(MCP_TRUST_KEY, {}) || {}); } catch { return {}; } +} + +async function saveMcpLaunchTrust(store) { + try { await ctx.workspaceState.update(MCP_TRUST_KEY, safeCopy(store || {})); } catch (e) { + dbg('mcp.launch.persistFailed', { error: String((e && e.message) || e) }); + } +} + async function mcpAllowAlways(name) { // isNamespacedToolName owns the rule (mcpConfig.js), rather than a second regex here: this used to // hand-roll one that required a `__` separator, which REJECTED names namespaceToolName legitimately @@ -1109,8 +1124,13 @@ async function agentFlow(text) { // application-scoped in package.json; this is the defense-in-depth half. See userScopedSetting. mcp: { servers: userScopedSetting(cfg.inspect('mcp.servers'), {}), - toolPolicy: userScopedSetting(cfg.inspect('mcp.toolPolicy'), {}) + toolPolicy: userScopedSetting(cfg.inspect('mcp.toolPolicy'), {}), + // G1 trust for repo-authored servers. workspaceState, NOT settings: consenting to a + // server in one repo must say nothing about another repo that declares one by the + // same name, and workspaceState is per-workspace by construction. + launchTrust: mcpLaunchTrust() }, + rememberMcpTrust: saveMcpLaunchTrust, contextLimit: contextLimitFor(req.providerId, capsModel(req.model)), // Auto → flagship window; the model SENT stays req.model openPreview: openPreview, // background server advertised a local URL → show it in-editor commandStops: commandStops, // runId → stop() (process-group kill); used by Stop button / ■ diff --git a/extensions/levelcode-ai/mcpConfig.js b/extensions/levelcode-ai/mcpConfig.js index 6ff6aaf..7888f9f 100644 --- a/extensions/levelcode-ai/mcpConfig.js +++ b/extensions/levelcode-ai/mcpConfig.js @@ -15,17 +15,20 @@ * * TRUST: a server entry names A PROCESS TO SPAWN. The user's setting is user-authored. A workspace * file is REPO-authored — i.e. attacker-controlled for any repo you clone — so entries from it are - * marked source:'workspace' and MUST NOT be started without explicit consent (the launch gate lives - * in a later slice; this module only reports the provenance it needs). For the same reason the + * marked source:'workspace' and MUST NOT be started without explicit consent — the trust-on-first-use + * gate at the bottom of this file (launchFingerprint / isLaunchTrusted / describeMcpLaunch), enforced + * by approveMcpLaunch in agent.js. For the same reason the * user's setting WINS on a name collision: a repo can never shadow a server the user defined. * - * Pure + dependency-free (path only) — file reading is injected as a readFile callback, so all of it + * Pure + dependency-free (node builtins `path` and `crypto` only) — file reading is injected as a + * readFile callback, so all of it * is unit-testable (test/mcpConfig.test.js) without a filesystem, a child process, or the editor. * Nothing here connects, spawns, or calls anything. *--------------------------------------------------------------------------------------------*/ 'use strict'; const path = require('path'); +const crypto = require('crypto'); // The agent's built-in tools (agent.js TOOLS). An MCP tool may never shadow one of these. const BUILTIN_TOOL_NAMES = [ @@ -487,6 +490,108 @@ function previewArgs(args) { * @param {{server?:string, tool?:string, annotations?:object}} [route] * @returns {{server:string, tool:string, argsText:string, destructive:boolean, canAllowAlways:boolean}} */ +// ---- G1: trust-on-first-use for repo-authored servers ---------------------- +// A `.levelcode/mcp.json` entry names a process to spawn, and the file is attacker-controlled for any +// repo you clone. These four functions are the launch gate: fingerprint what would be spawned, compare +// it to what this workspace has already trusted, and describe it for the consent card. + +/** + * A stable fingerprint of what a server entry would actually EXECUTE. + * + * Trust is remembered against this, not against the server's NAME, so a repo cannot be granted consent + * for `npx @modelcontextprotocol/server-filesystem` and then quietly swap in `sh -c 'curl … | sh'` under + * the same name — the fingerprint changes and the user is asked again. + * + * `env` is included, and that is not padding: `NODE_OPTIONS=--require /tmp/evil.js` turns an innocent + * `node` command into arbitrary code execution without touching command or args. Keys are sorted so an + * unrelated reordering of the JSON does not spuriously revoke trust. + * + * SHA-256, NOT the shortHash used for tool-name truncation. shortHash is a 32-bit djb2 variant emitted + * as 6 base36 chars — a ~2^31 space, and it is not collision-resistant by design or intent. Here the + * attacker both KNOWS the trusted value (they authored the command that earned trust) and controls the + * replacement, so they need a second preimage — measured at ~6.8M candidate hashes/sec on one core, + * i.e. roughly five minutes of offline work to forge a malicious command that inherits trust. A + * truncation helper is the wrong tool for an authorization decision; the cost of a real hash here is + * one call per server per run. + */ +/** + * The launch material, normalized ONCE — what would be executed, in canonical form. + * + * Shared by launchFingerprint and describeMcpLaunch on purpose. They were normalizing separately, and + * they drifted: the fingerprint learned to survive a non-array `args` while the card kept calling + * `.map` on it and threw. A consent card and the trust record it produces must describe the same thing, + * so they read it from the same place. + * + * MALFORMED SHAPES COLLAPSE TO `null`, which is the same value an ABSENT field gets. That is + * deliberate and conservative: normalizeServer rejects a non-array `args` or non-object `env` long + * before a server reaches the gate, so neither is reachable here, and "no usable args" is the honest + * reading of both. The command itself always differentiates. (An earlier comment claimed malformed and + * absent stayed distinct — they do not, and the tests assert the collapse.) + * + * Env pairs stay STRUCTURAL — [[k, v]] sorted — never joined into "k=v". Joining is ambiguous: + * { 'a': 'b=c' } and { 'a=b': 'c' } both flatten to "a=b=c", a collision handed over for free in the + * one place collisions are the threat. + */ +function launchMaterial(server) { + const s = server || {}; + const env = (s.env && typeof s.env === 'object' && !Array.isArray(s.env)) ? s.env : null; + return { + command: String(s.command || ''), + args: Array.isArray(s.args) ? s.args.map(String) : null, + env: env ? Object.keys(env).sort().map((k) => [k, String(env[k])]) : null + }; +} + +function launchFingerprint(server) { + return crypto.createHash('sha256') + .update(JSON.stringify(launchMaterial(server)), 'utf8') + .digest('hex'); +} + +/** + * Has THIS workspace already approved launching exactly this server? + * + * `store` is a plain `{ serverName: fingerprint }` map held in workspaceState, so trust is per-workspace + * by construction: approving a server in one repo says nothing about another repo that happens to + * declare a server by the same name. + */ +function isLaunchTrusted(server, store) { + if (!server || !server.name) { return false; } + const known = store && store[server.name]; + return typeof known === 'string' && known === launchFingerprint(server); +} + +/** Record trust for one server. Pure: returns the new store, so the caller owns persistence. */ +function rememberLaunchTrust(server, store) { + const next = safeCopy(store || {}); + if (server && server.name) { next[server.name] = launchFingerprint(server); } + return next; +} + +/** + * The consent card's data. docs/MCP.md G1: "shows the literal command line — no summarizing." + * + * So `commandLine` is the real thing, quoted only where an argument contains a space (otherwise + * `--path /a b` reads as two arguments when it is one). Env is surfaced separately as NAME=value, + * because it is part of the execution surface the user is consenting to and hiding it would make the + * card a half-truth. + */ +function describeMcpLaunch(server) { + const s = server || {}; + // Read from launchMaterial, not from `server` directly. Doing its own normalization is what let this + // throw on a string `args` (`.map` is not a function) and render `0=e 1=v 2=i 3=l` for a string + // `env` — junk on the one card whose whole purpose is showing the user exactly what will run. + const material = launchMaterial(s); + const quote = (a) => (/[\s"']/.test(a) ? JSON.stringify(a) : a); + return { + server: String(s.name || ''), + origin: String(s.origin || ''), + commandLine: [material.command].concat((material.args || []).map(quote)).join(' ').trim(), + envLines: (material.env || []).map(([k, v]) => k + '=' + v), + fingerprint: launchFingerprint(s) + }; +} + function describeMcpCall(name, args, route) { const r = route || {}; const fallback = String(name == null ? '' : name).split(NAME_SEPARATOR); @@ -500,5 +605,6 @@ module.exports = { loadServerConfig, userScopedSetting, namespaceToolName, isNamespacedToolName, assignToolNames, buildAgentTools, safeCopy, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall, + launchFingerprint, isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch, BUILTIN_TOOL_NAMES, MAX_TOOL_NAME, MAX_TOOL_DESC, MAX_ARG_CHARS, MAX_SERVERS, MAX_TOOLS_PER_SERVER, WORKSPACE_CONFIG_PATH }; diff --git a/extensions/levelcode-ai/media/chat.html b/extensions/levelcode-ai/media/chat.html index e47857e..4df11d1 100644 --- a/extensions/levelcode-ai/media/chat.html +++ b/extensions/levelcode-ai/media/chat.html @@ -1876,6 +1876,51 @@ let pendingApproval = null; // { done } while a decision is awaited — Enter approves, Esc skips // MCP tool call (S4) — its own card: server · tool · arguments, so the user sees exactly what a // third-party tool is about to do. Args are shown in full (capped host-side): that IS the decision. + // G1 consent card: a repo-authored .levelcode/mcp.json wants to SPAWN A PROCESS. This is the one + // prompt where the stakes are RCE-on-clone, so it shows the literal command line — docs/MCP.md G1 + // says "no summarizing" — plus any env it would set, since NODE_OPTIONS alone is enough to turn an + // innocent-looking `node` into arbitrary code. + // + // There is no "always allow" escape hatch by design: trust is remembered against a fingerprint of + // exactly this command, so approving is already the durable answer, and a second, vaguer button + // would only blur what was agreed to. + function addMcpLaunchApproval(m){ + clearEmpty(); clearStatus(); agentBubble = null; + closeGroup(); + const card = document.createElement('div'); card.className = 'tl tl-cmd tl-ask asking'; + const envWell = (m.envLines && m.envLines.length) + ? '
' + esc(m.envLines.join('\n')) + '' + esc(m.commandLine || '') + '
' + esc(m.server || '') + ''
+ + '' + codicon(approved ? 'check-circle' : 'circle-slash') + '