From d806974261a91907dbbb64d562a6d604c70e5275 Mon Sep 17 00:00:00 2001 From: Gordon Farquharson Date: Tue, 1 Sep 2026 14:01:25 +0100 Subject: [PATCH 01/10] Harden extension against local network and webview attacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security hardening across six advisories: Webview (SA-001, SA-002): - Add Content-Security-Policy with per-render nonce to debugger webview - Fix postMessage bridge: gate iframe commands on event.source + event.origin check - Replace postMessage(data, '*') with postMessage(data, FRAME_ORIGIN) on host responses - Restrict openExternal to http/https schemes only; refuse other URI handlers Debugger server auth (SA-003): - Generate per-session token on DebuggerServerManager construction - Inject FASTEDGE_DEBUG_TOKEN and FASTEDGE_BIND_HOST=127.0.0.1 into forked server - Add x-fastedge-token header to all extension-side /api/* fetch calls - Deliver token to iframe via URL fragment (#token=...) for frontend auth Autorun trigger file (SA-004): - Skip execution in untrusted workspaces - Require explicit user confirmation before running any trigger command - Remove commandArgs forwarding from trigger file Port file trust (SA-006): - Gate .debug-port reuse on vscode.workspace.isTrusted; always spawn fresh in untrusted workspaces MCP server config (SA-005): - Mask API key input (password: true) - Pin Docker image via mcp-server.version file injected at build time by esbuild - chmod 0600 on written mcp.json for local file URIs Cleanup: - Delete src/dotenv/ — superseded by server-side dotenv handling in fastedge-test - Add MCP_INTEGRATION.md context doc covering version pinning mechanism - Add vitest.config.ts to mirror esbuild define for __MCP_SERVER_VERSION__ - Add tests: webview HTML security properties, trigger file guards, mcpJson image pin --- context/CONTEXT_INDEX.md | 6 +- context/features/MCP_INTEGRATION.md | 105 +++++++++++++++++++ esbuild/build-ext.js | 11 ++ mcp-server.version | 1 + src/autorun/triggerFileHandler.test.ts | 69 ++++++++++++ src/autorun/triggerFileHandler.ts | 31 +++++- src/commands/mcpJson.test.ts | 7 ++ src/commands/mcpJson.ts | 10 +- src/debugger/DebuggerServerManager.ts | 37 +++++-- src/debugger/DebuggerWebviewProvider.test.ts | 40 +++++++ src/debugger/DebuggerWebviewProvider.ts | 83 +++++++++------ src/dotenv/index.ts | 65 ------------ src/globals.d.ts | 2 + vitest.config.ts | 15 +++ 14 files changed, 366 insertions(+), 116 deletions(-) create mode 100644 context/features/MCP_INTEGRATION.md create mode 100644 mcp-server.version create mode 100644 src/autorun/triggerFileHandler.test.ts create mode 100644 src/debugger/DebuggerWebviewProvider.test.ts delete mode 100644 src/dotenv/index.ts create mode 100644 src/globals.d.ts create mode 100644 vitest.config.ts diff --git a/context/CONTEXT_INDEX.md b/context/CONTEXT_INDEX.md index bbcc160..7eef6fd 100644 --- a/context/CONTEXT_INDEX.md +++ b/context/CONTEXT_INDEX.md @@ -73,6 +73,10 @@ Use this tree to find relevant documentation for your task: → Read: `features/MCP_INTEGRATION.md` → Read: `features/COMMANDS.md` (mcpJson command) +**Task: Bump the pinned MCP server Docker image version** +→ Edit: `mcp-server.version` (one line — the only file to change) +→ Read: `features/MCP_INTEGRATION.md` (explains the build-time injection) + **Task: Add new configuration option** → Read: `architecture/CONFIGURATION_SYSTEM.md` → Read: `BUNDLED_DEBUGGER.md` (fastedge-config.test.json section) @@ -134,7 +138,7 @@ Use this tree to find relevant documentation for your task: | **DOTENV_SYSTEM.md** | Dotenv file handling | Dotenv loading issues | | **CROSS_PLATFORM.md** | Linux/macOS/Windows support, CI matrix, spawn rules | Any platform-specific work or new process spawning | | **LAUNCH_CONFIG.md** | Launch.json generation | Launch config changes | -| **MCP_INTEGRATION.md** | MCP server configuration | MCP feature work | +| **MCP_INTEGRATION.md** | MCP server config, image version pinning, how to bump | MCP feature work or bumping the server version | | **AUTORUN_SYSTEM.md** | File watching, auto-trigger | Auto-run functionality | | **CODESPACE_SECRETS.md** | GitHub Codespaces integration | Codespaces features | diff --git a/context/features/MCP_INTEGRATION.md b/context/features/MCP_INTEGRATION.md new file mode 100644 index 0000000..5b8ff01 --- /dev/null +++ b/context/features/MCP_INTEGRATION.md @@ -0,0 +1,105 @@ +# MCP Integration + +The extension can generate a `.vscode/mcp.json` that wires up the +`fastedge-assistant` MCP server so AI clients (Claude, Codex, Cursor) can +use FastEdge tools from inside the workspace. + +--- + +## How it works + +Command: **FastEdge (Generate mcp.json)** → `src/commands/mcpJson.ts` + +1. Reads any existing `.vscode/mcp.json` and merges the new server entry in. +2. Detects Codespaces (`CODESPACE_NAME` env): offers `gh secret set` path + (stores key as a Codespace secret, emits `${env:GCORE_API_KEY}` in the + file) or falls back to inline key with a security notice. +3. Prompts for the API key (**masked input**, `password: true`). +4. Writes the file; sets `chmod 0600` on local `file://` URIs (no-op on + Windows / remote providers). +5. Offers to add `.vscode/mcp.json` to `.gitignore`. + +The generated entry looks like: + +```json +{ + "servers": { + "fastedge-assistant": { + "type": "stdio", + "command": "docker", + "args": ["run", "--rm", "-i", "--pull=always", + "-v", "${workspaceFolder}:/workspace", + "-e", "WORKSPACE_ROOT=/workspace", + "-e", "GCORE_API_KEY", + "ghcr.io/g-core/fastedge-mcp-server:v0.2.9"], + "env": { "GCORE_API_KEY": "" } + } + } +} +``` + +--- + +## Pinned image version — how to bump it + +The Docker image tag (`v0.2.9` above) is **not hardcoded in source**. It is +read from a single file at build time and injected by esbuild: + +``` +mcp-server.version ← edit this file to bump the version +esbuild/build-ext.js ← reads the file, passes to esbuild define +src/globals.d.ts ← TypeScript ambient declaration +src/commands/mcpJson.ts ← uses __MCP_SERVER_VERSION__ (injected constant) +``` + +**To bump the version:** + +```bash +echo "v0.3.0" > mcp-server.version +# rebuild — the new tag is baked into dist/extension.js +pnpm run build +``` + +A future CI job in the MCP server's release pipeline can automate this step +by committing the updated `mcp-server.version` file and triggering a new +extension release. + +**Do not edit the version string in `mcpJson.ts` directly** — it uses the +injected constant and will not reflect manual edits after a rebuild. + +--- + +## Key files + +| File | Role | +|------|------| +| `mcp-server.version` | Single source of truth for the pinned image tag | +| `esbuild/build-ext.js` | Reads version file, injects `__MCP_SERVER_VERSION__` via esbuild `define` | +| `src/globals.d.ts` | Ambient TS declaration for `__MCP_SERVER_VERSION__` | +| `src/commands/mcpJson.ts` | `createMCPJson` command + `getDockerCommand` builder | +| `src/commands/mcpJson.test.ts` | Unit tests for `getDockerCommand` argv shape | + +--- + +## `getDockerCommand` — security invariants + +The docker command is built as an **argv array** (no shell wrapper), so the +workspace path in `-v ${workspaceFolder}:/workspace` cannot inject shell +syntax. Tests in `mcpJson.test.ts` assert this. Do not add `bash -c` or +`cmd /c` wrappers. + +Credentials are forwarded with bare `-e GCORE_API_KEY` (value comes from the +MCP client's `env` block, never from shell expansion). + +--- + +## Codespace path + +When `CODESPACE_NAME` is set, the command offers to call `setupCodespaceSecret` +first. That function stores the key via `gh secret set` (spawned with `spawn`, +secret on stdin — not in argv). If the user takes this path, the generated +file uses `${env:GCORE_API_KEY}` instead of an inline key. + +--- + +**Last Updated**: 2026-09-01 diff --git a/esbuild/build-ext.js b/esbuild/build-ext.js index dbd64e4..0e7826b 100644 --- a/esbuild/build-ext.js +++ b/esbuild/build-ext.js @@ -1,8 +1,16 @@ const esbuild = require("esbuild"); +const fs = require("fs"); +const path = require("path"); const isProduction = process.argv.includes("--prod"); const isWatching = process.argv.includes("--watch"); +// Read the pinned MCP server image version from the version file. +// A future CI job can update this file on MCP server release. +const mcpServerVersion = fs + .readFileSync(path.join(__dirname, "../mcp-server.version"), "utf8") + .trim(); + async function main() { const ctx = await esbuild.context({ entryPoints: ["./src/extension.ts"], @@ -16,6 +24,9 @@ async function main() { external: ["vscode"], mainFields: ["module", "main"], logLevel: "info", + define: { + __MCP_SERVER_VERSION__: JSON.stringify(mcpServerVersion), + }, plugins: [ /* add to the end of plugins array */ esbuildProblemMatcherPlugin, diff --git a/mcp-server.version b/mcp-server.version new file mode 100644 index 0000000..8f51ea9 --- /dev/null +++ b/mcp-server.version @@ -0,0 +1 @@ +v0.2.9 diff --git a/src/autorun/triggerFileHandler.test.ts b/src/autorun/triggerFileHandler.test.ts new file mode 100644 index 0000000..f475639 --- /dev/null +++ b/src/autorun/triggerFileHandler.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Mocks must be declared with vi.hoisted() so they're available inside the +// vi.mock factory, which Vitest hoists above all import statements. +const mocks = vi.hoisted(() => ({ + state: { isTrusted: true }, + executeCommand: vi.fn().mockResolvedValue(undefined), + showWarningMessage: vi.fn(), + delete: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn(), +})); + +vi.mock("vscode", () => ({ + workspace: { + get isTrusted() { return mocks.state.isTrusted; }, + fs: { readFile: mocks.readFile, delete: mocks.delete }, + }, + window: { + showWarningMessage: mocks.showWarningMessage, + showErrorMessage: vi.fn(), + showInformationMessage: vi.fn(), + }, + commands: { executeCommand: mocks.executeCommand }, +})); + +import { executeTriggerFile } from "./triggerFileHandler"; + +const fakeUri = { fsPath: "/ws/.vscode/.fastedge-run-command" } as any; +const fakeOutput = { appendLine: vi.fn() } as any; +const allowedCommand = "fastedge.setup-codespace-secret"; + +beforeEach(() => { + mocks.state.isTrusted = true; + vi.clearAllMocks(); + mocks.readFile.mockResolvedValue(Buffer.from(allowedCommand)); + mocks.delete.mockResolvedValue(undefined); + mocks.executeCommand.mockResolvedValue(undefined); +}); + +describe("executeTriggerFile — security guards", () => { + it("skips executeCommand in an untrusted workspace", async () => { + mocks.state.isTrusted = false; + await executeTriggerFile(fakeUri, fakeOutput); + expect(mocks.executeCommand).not.toHaveBeenCalled(); + expect(mocks.delete).toHaveBeenCalled(); // cleans up the trigger file + }); + + it("skips executeCommand when the user dismisses the confirmation", async () => { + mocks.showWarningMessage.mockResolvedValue("Ignore"); + await executeTriggerFile(fakeUri, fakeOutput); + expect(mocks.executeCommand).not.toHaveBeenCalled(); + expect(mocks.delete).toHaveBeenCalled(); + }); + + it("runs the command when trusted and user confirms, without workspace-supplied args", async () => { + // File carries args — they must NOT be forwarded to executeCommand. + mocks.readFile.mockResolvedValue( + Buffer.from(JSON.stringify({ command: allowedCommand, args: ["injected-arg"] })), + ); + mocks.showWarningMessage.mockResolvedValue("Run"); + + await executeTriggerFile(fakeUri, fakeOutput); + + expect(mocks.executeCommand).toHaveBeenCalledTimes(1); + expect(mocks.executeCommand).toHaveBeenCalledWith(allowedCommand); + // Confirm args were not spread in — call must have exactly one argument. + expect(mocks.executeCommand.mock.calls[0]).toHaveLength(1); + }); +}); diff --git a/src/autorun/triggerFileHandler.ts b/src/autorun/triggerFileHandler.ts index 9aeaf6a..15dcc34 100644 --- a/src/autorun/triggerFileHandler.ts +++ b/src/autorun/triggerFileHandler.ts @@ -82,7 +82,7 @@ export function initializeTriggerFileHandler( /** * Execute command from trigger file */ -async function executeTriggerFile( +export async function executeTriggerFile( uri: vscode.Uri, outputChannel: vscode.OutputChannel, ): Promise { @@ -125,7 +125,30 @@ async function executeTriggerFile( return; } - // Execute command with timeout protection + // Never auto-execute in untrusted workspaces — a workspace-controlled file + // writing the trigger file would get a silent privileged command run. + if (!vscode.workspace.isTrusted) { + outputChannel.appendLine(`Skipping execution in untrusted workspace: ${commandId}`); + await vscode.workspace.fs.delete(uri); + return; + } + + // Require explicit user confirmation — the trigger file is workspace- + // authored, so execution without a click is an unintended privilege. + const ok = await vscode.window.showWarningMessage( + `This workspace is asking to run "${commandId}". Only allow this if you trust the workspace.`, + { modal: true }, + "Run", + "Ignore", + ); + if (ok !== "Run") { + await vscode.workspace.fs.delete(uri); + return; + } + + // Execute command with timeout protection. + // commandArgs from the file are never forwarded — the allowlisted command + // takes no arguments, and workspace-controlled args would be a privilege path. outputChannel.appendLine(`Executing command: ${commandId}`); let timeoutHandle: NodeJS.Timeout | undefined; const timeoutPromise = new Promise((_, reject) => { @@ -135,9 +158,7 @@ async function executeTriggerFile( ); }); - const executePromise = commandArgs - ? vscode.commands.executeCommand(commandId, ...commandArgs) - : vscode.commands.executeCommand(commandId); + const executePromise = vscode.commands.executeCommand(commandId); try { await Promise.race([executePromise, timeoutPromise]); diff --git a/src/commands/mcpJson.test.ts b/src/commands/mcpJson.test.ts index 07d4107..4fc51e8 100644 --- a/src/commands/mcpJson.test.ts +++ b/src/commands/mcpJson.test.ts @@ -56,6 +56,13 @@ describe("getDockerCommand", () => { expect(getDockerCommand(false).args).not.toContain("GCORE_API_BASE"); }); + it("uses a pinned version tag, not :latest", () => { + const { args } = getDockerCommand(false); + const imageArg = args[args.length - 1]; + expect(imageArg).not.toContain(":latest"); + expect(imageArg).toContain(__MCP_SERVER_VERSION__); + }); + it("is platform independent", () => { const original = Object.getOwnPropertyDescriptor(process, "platform")!; try { diff --git a/src/commands/mcpJson.ts b/src/commands/mcpJson.ts index 144fd18..f807880 100644 --- a/src/commands/mcpJson.ts +++ b/src/commands/mcpJson.ts @@ -1,4 +1,5 @@ import * as vscode from "vscode"; +import * as fs from "fs"; import { MCPConfiguration } from "../types"; import { isCodespace, setupCodespaceSecret } from "./codespaceSecrets"; @@ -35,7 +36,9 @@ function getDockerCommand(includeBaseOverride: boolean): { if (includeBaseOverride) { args.push("-e", "GCORE_API_BASE"); } - args.push("ghcr.io/g-core/fastedge-mcp-server:latest"); + // Version is read from mcp-server.version at build time and injected by esbuild. + // Update that file (not this line) when the MCP server releases a new version. + args.push(`ghcr.io/g-core/fastedge-mcp-server:${__MCP_SERVER_VERSION__}`); return { command: "docker", args }; } @@ -246,6 +249,7 @@ async function createMCPJson(context?: vscode.ExtensionContext) { prompt: "Enter your FastEdge API Key", placeHolder: defaultApiKey || "Your API key here...", value: defaultApiKey, // Pre-fill with saved value + password: true, // Mask input — mirrors setupCodespaceSecret behavior validateInput: (value) => { if (!value || value.trim().length === 0) { return "API Key is required"; @@ -304,6 +308,10 @@ async function createMCPJson(context?: vscode.ExtensionContext) { mcpJsonPath, Buffer.from(JSON.stringify(mcpJsonContent, null, 2)), ); + // Restrict read access on local files — no-op on Windows (best effort). + if (mcpJsonPath.scheme === "file") { + try { fs.chmodSync(mcpJsonPath.fsPath, 0o600); } catch { /* best effort */ } + } } catch (error: any) { vscode.window.showErrorMessage( `Failed to write mcp.json: ${error?.message || error}`, diff --git a/src/debugger/DebuggerServerManager.ts b/src/debugger/DebuggerServerManager.ts index ca5c54b..cdd90fd 100644 --- a/src/debugger/DebuggerServerManager.ts +++ b/src/debugger/DebuggerServerManager.ts @@ -1,4 +1,5 @@ import { fork, execFile, ChildProcess } from "child_process"; +import { randomBytes } from "crypto"; import * as vscode from "vscode"; import * as path from "path"; import * as fs from "fs"; @@ -16,12 +17,19 @@ export class DebuggerServerManager { private serverProcess: ChildProcess | null = null; private port: number = 5179; private isStarting: boolean = false; + // Per-instance token: generated once, injected into the server via env and + // passed to the webview iframe via URL fragment so the frontend can auth. + private readonly token: string = randomBytes(16).toString("hex"); constructor( private extensionPath: string, private appRoot: string ) {} + getToken(): string { + return this.token; + } + private get portFilePath(): string { return path.join(this.appRoot, DEBUG_DIR, ".debug-port"); } @@ -69,17 +77,21 @@ export class DebuggerServerManager { * Port selection is delegated to fastedge-test's auto-increment logic. */ async start(): Promise { - // Step 1: Check if a server is already running for this app via port file - const filePort = this.readPortFile(); - if (filePort !== null) { - if (await this.isHealthyOnPort(filePort)) { - this.port = filePort; - console.log(`Reusing existing debugger server on port ${this.port} for ${this.appRoot}`); - return; - } else { - // Stale port file — clean it up - console.log(`Stale port file found for ${this.appRoot}, removing...`); - this.deletePortFile(); + // Step 1: Check if a server is already running for this app via port file. + // In an untrusted workspace the port file is workspace-controlled, so ignore + // it and always spawn a fresh server that this session owns. + if (vscode.workspace.isTrusted) { + const filePort = this.readPortFile(); + if (filePort !== null) { + if (await this.isHealthyOnPort(filePort)) { + this.port = filePort; + console.log(`Reusing existing debugger server on port ${this.port} for ${this.appRoot}`); + return; + } else { + // Stale port file — clean it up + console.log(`Stale port file found for ${this.appRoot}, removing...`); + this.deletePortFile(); + } } } @@ -124,6 +136,8 @@ export class DebuggerServerManager { ...process.env, VSCODE_INTEGRATION: "true", WORKSPACE_PATH: this.appRoot, + FASTEDGE_DEBUG_TOKEN: this.token, + FASTEDGE_BIND_HOST: "127.0.0.1", }, }); @@ -253,6 +267,7 @@ export class DebuggerServerManager { method: "POST", headers: { "Content-Type": "application/json", + "x-fastedge-token": this.token, }, }); diff --git a/src/debugger/DebuggerWebviewProvider.test.ts b/src/debugger/DebuggerWebviewProvider.test.ts new file mode 100644 index 0000000..232f0e7 --- /dev/null +++ b/src/debugger/DebuggerWebviewProvider.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, vi } from "vitest"; + +vi.mock("vscode", () => ({ workspace: {}, window: {}, Uri: {}, env: {} })); + +import { DebuggerWebviewProvider } from "./DebuggerWebviewProvider"; + +const fakeServer = { + getPort: () => 5179, + getToken: () => "deadbeeftoken", + getAppRoot: () => "/app", + getUrl: () => "http://localhost:5179", +} as any; + +// getWebviewContent is private — access via cast for testing HTML output. +const html = (new DebuggerWebviewProvider({} as any, fakeServer) as any) + .getWebviewContent("http://localhost:5179"); + +describe("DebuggerWebviewProvider.getWebviewContent — security properties", () => { + it("includes a Content-Security-Policy with a per-render nonce", () => { + expect(html).toMatch(/Content-Security-Policy/); + expect(html).toMatch(/script-src 'nonce-[A-Za-z0-9+/=]+'/); + }); + + it("restricts frame-src to the debugger origin, not a wildcard", () => { + expect(html).toMatch(/frame-src http:\/\/localhost:5179/); + }); + + it("never posts messages with target origin '*'", () => { + expect(html).not.toContain("postMessage(event.data, '*')"); + expect(html).not.toContain('postMessage(event.data,"*")'); + }); + + it("sets FRAME_ORIGIN to the debugger origin as a string literal", () => { + expect(html).toContain('const FRAME_ORIGIN = "http://localhost:5179"'); + }); + + it("injects the session token into the iframe src fragment", () => { + expect(html).toContain("#token=deadbeeftoken"); + }); +}); diff --git a/src/debugger/DebuggerWebviewProvider.ts b/src/debugger/DebuggerWebviewProvider.ts index eb192bf..9c2c1da 100644 --- a/src/debugger/DebuggerWebviewProvider.ts +++ b/src/debugger/DebuggerWebviewProvider.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; import * as path from "path"; +import { randomBytes } from "crypto"; import { readFile } from "fs/promises"; import { DebuggerServerManager } from "./DebuggerServerManager"; @@ -58,7 +59,20 @@ export class DebuggerWebviewProvider { // Handle messages from the webview (forwarded from the debugger iframe) this.panel.webview.onDidReceiveMessage(async (message) => { if (message.command === "openExternal") { - await vscode.env.openExternal(vscode.Uri.parse(message.url)); + let uri: vscode.Uri; + try { + uri = vscode.Uri.parse(message.url, true); + } catch { + return; // unparseable → refuse + } + // Only allow http/https — no vscode:, file:, or other OS handlers. + if (uri.scheme !== "https" && uri.scheme !== "http") { + vscode.window.showWarningMessage( + `FastEdge: refused to open a non-web link (${uri.scheme}:).`, + ); + return; + } + await vscode.env.openExternal(uri); } if (message.command === "openFilePicker") { @@ -159,7 +173,7 @@ export class DebuggerWebviewProvider { method: "POST", headers: { "Content-Type": "application/json", - "X-Source": "vscode", + "x-fastedge-token": this.serverManager.getToken(), }, body: JSON.stringify({ wasmPath, @@ -202,7 +216,7 @@ export class DebuggerWebviewProvider { method: "POST", headers: { "Content-Type": "application/json", - "X-Source": "vscode", + "x-fastedge-token": this.serverManager.getToken(), }, body: JSON.stringify({ config }), } @@ -231,6 +245,7 @@ export class DebuggerWebviewProvider { while (Date.now() - start < timeoutMs) { try { const response = await fetch(`${this.serverManager.getUrl()}/api/client-count`, { + headers: { "x-fastedge-token": this.serverManager.getToken() }, signal: AbortSignal.timeout(2000), }); const { count } = await response.json(); @@ -247,12 +262,19 @@ export class DebuggerWebviewProvider { * Get the webview HTML content */ private getWebviewContent(debuggerUrl: string): string { + const nonce = randomBytes(16).toString("base64"); + const frameOrigin = new URL(debuggerUrl).origin; + // Deliver the session token to the iframe via URL fragment — fragments are + // never sent in HTTP requests, so they don't appear in server logs, and only + // the same-origin iframe page can read location.hash. + const iframeUrl = `${debuggerUrl}#token=${encodeURIComponent(this.serverManager.getToken())}`; return ` + FastEdge Debugger