diff --git a/docs/MCP.md b/docs/MCP.md index 9b072b8..53dfb36 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -194,8 +194,29 @@ servers, mirroring the project-rules chip (`agent.js:493`). the bar would stop summing to `used`. Hidden entirely when no server contributed one. Every tool schema rides every turn, so this is the standing cost a chatty server imposes, and it was invisible. -**S6 — later.** Streamable HTTP transport + the `2026-07-28` revision; resources/prompts; a -"Manage MCP servers…" QuickPick on the `pickModel` pattern (`extension.js:1165-1228`). +**S6a — "Manage MCP servers…". DONE.** `levelcode.ai.manageMcp`, on the `pickModel` QuickPick pattern, +linked from the foot of `/mcp`. Rows come from the same `mcpOverview()` as S5, so the two views can +never disagree. Three things it adds beyond looking: +- **Add / remove a server** without hand-writing JSON — the last place MCP forced people into a settings + file. It writes the **Global** tier only, never Workspace: `mcp.servers` is `application`-scoped + precisely so a repo cannot introduce a server that starts without consent (G1), and a UI that offered + the workspace tier would quietly undo that. The arguments box takes a **command line**, split by the + quote-aware `parseArgv()` — a whitespace split would break + `-y @modelcontextprotocol/server-filesystem "/Users/me/My Documents"`, which is close to the single + most common MCP server there is. It is a splitter, **not a shell**: no expansion, no globbing, matching + `shell:false` at the spawn site. +- **Revoke G1 trust**, per server or workspace-wide — the missing half of trust-on-first-use. Approving + was write-once with no way back short of editing `workspaceState` by hand, which makes the consent + prompt harder to say yes to than it should be. Revoking does not kill a running server; the wording + says "will ask before starting again" rather than implying otherwise. +- **Names a stale approval.** `summarizeMcp` reports "never approved" and "approved, then the repo + changed the command" both as `trusted:false`. Correct for *starting* the server, wrong for + *explaining* it: the second case is exactly the G1 attack — get something benign approved, then swap + the command. The row says `command changed — needs approval`, and the detail view offers to forget the + dead approval. (`mcpTrustIsStale` / `mcpServerItem`, tested in `test/mcpManage.test.js`.) + +**S6b — later.** Streamable HTTP transport + the `2026-07-28` revision; resources/prompts. +Until HTTP lands, a hosted server is reachable only through a stdio bridge — see §9. --- @@ -227,3 +248,50 @@ servers, mirroring the project-rules chip (`agent.js:493`). Remote/HTTP servers and OAuth · sampling (a server driving our model) · resources & prompts · MCP "apps"/UI extensions · auto-discovery or an in-editor server marketplace. + +--- + +## 9. Recipe: the GitHub MCP server + +The most-asked-for server, and the one that shows where D2 (stdio only) actually bites. **Verified +end-to-end on 2026-07-28** against `ghcr.io/github/github-mcp-server` through `mcpClient.connect`: +handshake 122 ms, **41 tools**, all names legal under D4, a real PR read back, and write access +confirmed. Settings, user tier: + +```jsonc +"levelcode.ai.mcp.servers": { + "github": { + "command": "docker", + "args": ["run", "-i", "--rm", + "-e", "GITHUB_PERSONAL_ACCESS_TOKEN", + "-e", "GITHUB_TOOLSETS", + "ghcr.io/github/github-mcp-server"], + "env": { "GITHUB_TOOLSETS": "pull_requests,repos,issues,context" } + } +} +``` + +Four things worth knowing, each of which is a design constraint rather than a detail: + +- **GitHub's hosted server (`api.githubcopilot.com/mcp/`) is Streamable HTTP, which we do not speak + (D2).** The container above is the same server over stdio. A bridge like `mcp-remote` also works and + is the only way to get the OAuth flow, at the cost of a second process in the chain. +- **`-e NAME` with no `=value` is deliberate.** `connect()` spawns with `Object.assign({}, process.env, + server.env)` (`mcpClient.js:52`), so Docker inherits the token from the editor's environment and the + credential never lands in `settings.json`. Putting the value in the `env` block works too — it is also + a plaintext secret in a synced settings file. This is why the S6a Add wizard has no `env` step. +- **`GITHUB_TOOLSETS` is a context-budget decision, not a preference.** The full server advertises far + more tools, every schema rides every turn, and the S5 context meter will show exactly what that costs. +- **Everything is `ask` by default, including `merge_pull_request`** — no GitHub tool sets + `destructiveHint`, so nothing is force-asked by G2's hard rule, which means the allow-list *can* grant + any of them. `"levelcode.ai.mcp.toolPolicy": { "github__pull_request_read": "allow" }` is the sane + shape: allow-list the reads, keep the writes on the card. `"*": "allow"` would let autopilot merge. + +| Ask | Tool | +| --- | --- | +| read a PR — diff, files, reviews, comments, checks | `github__pull_request_read` (`method` enum, 9 values) | +| create a PR | `github__create_pull_request` | +| update a PR — title, body, base, reviewers, draft | `github__update_pull_request` | +| **close** a PR | `github__update_pull_request` with `state: "closed"` | +| review a PR | `github__pull_request_review_write`, `github__add_comment_to_pending_review` | +| merge a PR | `github__merge_pull_request` | diff --git a/extensions/levelcode-ai/extension.js b/extensions/levelcode-ai/extension.js index 8d0eb09..3652059 100644 --- a/extensions/levelcode-ai/extension.js +++ b/extensions/levelcode-ai/extension.js @@ -27,7 +27,7 @@ const { loadSkills, skillsMenu, getSkillBody } = require('./skills'); const { openCustomize } = require('./customize'); const { importFromVscode } = require('./importVscode'); const { reapMcp, listActive, getServer } = require('./mcpClient'); -const { userScopedSetting, isNamespacedToolName, safeCopy, loadServerConfig, summarizeMcp } = require('./mcpConfig'); +const { userScopedSetting, isNamespacedToolName, safeCopy, loadServerConfig, summarizeMcp, parseArgv, UNSAFE_KEYS } = require('./mcpConfig'); const SECRET_KEY = 'levelcode.ai.anthropicKey'; // legacy Anthropic key location (kept for back-compat) const FILE_EXCLUDES = '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/.vscode-test/**,**/*.map}'; @@ -1350,6 +1350,230 @@ async function pickCloudModel() { } } +// ---- "Manage MCP servers…" (docs/MCP.md S6) -------------------------------- +// The last place MCP forced people into raw JSON, and the only UI for revoking a repo server's G1 +// launch trust — until now that decision was write-once, with no way back short of clearing +// workspaceState by hand. + +/** + * Is this server's G1 approval STALE — approved once, but the command line has changed since? + * + * summarizeMcp collapses "never approved" and "approved, then the repo edited the command" into one + * `trusted:false`, which is right for starting the server and wrong for explaining it. The second case + * is the exact attack G1 exists to stop: get a benign entry approved, then swap the command. It + * deserves to be named rather than shown as a server that was simply never set up. + */ +function mcpTrustIsStale(s, store) { + return s.trusted === false && Object.prototype.hasOwnProperty.call(store || {}, s.name); +} + +/** One row per configured server, on the same shape summarizeMcp already produces for `/mcp`. */ +function mcpServerItem(s, store) { + let state, icon; + if (s.running) { state = 'running'; icon = '$(pass-filled)'; } + else if (mcpTrustIsStale(s, store)) { state = 'command changed — needs approval'; icon = '$(warning)'; } + // A repo server waiting on the consent card is not broken — saying only "not started" would send + // the user debugging a gate that is working. + else if (s.trusted === false) { state = 'needs approval'; icon = '$(shield)'; } + else { state = 'not started'; icon = '$(circle-outline)'; } + + const bits = [ s.source === 'settings' ? 'your settings' : (s.origin || 'workspace'), state ]; + if (s.running) { + bits.push(s.tools + ' tool' + (s.tools === 1 ? '' : 's')); + if (s.allowed != null) { bits.push(s.allowed + ' allow-listed'); } + } + return { label: icon + ' ' + s.name, description: bits.join(' · '), detail: s.commandLine, _server: s }; +} + +async function manageMcpServers() { + const view = mcpOverview(); + const store = mcpLaunchTrust(); + const items = []; + + if (view.servers.length) { + items.push({ label: 'Configured servers', kind: vscode.QuickPickItemKind.Separator }); + for (const s of view.servers) { items.push(mcpServerItem(s, store)); } + } + items.push({ label: view.servers.length ? 'Actions' : 'No servers configured', kind: vscode.QuickPickItemKind.Separator }); + items.push({ label: '$(add) Add a server…', description: 'Write a new entry to your settings', _action: 'add' }); + items.push({ label: '$(edit) Open settings JSON', description: 'levelcode.ai.mcp.servers', _action: 'settings' }); + if (Object.keys(store).length) { + items.push({ label: '$(discard) Revoke all workspace trust…', description: 'Repo-defined servers will ask again before starting', _action: 'revokeAll' }); + } + // Config problems belong here rather than only in `/mcp`: a server the user typed and cannot find is + // most often a rejected entry, and this is where they came to look for it. + if (view.problems.length) { + items.push({ label: 'Problems', kind: vscode.QuickPickItemKind.Separator }); + for (const p of view.problems) { + items.push({ label: '$(warning) ' + p.message, description: p.level, _action: 'settings' }); + } + } + + const pick = await vscode.window.showQuickPick(items, { + placeHolder: view.configured + ? view.configured + ' configured · ' + view.running + ' running' + : 'No MCP servers configured yet', + matchOnDescription: true, + matchOnDetail: true + }); + if (!pick) { return; } + + if (pick._action === 'add') { return mcpAddServer(); } + if (pick._action === 'settings') { return openMcpSettings(); } + if (pick._action === 'revokeAll') { return mcpRevokeTrust(null, Object.keys(store).length); } + if (pick._server) { return mcpServerActions(pick._server, store); } +} + +/** Second level: what you can do to one server. */ +async function mcpServerActions(s, store) { + const items = [ { label: '$(clippy) Copy command line', description: s.commandLine, _action: 'copy' } ]; + + if (s.source === 'settings') { + items.push({ label: '$(edit) Edit in settings JSON', _action: 'settings' }); + items.push({ label: '$(trash) Remove from settings', _action: 'remove' }); + } else if (mcpTrustIsStale(s, store)) { + items.push({ + label: '$(warning) This server\'s command line changed since you approved it', + description: 'Approve the new one on the consent card, or forget the old approval', + _action: 'noop' + }); + items.push({ label: '$(discard) Forget the old approval', _action: 'revoke' }); + } else if (s.trusted) { + items.push({ label: '$(discard) Revoke trust for this workspace', description: 'It will ask again before starting', _action: 'revoke' }); + } else { + items.push({ label: '$(shield) Not yet approved', description: 'The consent card appears when the agent first needs it', _action: 'noop' }); + } + + const pick = await vscode.window.showQuickPick(items, { placeHolder: s.name + ' · ' + (s.source === 'settings' ? 'your settings' : s.origin) }); + if (!pick || pick._action === 'noop') { return; } + if (pick._action === 'copy') { + try { await vscode.env.clipboard.writeText(s.commandLine || ''); } catch (e) { dbg('mcp.manage.copyFailed', { error: String((e && e.message) || e) }); } + return; + } + if (pick._action === 'settings') { return openMcpSettings(); } + if (pick._action === 'revoke') { return mcpRevokeTrust(s.name); } + if (pick._action === 'remove') { return mcpRemoveServer(s.name); } +} + +function openMcpSettings() { + return vscode.commands.executeCommand('workbench.action.openSettingsJson', { revealSetting: { key: 'levelcode.ai.mcp.servers' } }); +} + +/** + * Add a server to the USER settings tier. + * + * Global on purpose, and not offered for the workspace tier: `levelcode.ai.mcp.servers` is declared + * `application` scope precisely so a repo cannot introduce a server that starts without consent + * (docs/MCP.md G1). A UI that wrote it anywhere else would quietly undo that. + */ +async function mcpAddServer() { + const cfg = aiConfig(); + const existing = safeCopy(userScopedSetting(cfg.inspect('mcp.servers'), {}) || {}); + + const name = (await vscode.window.showInputBox({ + title: 'Add an MCP server — name', + prompt: 'Short id, used to namespace its tools as name__tool', + placeHolder: 'github', + ignoreFocusOut: true, + validateInput: (v) => { + const t = String(v || '').trim(); + if (!t) { return 'A name is required.'; } + // Reserved object keys pass the charset rule below but would corrupt the settings map when + // written as existing[name] — existing['__proto__'] = entry hits the prototype setter rather + // than adding a key. Reject them with the same list safeCopy()/normalizeServer() enforce. + if (UNSAFE_KEYS.indexOf(t) !== -1) { return 'That name is reserved — choose another.'; } + if (Object.prototype.hasOwnProperty.call(existing, t)) { return 'A server called "' + t + '" already exists.'; } + // Not a hard rule — namespaceToolName sanitizes anyway — but a name that survives verbatim + // makes the tool names in the transcript readable. + if (!/^[A-Za-z0-9_-]+$/.test(t)) { return 'Use letters, digits, dashes or underscores so the tool names stay readable.'; } + return null; + } + }) || '').trim(); + if (!name) { return; } + + const command = (await vscode.window.showInputBox({ + title: 'Add an MCP server — command', + prompt: 'The executable to run (it is spawned directly, not through a shell)', + placeHolder: 'npx', + ignoreFocusOut: true, + validateInput: (v) => (String(v || '').trim() ? null : 'A command is required.') + }) || '').trim(); + if (!command) { return; } + + const argsLine = await vscode.window.showInputBox({ + title: 'Add an MCP server — arguments', + prompt: 'Space-separated. Quote anything containing spaces, e.g. "/Users/me/My Documents"', + placeHolder: '-y @modelcontextprotocol/server-filesystem /path/to/dir', + ignoreFocusOut: true + }); + if (argsLine === undefined) { return; } // escaped — an empty string is a valid "no arguments" + + const entry = { command: command }; + const args = parseArgv(argsLine); + if (args.length) { entry.args = args; } + + existing[name] = entry; + try { + await cfg.update('mcp.servers', existing, vscode.ConfigurationTarget.Global); + dbg('mcp.manage.added', { name: name }); + } catch (e) { + vscode.window.showErrorMessage('Could not save the MCP server: ' + String((e && e.message) || e)); + return; + } + + // The wizard deliberately does not ask for `env`: the servers that need one need an API TOKEN, and a + // prompt for it would end with a live credential in plaintext settings.json — worse, it would look + // like the recommended way to do it. mcpClient spawns with process.env inherited, so exporting the + // variable and launching the editor from that shell keeps the secret out of any file. Anyone who + // wants it in settings anyway can put it there; this just points at the file instead of pretending + // the wizard covered everything. + const next = await vscode.window.showInformationMessage( + 'Added MCP server "' + name + '". It starts on the next agent run.', + 'Open settings JSON'); + if (next === 'Open settings JSON') { return openMcpSettings(); } +} + +async function mcpRemoveServer(name) { + const ok = await vscode.window.showWarningMessage( + 'Remove the MCP server "' + name + '" from your settings?', { modal: true }, 'Remove'); + if (ok !== 'Remove') { return; } + + const cfg = aiConfig(); + const existing = safeCopy(userScopedSetting(cfg.inspect('mcp.servers'), {}) || {}); + delete existing[name]; + try { + await cfg.update('mcp.servers', existing, vscode.ConfigurationTarget.Global); + dbg('mcp.manage.removed', { name: name }); + } catch (e) { + vscode.window.showErrorMessage('Could not remove the MCP server: ' + String((e && e.message) || e)); + } +} + +/** + * Forget a G1 launch approval — for one server, or all of them in this workspace. + * + * The missing half of trust-on-first-use: approving was permanent with no way back, which makes the + * prompt harder to say yes to than it should be. Revoking does not stop a server already running in + * this session; it means the next run asks again, so the message says exactly that rather than + * implying the process was killed. + */ +async function mcpRevokeTrust(name, count) { + const target = name ? ('trust for "' + name + '"') : ('trust for all ' + count + ' repo-defined server(s)'); + const ok = await vscode.window.showWarningMessage( + 'Revoke ' + target + ' in this workspace? They will ask for approval again before starting.', + { modal: true }, 'Revoke'); + if (ok !== 'Revoke') { return; } + + // Re-read rather than reuse the snapshot the menu was built from: a modal was just open, and + // approving a different server in the meantime must not be silently rolled back by this write. + let next; + if (name) { next = mcpLaunchTrust(); delete next[name]; } else { next = {}; } + await saveMcpLaunchTrust(next); + dbg('mcp.manage.trustRevoked', { server: name || '*' }); + vscode.window.showInformationMessage( + name ? '"' + name + '" will ask before starting again.' : 'Repo-defined servers will ask before starting again.'); +} + async function pickModel() { const cfg = aiConfig(); // Gateway mode + signed in → a plan-scoped LevelCode Cloud menu (free engine + flagship, the latter @@ -1521,6 +1745,7 @@ class ChatViewProvider { } case 'setKey': await promptForKey(); break; case 'pickModel': await pickModel(); break; + case 'manageMcp': await manageMcpServers(); break; case 'openSettings': vscode.commands.executeCommand('workbench.action.openSettings', '@ext:levelcode.levelcode-ai'); break; } }); @@ -1795,6 +2020,7 @@ function activate(context) { vscode.commands.registerCommand('levelcode.import.vscode', () => importFromVscode(context)), vscode.commands.registerCommand('levelcode.ai.newChat', newChat), vscode.commands.registerCommand('levelcode.ai.pickModel', pickModel), + vscode.commands.registerCommand('levelcode.ai.manageMcp', manageMcpServers), vscode.commands.registerCommand('levelcode.ai.addSelection', addSelection), vscode.commands.registerCommand('levelcode.ai.addFileContext', addContext), vscode.commands.registerCommand('levelcode.ai.setApiKey', () => promptForKey()), diff --git a/extensions/levelcode-ai/mcpConfig.js b/extensions/levelcode-ai/mcpConfig.js index d2a8058..c81bd49 100644 --- a/extensions/levelcode-ai/mcpConfig.js +++ b/extensions/levelcode-ai/mcpConfig.js @@ -592,6 +592,57 @@ function describeMcpLaunch(server) { }; } +/** + * Split a typed argument line into the `args` array a server entry needs. + * + * Quote-aware, and that is the point rather than polish: the single most common MCP server on earth is + * `npx -y @modelcontextprotocol/server-filesystem /some/path`, and on macOS that path is very often + * `/Users/me/My Documents/…`. Splitting on whitespace alone turns one argument into three and the + * server fails to start with a message about the wrong directory — a bad first experience caused + * entirely by the input box. + * + * Deliberately NOT a shell parser. No variable expansion, no globbing, no operators: `args` is handed + * to spawn as a literal list, so anything clever here would be a lie about what runs. Quotes group; a + * backslash escapes only a following space, quote, or backslash and is otherwise a literal character — + * so a regex arg like `\bword\b`, a Windows path `C:\Users\me`, or a JSON string survives verbatim + * instead of quietly losing its backslashes. That is all. + */ +function parseArgv(line) { + const s = String(line == null ? '' : line); + const out = []; + let cur = ''; + let quote = ''; // '"' or "'" while inside a quoted run + let started = false; // distinguishes a deliberate empty argument ("") from whitespace + + for (let i = 0; i < s.length; i++) { + const ch = s[i]; + if (ch === '\\' && quote !== "'") { + // A SPLITTER, not a shell: a backslash escapes ONLY a delimiter — a whitespace char, a + // quote, or another backslash. Anything else keeps the backslash literally, so a regex + // (`\bword\b`), a Windows path, or a JSON string is not silently rewritten. (Inside single + // quotes it is always literal — the quote !== "'" guard. A trailing backslash falls through + // and is kept as an ordinary character.) + const nxt = s[i + 1]; + if (nxt !== undefined && (/\s/.test(nxt) || nxt === '"' || nxt === "'" || nxt === '\\')) { + cur += nxt; i++; started = true; continue; + } + // not escaping a delimiter → fall through and treat this backslash as an ordinary char + } + if (quote) { + if (ch === quote) { quote = ''; } else { cur += ch; } + continue; + } + if (ch === '"' || ch === "'") { quote = ch; started = true; continue; } + if (/\s/.test(ch)) { + if (started) { out.push(cur); cur = ''; started = false; } + continue; + } + cur += ch; started = true; + } + if (started) { out.push(cur); } + return out; +} + // ---- S5: visibility -------------------------------------------------------- /** @@ -676,8 +727,8 @@ function describeMcpCall(name, args, route) { module.exports = { loadServerConfig, userScopedSetting, namespaceToolName, isNamespacedToolName, assignToolNames, - buildAgentTools, safeCopy, + buildAgentTools, safeCopy, UNSAFE_KEYS, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall, - launchFingerprint, isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch, summarizeMcp, + launchFingerprint, isLaunchTrusted, rememberLaunchTrust, describeMcpLaunch, summarizeMcp, parseArgv, 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 5ad43e3..cbee342 100644 --- a/extensions/levelcode-ai/media/chat.html +++ b/extensions/levelcode-ai/media/chat.html @@ -3132,7 +3132,7 @@ let inner = '
LevelCode AI
'; if (!m.configured){ - inner += 'No MCP servers configured. Add one under levelcode.ai.mcp.servers, ' + inner += 'No MCP servers configured. Add one with Manage servers… below, ' + 'or commit a .levelcode/mcp.json to a repo — a repo-defined server asks before it starts.'; } else { const bits = [ m.configured + ' configured', m.running + ' running' ]; @@ -3177,7 +3177,13 @@ inner += '
' + esc(p.message) + '
'; } + // `/mcp` answers "what is going on"; every follow-up from here — add one, remove one, take back an + // approval — lives in the QuickPick. Without this link that command is only findable by name. + inner += '
Manage servers…
'; + d.innerHTML = inner + '
'; + const manage = d.querySelector('.mcpmanage'); + if (manage){ manage.onclick = (e) => { e.preventDefault(); vscode.postMessage({ type: 'manageMcp' }); }; } log.appendChild(d); scrollIfStuck(); } diff --git a/extensions/levelcode-ai/package.json b/extensions/levelcode-ai/package.json index ffdba4f..e1cd03e 100644 --- a/extensions/levelcode-ai/package.json +++ b/extensions/levelcode-ai/package.json @@ -119,6 +119,11 @@ "title": "AI: Select Model…", "category": "LevelCode" }, + { + "command": "levelcode.ai.manageMcp", + "title": "AI: Manage MCP Servers…", + "category": "LevelCode" + }, { "command": "levelcode.ai.sketch", "title": "AI: Agent Sketch…", diff --git a/extensions/levelcode-ai/test/mcpConfig.test.js b/extensions/levelcode-ai/test/mcpConfig.test.js index e35a393..a2800dd 100644 --- a/extensions/levelcode-ai/test/mcpConfig.test.js +++ b/extensions/levelcode-ai/test/mcpConfig.test.js @@ -863,4 +863,55 @@ test('S5: config problems are carried through, and junk input never throws', () assert.strictEqual(M.summarizeMcp(null).configured, 0); }); +// ---- S6a: typed argument lines ---- +// The input box in "Manage MCP servers…" produces a string; `args` must be a list. + +test('ARGV: a quoted path with spaces stays ONE argument', () => { + // The whole reason this is not a whitespace split. The most common MCP server takes a directory, + // and on macOS that directory is very often "~/My Documents/…" — splitting it turns one argument + // into three and the server fails with a confusing message about the wrong path. + assert.deepStrictEqual( + M.parseArgv('-y @modelcontextprotocol/server-filesystem "/Users/me/My Documents/notes"'), + [ '-y', '@modelcontextprotocol/server-filesystem', '/Users/me/My Documents/notes' ] + ); + assert.deepStrictEqual(M.parseArgv("--dir 'a b' c"), [ '--dir', 'a b', 'c' ]); + assert.deepStrictEqual(M.parseArgv('a\\ b'), [ 'a b' ], 'a backslash escapes a space'); +}); + +test('ARGV: ordinary lines split on whitespace, and blank input is no arguments', () => { + assert.deepStrictEqual(M.parseArgv('-y pkg /a/b'), [ '-y', 'pkg', '/a/b' ]); + assert.deepStrictEqual(M.parseArgv(' spaced out '), [ 'spaced', 'out' ]); + for (const empty of [ '', ' ', '\t\n', null, undefined ]) { + assert.deepStrictEqual(M.parseArgv(empty), [], 'blank input yields no args: ' + JSON.stringify(empty)); + } +}); + +test('ARGV: a deliberate empty argument survives, and quotes can join', () => { + assert.deepStrictEqual(M.parseArgv('--name "" --x'), [ '--name', '', '--x' ], + 'an explicitly quoted empty string is an argument, not nothing'); + assert.deepStrictEqual(M.parseArgv('--flag="a b"'), [ '--flag=a b' ], 'quotes group mid-token'); + assert.deepStrictEqual(M.parseArgv("it's"), [ 'its' ], 'an unbalanced quote consumes to end, never throws'); +}); + +test('ARGV: it is a splitter, NOT a shell — nothing is expanded or interpreted', () => { + // args goes to spawn as a literal list. Anything clever here would be a lie about what runs. + assert.deepStrictEqual(M.parseArgv('$HOME/x'), [ '$HOME/x' ], 'no variable expansion'); + assert.deepStrictEqual(M.parseArgv('*.js'), [ '*.js' ], 'no globbing'); + assert.deepStrictEqual(M.parseArgv('a && b'), [ 'a', '&&', 'b' ], 'operators are just text'); + assert.deepStrictEqual(M.parseArgv('a | b'), [ 'a', '|', 'b' ]); +}); + +test('ARGV: a backslash is literal unless it escapes a space/quote/backslash (splitter, not a shell)', () => { + // The bug this guards: treating EVERY backslash as an escape silently rewrote arguments — a regex + // lost its anchors, a Windows path lost its separators. A splitter must not change what the user typed. + assert.deepStrictEqual(M.parseArgv('\\bword\\b'), [ '\\bword\\b' ], 'a regex keeps its backslashes'); + assert.deepStrictEqual(M.parseArgv('C:\\Users\\me'), [ 'C:\\Users\\me' ], 'a Windows path is preserved'); + assert.deepStrictEqual(M.parseArgv('"a\\b"'), [ 'a\\b' ], 'a backslash is literal inside double quotes too'); + // The three things a backslash DOES escape: + assert.deepStrictEqual(M.parseArgv('a\\ b'), [ 'a b' ], 'a space → one argument'); + assert.deepStrictEqual(M.parseArgv('\\"x'), [ '"x' ], 'a quote → a literal quote, no grouping'); + assert.deepStrictEqual(M.parseArgv('a\\\\b'), [ 'a\\b' ], 'a doubled backslash → one literal backslash'); + assert.deepStrictEqual(M.parseArgv('end\\'), [ 'end\\' ], 'a trailing backslash is kept literally'); +}); + console.log('\nmcpConfig.js: ' + n + ' tests passed.'); diff --git a/extensions/levelcode-ai/test/mcpManage.test.js b/extensions/levelcode-ai/test/mcpManage.test.js new file mode 100644 index 0000000..1cd4cea --- /dev/null +++ b/extensions/levelcode-ai/test/mcpManage.test.js @@ -0,0 +1,143 @@ +/*--------------------------------------------------------------------------------------------- + * The "Manage MCP servers…" rows — run: node test/mcpManage.test.js + * + * Two pure functions out of extension.js decide what a user is TOLD about a server they cannot + * start. Both are claims about a security gate, which is why they get tests despite being three + * lines each: + * + * mcpTrustIsStale — "you approved this, and the repo has changed it since". The G1 attack in + * docs/MCP.md is exactly this: get a benign command approved, then swap it. + * summarizeMcp reports both cases as trusted:false, so if this collapses them + * the UI silently downgrades an attack to "never set up". + * mcpServerItem — the row itself. "not started" on a server that is actually waiting for + * consent sends the user to debug a gate that is working correctly. + * + * Extracted from the shipped extension.js the way ctxSegments/narrativeUi do with chat.html: + * extension.js requires the `vscode` module at load, which does not exist outside the editor, and a + * copy of the function here would be a test of the copy. + *--------------------------------------------------------------------------------------------*/ +// @ts-check +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const src = fs.readFileSync(path.join(__dirname, '..', 'extension.js'), 'utf8'); + +// Slice a top-level `function (...) {…}` out of the source by matching its braces, not by +// looking for the first `\n}` (which truncates the moment a nested block's closing brace lands on its +// own line — i.e. on any reformat). Scan brace depth from the header's opening brace, skipping braces +// that live inside strings or comments, and stop at the brace that returns depth to zero. Dependency-free. +function extract(name) { + const start = src.indexOf('function ' + name + '('); + assert.ok(start >= 0, 'extension.js no longer defines ' + name + '()'); + const open = src.indexOf('{', start); + assert.ok(open >= 0, 'no opening brace found for ' + name + '()'); + let depth = 0, str = '', comment = ''; + for (let i = open; i < src.length; i++) { + const ch = src[i], next = src[i + 1]; + if (comment === 'line') { if (ch === '\n') { comment = ''; } continue; } + if (comment === 'block') { if (ch === '*' && next === '/') { comment = ''; i++; } continue; } + if (str) { + if (ch === '\\') { i++; } // escaped char inside a string — skip it + else if (ch === str) { str = ''; } + continue; + } + if (ch === '/' && next === '/') { comment = 'line'; i++; continue; } + if (ch === '/' && next === '*') { comment = 'block'; i++; continue; } + if (ch === '"' || ch === "'" || ch === '`') { str = ch; continue; } + if (ch === '{') { depth++; } + else if (ch === '}' && --depth === 0) { return src.slice(start, i + 1); } + } + assert.fail('no matching closing brace found for ' + name + '()'); +} + +// eslint-disable-next-line no-new-func +const load = new Function(extract('mcpTrustIsStale') + '\n' + extract('mcpServerItem') + + '\nreturn { mcpTrustIsStale, mcpServerItem };'); +const { mcpTrustIsStale, mcpServerItem } = load(); + +let n = 0; +function test(name, fn) { fn(); n++; console.log(' ok - ' + name); } + +/** A summarizeMcp-shaped row. */ +function row(over) { + return Object.assign({ + name: 'fs', source: 'workspace', origin: '.levelcode/mcp.json', running: false, + trusted: false, commandLine: 'npx -y server-filesystem /work', tools: 0, allowed: null, toolNames: [] + }, over); +} + +// ---- mcpTrustIsStale ---- + +test('STALE: an approval that exists but no longer matches is stale, not "never approved"', () => { + // The whole point. Same server name in the trust store => it WAS approved; trusted:false now => + // the fingerprint changed. Both facts are needed, and only their combination means "changed". + assert.strictEqual(mcpTrustIsStale(row(), { fs: 'sha256-of-the-old-command' }), true); + assert.strictEqual(mcpTrustIsStale(row(), {}), false, 'no entry at all is simply never approved'); + assert.strictEqual(mcpTrustIsStale(row(), { other: 'x' }), false, 'a different server being trusted says nothing'); +}); + +test('STALE: a server that IS currently trusted is never reported as changed', () => { + // trusted:true means the stored fingerprint matched — there is nothing stale about it. + assert.strictEqual(mcpTrustIsStale(row({ trusted: true }), { fs: 'sha256-current' }), false); +}); + +test('STALE: a settings server is never stale — it has no fingerprint to go stale', () => { + // summarizeMcp deliberately reports trusted:null for user-authored servers (they need no consent). + // A stale badge on one would be pure fiction. + assert.strictEqual(mcpTrustIsStale(row({ source: 'settings', trusted: null }), { fs: 'x' }), false); +}); + +test('STALE: junk trust stores do not throw and do not fake an approval', () => { + for (const store of [ null, undefined, {} ]) { + assert.strictEqual(mcpTrustIsStale(row(), store), false, 'store: ' + JSON.stringify(store)); + } + // hasOwnProperty, not `in` / `store[name]`: every object inherits `toString`, and a server may be + // legally named that. `'toString' in {}` is true, which would invent an approval nobody gave. + assert.strictEqual(mcpTrustIsStale(row({ name: 'toString' }), {}), false, + 'an inherited property must not be mistaken for a stored approval'); + assert.strictEqual(mcpTrustIsStale(row({ name: 'constructor' }), {}), false); +}); + +// ---- mcpServerItem ---- + +test('ROW: a repo server awaiting consent says so, instead of looking merely unstarted', () => { + const it = mcpServerItem(row(), {}); + assert.match(it.description, /needs approval/); + assert.doesNotMatch(it.description, /not started/); + assert.match(it.label, /^\$\(shield\) fs$/); + assert.strictEqual(it.detail, 'npx -y server-filesystem /work', 'the command line is what consent is about'); +}); + +test('ROW: a changed command line outranks the plain "needs approval" wording', () => { + const it = mcpServerItem(row(), { fs: 'sha256-of-the-old-command' }); + assert.match(it.description, /command changed/); + assert.match(it.label, /^\$\(warning\)/, 'and it is flagged, not shown as a routine unstarted server'); +}); + +test('ROW: a running server reports its tool counts; a stopped one invents none', () => { + const live = mcpServerItem(row({ running: true, trusted: true, tools: 12, allowed: 3 }), { fs: 'x' }); + assert.match(live.description, /running/); + assert.match(live.description, /12 tools/); + assert.match(live.description, /3 allow-listed/); + + const idle = mcpServerItem(row({ source: 'settings', trusted: null }), {}); + assert.match(idle.description, /not started/); + assert.doesNotMatch(idle.description, /tool/, 'a server that never started has no tool count to report'); +}); + +test('ROW: one tool is not "1 tools"', () => { + assert.match(mcpServerItem(row({ running: true, trusted: true, tools: 1, allowed: 0 }), {}).description, /1 tool · /); +}); + +test('ROW: provenance is always visible — settings vs which repo file', () => { + // The difference that decides whether the entry is user-authored or attacker-authored. It belongs + // on the row, not one level down. + assert.match(mcpServerItem(row({ source: 'settings', trusted: null }), {}).description, /your settings/); + assert.match(mcpServerItem(row(), {}).description, /\.levelcode\/mcp\.json/); + assert.match(mcpServerItem(row({ origin: '' }), {}).description, /workspace/, 'a missing origin still says untrusted'); +}); + +console.log('\nmcpManage (extension.js): ' + n + ' tests passed.');