diff --git a/README.md b/README.md index c09e4b0..7eccf4e 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,9 @@ OPENROUTER_API_KEY=... npm run cli -- --workspace /path/to/project --chat # Interactive chat with a specific model OPENROUTER_API_KEY=... npm run cli -- --workspace /path/to/project --chat --modelId openrouter/anthropic/claude-3.7-sonnet + +# Interactive chat with MCP servers from a JSON config file +OPENROUTER_API_KEY=... npm run cli -- --workspace /path/to/project --chat --mcp-config ./mcp-servers.json ``` This starts an interactive streaming chat session in your terminal with access to the workspace and tools. @@ -152,6 +155,27 @@ Keep `NODE_OPTIONS="--experimental-ffi"` scoped to the Node.js OpenTUI launch command rather than exporting it globally. It is not needed for Bun, plain chat, the HTTP service or ACP, and older Node.js versions may reject the flag. +The MCP config file can contain a server array or an object with `mcpServers` +as either an array or a map keyed by server name. Keyed entries inherit their +key as the server `id` and `name` unless those fields are explicitly set. +For example, create `mcp-servers.json`: + +```json +{ + "mcpServers": { + "filesystem": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/path/to/project"] + } + } +} +``` + +Use `--mcp-config ` with `--chat`; relative paths are resolved from the +current directory. Local server processes inherit the CLI environment, so set +required credentials in the shell before launching. Keep config files containing +secrets out of source control. + #### Classic terminal chat (readline) To use the old terminal chat instead of OpenTUI, explicitly select `plain`: diff --git a/api/core/agent/tools/mcpTools.ts b/api/core/agent/tools/mcpTools.ts index 92c5297..15f7cc6 100644 --- a/api/core/agent/tools/mcpTools.ts +++ b/api/core/agent/tools/mcpTools.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import Ajv from "ajv"; import type { JSONSchema7 } from "json-schema"; import os from "os"; +import { delimiter as pathDelimiter } from "node:path"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport, @@ -134,11 +135,12 @@ export const summarizeMcpToolError = (result: { return summarizeStructuredError(result.structuredContent) || summarizeMcpContent(result.content); }; -const buildAugmentedPath = (): string => { +export const buildAugmentedPath = ( + basePath = process.env.PATH || "", +): string => { const home = process.env.HOME || os.homedir() || ""; - const currentPath = process.env.PATH || ""; const pathParts = [ - currentPath, + basePath, "/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", @@ -147,7 +149,9 @@ const buildAugmentedPath = (): string => { home ? `${home}/.cargo/bin` : "", ].filter(Boolean); - return Array.from(new Set(pathParts.join(":").split(":"))).join(":"); + return Array.from( + new Set(pathParts.join(pathDelimiter).split(pathDelimiter)), + ).join(pathDelimiter); }; type CommandCandidate = { @@ -208,7 +212,7 @@ const createTransportParams = ( ...process.env, ...server.env, } as Record; - env.PATH = buildAugmentedPath(); + env.PATH = buildAugmentedPath(env.PATH); return { command: server.command!.trim(), diff --git a/api/core/library/mcpSettings.ts b/api/core/library/mcpSettings.ts index 86d4924..ed90175 100644 --- a/api/core/library/mcpSettings.ts +++ b/api/core/library/mcpSettings.ts @@ -27,6 +27,34 @@ export type McpServerConfig = { }; const isSafeEnvKey = (key: string): boolean => /^[A-Za-z_][A-Za-z0-9_]*$/.test(key); +const isValidHttpHeaderName = (name: string): boolean => + /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/.test(name); +const isValidHttpHeaderValue = (value: unknown): value is string => + typeof value === "string" && + value.length <= MAX_HEADER_LENGTH && + /^[\t\x20-\x7E\x80-\xFF]*$/.test(value); + +const isValidHttpHeaders = ( + headers: unknown, +): headers is Record => { + if (!headers || typeof headers !== "object" || Array.isArray(headers)) { + return false; + } + + const entries = Object.entries(headers); + return ( + entries.length <= MAX_HEADERS && + entries.every( + ([name, value]) => + name.length <= MAX_HEADER_LENGTH && + isValidHttpHeaderName(name) && + isValidHttpHeaderValue(value), + ) + ); +}; + +const hasValidHttpHeaders = (headers: unknown): boolean => + headers === undefined || isValidHttpHeaders(headers); const splitCommandString = (value: string): string[] => { const parts: string[] = []; @@ -79,16 +107,10 @@ const sanitizeEnv = (env: unknown): Record => { }; const sanitizeHeaders = (headers: unknown): Record => { - if (!headers || typeof headers !== "object" || Array.isArray(headers)) return {}; + if (!isValidHttpHeaders(headers)) return {}; return Object.fromEntries( - Object.entries(headers) - .map(([key, value]) => [ - safeString(key, MAX_HEADER_LENGTH), - safeString(value, MAX_HEADER_LENGTH), - ] as const) - .filter(([key, value]) => key.length > 0 && value.length > 0) - .slice(0, MAX_HEADERS), + Object.entries(headers).slice(0, MAX_HEADERS), ); }; @@ -157,6 +179,7 @@ const normalizeMcpServerDraft = ( if (!input || typeof input !== "object" || Array.isArray(input)) return null; const raw = input as Record; + if (!hasValidHttpHeaders(raw.headers)) return null; const id = safeString(raw.id, 120) || getDefaultId(index); const hasUrlInput = typeof raw.url === "string" && raw.url.trim().length > 0; const url = sanitizeUrl(raw.url); @@ -213,6 +236,7 @@ export const sanitizeMcpServer = (input: unknown, index = 0): McpServerConfig | if (!input || typeof input !== "object" || Array.isArray(input)) return null; const raw = input as Record; + if (!hasValidHttpHeaders(raw.headers)) return null; const hasUrlInput = typeof raw.url === "string" && raw.url.trim().length > 0; const url = sanitizeUrl(raw.url); // A non-empty url that fails sanitization is malformed/unsupported, not @@ -264,6 +288,216 @@ export const sanitizeMcpServers = ( .slice(0, maxServers); }; +export const parseMcpServerConfigFile = (contents: string): McpServerConfig[] => { + let parsed: unknown; + try { + parsed = JSON.parse(contents); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`MCP config is not valid JSON: ${message}`); + } + + let servers: unknown[]; + if (Array.isArray(parsed)) { + servers = parsed; + } else if (parsed && typeof parsed === "object") { + const configuredServers = (parsed as Record).mcpServers; + if (Array.isArray(configuredServers)) { + servers = configuredServers; + } else if (configuredServers && typeof configuredServers === "object") { + servers = Object.entries(configuredServers).map(([key, value]) => { + if (!key.trim() || key !== key.trim() || key.length > MAX_NAME_LENGTH) { + throw new Error(`MCP config has an invalid server key: "${key}"`); + } + if (!value || typeof value !== "object" || Array.isArray(value)) { + return value; + } + const server = value as Record; + return { + ...server, + id: server.id ?? key, + name: server.name ?? key, + }; + }); + } else { + throw new Error( + 'MCP config must be an array or an object with a "mcpServers" array or object', + ); + } + } else { + throw new Error( + 'MCP config must be an array or an object with a "mcpServers" array or object', + ); + } + if (servers.length > MAX_MCP_SERVERS) { + throw new Error(`MCP config supports at most ${MAX_MCP_SERVERS} servers`); + } + + const validatedServers: Array<{ + config: unknown; + args?: string[]; + env?: Record; + headers?: Record; + }> = servers.map((server) => { + if (server && typeof server === "object" && !Array.isArray(server)) { + const config = server as Record; + const validatedConfig = { ...config }; + if ("enabled" in config && typeof config.enabled !== "boolean") { + throw new Error('MCP config server "enabled" value must be a boolean'); + } + if ("url" in config) { + if ( + typeof config.url !== "string" || + !config.url.trim() + ) { + throw new Error( + 'MCP config server "url" must be a non-empty string', + ); + } + if (config.url.length > MAX_URL_LENGTH) { + throw new Error( + `MCP config server "url" exceeds ${MAX_URL_LENGTH} characters`, + ); + } + } + if ("command" in config) { + if (typeof config.command !== "string" || !config.command.trim()) { + throw new Error( + 'MCP config server "command" must be a non-empty string', + ); + } + if (config.command !== config.command.trim()) { + throw new Error( + 'MCP config server "command" cannot have surrounding whitespace', + ); + } + if (config.command.length > MAX_COMMAND_LENGTH) { + throw new Error( + `MCP config server "command" exceeds ${MAX_COMMAND_LENGTH} characters`, + ); + } + } + + let args: string[] | undefined; + if ("args" in config) { + if (!Array.isArray(config.args)) { + throw new Error( + 'MCP config server "args" must be an array of strings', + ); + } + if (config.args.length > MAX_ARGS) { + throw new Error( + `MCP config server "args" supports at most ${MAX_ARGS} values`, + ); + } + args = config.args.map((argument) => { + if (typeof argument !== "string") { + throw new Error( + 'MCP config server "args" must contain only strings', + ); + } + if (argument.length > MAX_ARG_LENGTH) { + throw new Error( + `MCP config server arguments cannot exceed ${MAX_ARG_LENGTH} characters`, + ); + } + return argument; + }); + validatedConfig.args = args; + } + + let env: Record | undefined; + if ("env" in config) { + if ( + !config.env || + typeof config.env !== "object" || + Array.isArray(config.env) + ) { + throw new Error('MCP config server "env" must be an object of strings'); + } + const entries = Object.entries(config.env); + if (entries.length > MAX_ENV_VARS) { + throw new Error( + `MCP config server "env" supports at most ${MAX_ENV_VARS} values`, + ); + } + env = Object.fromEntries( + entries.map(([key, value]) => { + if ( + key.length > MAX_ENV_KEY_LENGTH || + !isSafeEnvKey(key) || + typeof value !== "string" || + value.length > MAX_ENV_VALUE_LENGTH + ) { + throw new Error( + 'MCP config server "env" contains an invalid key or value', + ); + } + return [key, value]; + }), + ); + validatedConfig.env = env; + } + + let headers: Record | undefined; + if ("headers" in config) { + if ( + !config.headers || + typeof config.headers !== "object" || + Array.isArray(config.headers) + ) { + throw new Error( + 'MCP config server "headers" must be an object of strings', + ); + } + const entries = Object.entries(config.headers); + if (entries.length > MAX_HEADERS) { + throw new Error( + `MCP config server "headers" supports at most ${MAX_HEADERS} values`, + ); + } + headers = Object.fromEntries( + entries.map(([key, value]) => { + if ( + key.length > MAX_HEADER_LENGTH || + !isValidHttpHeaderName(key) || + !isValidHttpHeaderValue(value) + ) { + throw new Error( + 'MCP config server "headers" contains an invalid key or value', + ); + } + return [key, value]; + }), + ); + validatedConfig.headers = headers; + } + + return { config: validatedConfig, args, env, headers }; + } + return { config: server }; + }); + + const sanitizedServers = sanitizeMcpServers( + validatedServers.map(({ config }) => config), + ); + if (sanitizedServers.length !== servers.length) { + throw new Error("MCP config contains an invalid server entry"); + } + return sanitizedServers.map((server, index) => ({ + ...server, + ...(validatedServers[index].args !== undefined + ? { args: validatedServers[index].args } + : {}), + ...(validatedServers[index].env !== undefined + ? { env: validatedServers[index].env } + : {}), + ...(validatedServers[index].headers !== undefined + ? { headers: validatedServers[index].headers } + : {}), + })); +}; + export const toStableMcpFingerprint = (servers: McpServerConfig[]): string => { const normalized = servers .map((server) => ({ diff --git a/cli.ts b/cli.ts index d71d6b0..4ac79fe 100644 --- a/cli.ts +++ b/cli.ts @@ -9,9 +9,12 @@ import yargs from "yargs"; import { hideBin } from "yargs/helpers"; +import { readFile } from "node:fs/promises"; +import { resolve } from "node:path"; import { createCodingAgent } from "./api/core/agent/index.js"; import { startAcpServer } from "./api/acp/acpServer.js"; import { getMissingProviderSetup } from "./api/core/library/providerSetup.js"; +import { parseMcpServerConfigFile } from "./api/core/library/mcpSettings.js"; import { createCliChatUi, type CliChatUiMode, @@ -58,6 +61,10 @@ const argv = yargs(hideBin(process.argv)) description: "Language model identifier", default: defaultModelId, }) + .option("mcp-config", { + type: "string", + description: "Path to a JSON file containing MCP server configuration (chat mode)", + }) .help() .parseSync(); @@ -65,6 +72,10 @@ async function main() { const workspaceRoot = argv.workspace as string; const modelId = argv.modelId; + if (argv.mcpConfig && (!argv.chat || argv.acp)) { + throw new Error("--mcp-config can only be used with --chat"); + } + if (argv.acp) { const stderrLog = console.error.bind(console); console.log = stderrLog; @@ -108,7 +119,15 @@ async function main() { process.exitCode = 1; return; } - const agent = await createCodingAgent(modelId, workspaceRoot); + const mcpServers = argv.mcpConfig + ? parseMcpServerConfigFile( + await readFile(resolve(argv.mcpConfig), "utf8"), + ) + : []; + const agent = await createCodingAgent(modelId, workspaceRoot, { mcpServers }); + if (mcpServers.length > 0) { + console.log(`🧩 MCP servers configured: ${mcpServers.length}`); + } console.log(`💬 Entering chat mode (type "exit" to quit)`); await startChatMode(agent, workspaceRoot, modelId, argv["chat-ui"] as CliChatUiMode); } else { diff --git a/package-lock.json b/package-lock.json index a4fa06f..360d8e1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@4onstudios/iris-agent", - "version": "0.4.1", + "version": "0.4.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@4onstudios/iris-agent", - "version": "0.4.1", + "version": "0.4.2", "license": "MIT", "dependencies": { "@agentclientprotocol/sdk": "^1.4.0", diff --git a/package.json b/package.json index 1995f15..1daca0e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@4onstudios/iris-agent", - "version": "0.4.1", + "version": "0.4.2", "private": false, "description": "Standalone HTTP coding-agent service and CLI with ACP (Agent Client Protocol) support.", "type": "module", diff --git a/tests/mcpSettings.test.ts b/tests/mcpSettings.test.ts index 41d9e79..d5d3f85 100644 --- a/tests/mcpSettings.test.ts +++ b/tests/mcpSettings.test.ts @@ -1,4 +1,5 @@ import { + parseMcpServerConfigFile, sanitizeMcpServer, toStableMcpFingerprint, redactMcpUrlForDisplay, @@ -46,6 +47,141 @@ describe("MCP server settings", () => { ).toBeNull(); }); + it("parses array and keyed-object MCP server configs", () => { + const server = { + id: "local-tools", + name: "Local tools", + command: "npx", + args: ["-y", "example-mcp-server"], + }; + + expect(parseMcpServerConfigFile(JSON.stringify([server]))).toMatchObject([ + { ...server, env: {}, enabled: true, command: "npx" }, + ]); + expect( + parseMcpServerConfigFile(JSON.stringify({ mcpServers: [server] })), + ).toMatchObject([{ ...server, env: {}, enabled: true, command: "npx" }]); + + expect( + parseMcpServerConfigFile( + JSON.stringify({ + mcpServers: { + filesystem: { + command: "npx", + args: ["-y", "server-filesystem", " /path with spaces ", ""], + env: { TOKEN: " token " }, + }, + }, + }), + ), + ).toMatchObject([ + { + id: "filesystem", + name: "filesystem", + command: "npx", + args: ["-y", "server-filesystem", " /path with spaces ", ""], + env: { TOKEN: " token " }, + enabled: true, + }, + ]); + }); + + it("rejects malformed MCP entries instead of rewriting their executable fields", () => { + expect(() => parseMcpServerConfigFile("{")).toThrow("not valid JSON"); + expect(() => parseMcpServerConfigFile("{}")).toThrow("mcpServers"); + expect(() => parseMcpServerConfigFile(JSON.stringify([{}]))).toThrow( + "invalid server entry", + ); + expect(() => + parseMcpServerConfigFile( + JSON.stringify([{ command: "npx", enabled: "false" }]), + ), + ).toThrow('"enabled" value must be a boolean'); + expect(() => + parseMcpServerConfigFile(JSON.stringify([{ command: 42, args: [] }])), + ).toThrow('"command" must be a non-empty string'); + expect(() => + parseMcpServerConfigFile(JSON.stringify([{ command: "npx", args: ["-y", 42] }])), + ).toThrow('"args" must contain only strings'); + expect(() => + parseMcpServerConfigFile( + JSON.stringify([{ command: "npx", args: [], env: { TOKEN: 42 } }]), + ), + ).toThrow('"env" contains an invalid key or value'); + }); + + it("rejects invalid present URLs rather than falling back to a local command", () => { + for (const url of [42, "", " "]) { + expect(() => + parseMcpServerConfigFile( + JSON.stringify([{ url, command: "some-stale-command" }]), + ), + ).toThrow('"url" must be a non-empty string'); + } + }); + + it("reports the maximum length when a configured URL is oversized", () => { + const url = `https://example.com/${"a".repeat(2000)}`; + expect(() => + parseMcpServerConfigFile(JSON.stringify([{ url }])), + ).toThrow('"url" exceeds 2000 characters'); + }); + + it("validates and preserves MCP header values from config files", () => { + expect(() => + parseMcpServerConfigFile( + JSON.stringify([ + { url: "https://example.com/mcp", headers: { Authorization: 42 } }, + ]), + ), + ).toThrow('"headers" contains an invalid key or value'); + + expect( + parseMcpServerConfigFile( + JSON.stringify([ + { + url: "https://example.com/mcp", + headers: { Authorization: " Bearer token " }, + }, + ]), + )[0].headers, + ).toEqual({ Authorization: " Bearer token " }); + }); + + it("rejects malformed HTTP header names and values", () => { + for (const headers of [ + { "Bad Header": "value" }, + { Authorization: "Bearer token\r\nX-Injected: yes" }, + ]) { + expect(() => + parseMcpServerConfigFile( + JSON.stringify([{ url: "https://example.com/mcp", headers }]), + ), + ).toThrow('"headers" contains an invalid key or value'); + + expect( + sanitizeMcpServer({ + url: "https://example.com/mcp", + headers, + }), + ).toBeNull(); + } + }); + + it("rejects HTTP header values containing control characters", () => { + const value = `Bearer token${String.fromCharCode(13, 10)}X-Injected: yes`; + const headers = { Authorization: value + String.fromCharCode(13) }; + + expect(() => + parseMcpServerConfigFile( + JSON.stringify([{ url: "https://example.com/mcp", headers }]), + ), + ).toThrow('"headers" contains an invalid key or value'); + expect( + sanitizeMcpServer({ url: "https://example.com/mcp", headers }), + ).toBeNull(); + }); + it("rejects the whole config when a malformed URL is paired with a stale command, instead of falling back to local mode", () => { // A config that was previously remote but now has a broken/unsupported // url must not silently resurrect the leftover `command` and launch a diff --git a/tests/mcpTools.test.ts b/tests/mcpTools.test.ts new file mode 100644 index 0000000..ecadae9 --- /dev/null +++ b/tests/mcpTools.test.ts @@ -0,0 +1,12 @@ +import { buildAugmentedPath } from "../api/core/agent/tools/mcpTools"; +import { delimiter as pathDelimiter } from "node:path"; + +describe("MCP tool environment", () => { + it("preserves configured PATH entries while adding standard executable paths", () => { + const configuredPath = ["/custom/mcp-bin", "/usr/bin"].join(pathDelimiter); + const pathEntries = buildAugmentedPath(configuredPath).split(pathDelimiter); + + expect(pathEntries.slice(0, 2)).toEqual(configuredPath.split(pathDelimiter)); + expect(pathEntries).toContain("/usr/local/bin"); + }); +});