From eda55408c7443f976da60f1400d2dd4c7009af0b Mon Sep 17 00:00:00 2001 From: It Apilium Date: Thu, 12 Mar 2026 22:51:49 +0100 Subject: [PATCH 1/3] fix: production hardening for MCP server - Prevent command injection in setup-claude host validation - Timing-safe bearer token comparison in HTTP transport - Graceful shutdown with closeAllConnections + SSE session tracking - Shutdown guard prevents double SIGINT execution - Word-boundary regex in governance to avoid substring false positives - Wrap all bare fetch calls in try/catch for Cortex-down resilience - Cap cortex_query limit at 500 to prevent abuse - Add hardening.test.ts covering all fixes (12 tests) --- extensions/mcp-server/cortex-tools.ts | 112 ++++++---- extensions/mcp-server/governance-tools.ts | 50 ++++- extensions/mcp-server/hardening.test.ts | 247 ++++++++++++++++++++++ extensions/mcp-server/memory-tools.ts | 183 +++++++++++----- extensions/mcp-server/setup-claude.ts | 23 +- extensions/mcp-server/transport-http.ts | 21 +- src/cli/serve-cli.ts | 31 ++- 7 files changed, 539 insertions(+), 128 deletions(-) create mode 100644 extensions/mcp-server/hardening.test.ts diff --git a/extensions/mcp-server/cortex-tools.ts b/extensions/mcp-server/cortex-tools.ts index 30041fe..9748060 100644 --- a/extensions/mcp-server/cortex-tools.ts +++ b/extensions/mcp-server/cortex-tools.ts @@ -31,39 +31,50 @@ export function createCortexTools(deps: CortexToolDeps): AdaptableTool[] { limit: Type.Optional(Type.Number({ description: "Max results (default 20)" })), }), execute: async (_id: string, params: Record) => { - const limit = (params.limit as number) ?? 20; + const limit = Math.min((params.limit as number) ?? 20, 500); const queryParams = new URLSearchParams(); if (params.subject) queryParams.set("subject", params.subject as string); if (params.predicate) queryParams.set("predicate", params.predicate as string); if (params.object) queryParams.set("object", params.object as string); queryParams.set("limit", String(limit)); - const res = await fetch(`${cortexBaseUrl}/api/v1/triples?${queryParams}`); - if (!res.ok) { - return { - content: [{ type: "text" as const, text: `Query failed: ${res.statusText}` }], - }; - } + try { + const res = await fetch(`${cortexBaseUrl}/api/v1/triples?${queryParams}`); + if (!res.ok) { + return { + content: [{ type: "text" as const, text: `Query failed: ${res.statusText}` }], + }; + } - const data = (await res.json()) as { - triples: Array<{ subject: string; predicate: string; object: unknown }>; - }; - if (!data.triples || data.triples.length === 0) { - return { content: [{ type: "text" as const, text: "No triples found." }] }; - } + const data = (await res.json()) as { + triples: Array<{ subject: string; predicate: string; object: unknown }>; + }; + if (!data.triples || data.triples.length === 0) { + return { content: [{ type: "text" as const, text: "No triples found." }] }; + } - const formatted = data.triples - .map((t) => ` ${t.subject} -> ${t.predicate} -> ${JSON.stringify(t.object)}`) - .join("\n"); + const formatted = data.triples + .map((t) => ` ${t.subject} -> ${t.predicate} -> ${JSON.stringify(t.object)}`) + .join("\n"); - return { - content: [ - { - type: "text" as const, - text: `Found ${data.triples.length} triples:\n${formatted}`, - }, - ], - }; + return { + content: [ + { + type: "text" as const, + text: `Found ${data.triples.length} triples:\n${formatted}`, + }, + ], + }; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Cortex query unavailable. Cortex may not be running.", + }, + ], + }; + } }, }, @@ -80,30 +91,41 @@ export function createCortexTools(deps: CortexToolDeps): AdaptableTool[] { object: Type.String({ description: "Object/value (e.g., 'Express.js')" }), }), execute: async (_id: string, params: Record) => { - const res = await fetch(`${cortexBaseUrl}/api/v1/triples`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - subject: params.subject, - predicate: params.predicate, - object: params.object, - }), - }); - - if (!res.ok) { + try { + const res = await fetch(`${cortexBaseUrl}/api/v1/triples`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + subject: params.subject, + predicate: params.predicate, + object: params.object, + }), + }); + + if (!res.ok) { + return { + content: [{ type: "text" as const, text: `Store failed: ${res.statusText}` }], + }; + } + return { - content: [{ type: "text" as const, text: `Store failed: ${res.statusText}` }], + content: [ + { + type: "text" as const, + text: `Stored: ${params.subject as string} -> ${params.predicate as string} -> ${params.object as string}`, + }, + ], + }; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Cortex store unavailable. Cortex may not be running.", + }, + ], }; } - - return { - content: [ - { - type: "text" as const, - text: `Stored: ${params.subject as string} -> ${params.predicate as string} -> ${params.object as string}`, - }, - ], - }; }, }, diff --git a/extensions/mcp-server/governance-tools.ts b/extensions/mcp-server/governance-tools.ts index c7dc889..aa89f8b 100644 --- a/extensions/mcp-server/governance-tools.ts +++ b/extensions/mcp-server/governance-tools.ts @@ -26,29 +26,59 @@ export function createGovernanceTools(): AdaptableTool[] { const target = params.target as string; const { readFile, access } = await import("node:fs/promises"); - const policyPath = `${process.cwd()}/MAYROS.md`; + const { join } = await import("node:path"); + const { homedir } = await import("node:os"); + + // Search policy file in project dir, then user config + const candidates = [ + join(process.cwd(), "MAYROS.md"), + join(homedir(), ".mayros", "MAYROS.md"), + ]; + + let policyContent: string | null = null; + let policyPath = candidates[0]; + for (const candidate of candidates) { + try { + await access(candidate); + policyContent = await readFile(candidate, "utf-8"); + policyPath = candidate; + break; + } catch { + // Try next candidate + } + } try { - await access(policyPath); - const content = await readFile(policyPath, "utf-8"); + if (!policyContent) { + return { + content: [ + { + type: "text" as const, + text: `ALLOWED (no policy): No MAYROS.md found. All actions permitted.`, + }, + ], + }; + } // Pattern matching against DENY/ALLOW rules const denyPatterns: string[] = []; - for (const line of content.split("\n")) { + for (const line of policyContent.split("\n")) { const trimmed = line.trim(); if (trimmed.startsWith("- DENY:")) { denyPatterns.push(trimmed.slice(7).trim()); } } - // Check deny rules + // Check deny rules with word-boundary matching for (const pattern of denyPatterns) { - if (target.includes(pattern) || action.includes(pattern)) { + const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const regex = new RegExp(`(?:^|[\\s/\\\\.:_-])${escaped}(?:$|[\\s/\\\\.:_-])`, "i"); + if (regex.test(target) || regex.test(action)) { return { content: [ { type: "text" as const, - text: `DENIED: "${target}" matches deny rule "${pattern}"`, + text: `DENIED: "${target}" matches deny rule "${pattern}" (from ${policyPath})`, }, ], }; @@ -59,16 +89,16 @@ export function createGovernanceTools(): AdaptableTool[] { content: [ { type: "text" as const, - text: `ALLOWED: "${action}" on "${target}" — no deny rules matched (${denyPatterns.length} rules checked)`, + text: `ALLOWED: "${action}" on "${target}" — no deny rules matched (${denyPatterns.length} rules checked, from ${policyPath})`, }, ], }; - } catch { + } catch (err) { return { content: [ { type: "text" as const, - text: `ALLOWED (no policy): No MAYROS.md found at ${policyPath}. All actions permitted.`, + text: `Policy check error: ${err instanceof Error ? err.message : String(err)}`, }, ], }; diff --git a/extensions/mcp-server/hardening.test.ts b/extensions/mcp-server/hardening.test.ts new file mode 100644 index 0000000..0ed4583 --- /dev/null +++ b/extensions/mcp-server/hardening.test.ts @@ -0,0 +1,247 @@ +/** + * Tests for production hardening fixes: + * - Command injection prevention in setup-claude + * - Governance word-boundary matching + * - Memory tools error handling when Cortex is down + * - Cortex tools error handling when Cortex is down + * - Shutdown guard idempotency + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// ── setup-claude: host validation ────────────────────────────────── + +describe("setup-claude host validation", () => { + it("rejects hosts with shell metacharacters", async () => { + const logs: string[] = []; + const origError = console.error; + console.error = (msg: string) => logs.push(msg); + + const mod = await import("./setup-claude.js"); + await mod.setupClaudeCodeMcp({ + port: 19100, + host: "; rm -rf /", + transport: "http", + target: "code", + }); + + console.error = origError; + expect(logs.some((l) => l.includes("Invalid host"))).toBe(true); + }); + + it("accepts valid hostnames", async () => { + // Should not throw for valid hosts (will fail at execSync but that's OK) + const mod = await import("./setup-claude.js"); + // We just verify no "Invalid host" error for valid hosts + const logs: string[] = []; + const origError = console.error; + console.error = (msg: string) => logs.push(msg); + + // This will fail at execSync (claude not found) but should NOT reject the host + await mod.setupClaudeCodeMcp({ + port: 19100, + host: "127.0.0.1", + transport: "http", + target: "code", + }); + + console.error = origError; + expect(logs.some((l) => l.includes("Invalid host"))).toBe(false); + }); +}); + +// ── governance: word-boundary matching ───────────────────────────── + +describe("governance word-boundary matching", () => { + let mockFs: { access: ReturnType; readFile: ReturnType }; + + beforeEach(() => { + mockFs = { + access: vi.fn(), + readFile: vi.fn(), + }; + }); + + it("DENY rule 'rm' should not match 'format'", async () => { + const mod = await import("./governance-tools.js"); + const tools = mod.createGovernanceTools(); + const tool = tools[0]; + + // Mock fs to return a policy with "- DENY: rm" + vi.doMock("node:fs/promises", () => mockFs); + mockFs.access.mockResolvedValue(undefined); + mockFs.readFile.mockResolvedValue("- DENY: rm\n"); + + // Since we can't easily mock fs/promises inside the tool's dynamic import, + // test the regex pattern directly + const pattern = "rm"; + const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const regex = new RegExp(`(?:^|[\\s/\\\\.:_-])${escaped}(?:$|[\\s/\\\\.:_-])`, "i"); + + expect(regex.test("format")).toBe(false); + expect(regex.test("rm -rf /")).toBe(true); + expect(regex.test("/bin/rm")).toBe(true); + expect(regex.test("perform")).toBe(false); + expect(regex.test("rm")).toBe(true); + + vi.doUnmock("node:fs/promises"); + }); + + it("DENY rule 'eval' should not match 'evaluate'", () => { + const pattern = "eval"; + const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const regex = new RegExp(`(?:^|[\\s/\\\\.:_-])${escaped}(?:$|[\\s/\\\\.:_-])`, "i"); + + expect(regex.test("evaluate")).toBe(false); + expect(regex.test("eval()")).toBe(false); // () is not a boundary char + expect(regex.test("node -e eval")).toBe(true); + expect(regex.test("eval")).toBe(true); + }); +}); + +// ── memory-tools: Cortex down ────────────────────────────────────── + +describe("memory-tools with Cortex unreachable", () => { + let origFetch: typeof globalThis.fetch; + + beforeEach(() => { + origFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + }); + + afterEach(() => { + globalThis.fetch = origFetch; + }); + + it("mayros_remember returns warning when Cortex is down", async () => { + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const remember = tools.find((t) => t.name === "mayros_remember")!; + + const result = await remember.execute("id", { content: "test fact", category: "fact" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("Remembered"); + expect(text).toContain("Warnings"); + }); + + it("mayros_recall returns friendly message when Cortex is down", async () => { + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const recall = tools.find((t) => t.name === "mayros_recall")!; + + const result = await recall.execute("id", { query: "test" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("unavailable"); + }); + + it("mayros_search returns friendly message when Cortex is down", async () => { + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const search = tools.find((t) => t.name === "mayros_search")!; + + const result = await search.execute("id", { text: "test" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("unavailable"); + }); + + it("mayros_forget returns friendly message when Cortex is down", async () => { + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const forget = tools.find((t) => t.name === "mayros_forget")!; + + const result = await forget.execute("id", { id: "mem-123" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("unavailable"); + }); +}); + +// ── cortex-tools: Cortex down ────────────────────────────────────── + +describe("cortex-tools with Cortex unreachable", () => { + let origFetch: typeof globalThis.fetch; + + beforeEach(() => { + origFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + }); + + afterEach(() => { + globalThis.fetch = origFetch; + }); + + it("mayros_cortex_query returns friendly message", async () => { + const mod = await import("./cortex-tools.js"); + const tools = mod.createCortexTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const query = tools.find((t) => t.name === "mayros_cortex_query")!; + + const result = await query.execute("id", { subject: "test" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("unavailable"); + }); + + it("mayros_cortex_store returns friendly message", async () => { + const mod = await import("./cortex-tools.js"); + const tools = mod.createCortexTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const store = tools.find((t) => t.name === "mayros_cortex_store")!; + + const result = await store.execute("id", { subject: "a", predicate: "b", object: "c" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("unavailable"); + }); + + it("mayros_cortex_query caps limit at 500", async () => { + globalThis.fetch = origFetch; // restore for this test + const mod = await import("./cortex-tools.js"); + const tools = mod.createCortexTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const query = tools.find((t) => t.name === "mayros_cortex_query")!; + + // Mock fetch to capture the URL + const mockFetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + globalThis.fetch = mockFetch; + + await query.execute("id", { limit: 1000000 }); + const calledUrl = mockFetch.mock.calls[0][0] as string; + expect(calledUrl).toContain("limit=500"); + }); +}); + +// ── shutdown guard ───────────────────────────────────────────────── + +describe("shutdown guard", () => { + it("shuttingDown flag prevents double execution", async () => { + let callCount = 0; + let shuttingDown = false; + + const shutdown = async () => { + if (shuttingDown) return; + shuttingDown = true; + callCount++; + await new Promise((r) => setTimeout(r, 10)); + }; + + // Simulate double SIGINT + await Promise.all([shutdown(), shutdown()]); + expect(callCount).toBe(1); + }); +}); diff --git a/extensions/mcp-server/memory-tools.ts b/extensions/mcp-server/memory-tools.ts index ecf8f76..747267c 100644 --- a/extensions/mcp-server/memory-tools.ts +++ b/extensions/mcp-server/memory-tools.ts @@ -66,31 +66,43 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { ]; // Store in Cortex graph + const errors: string[] = []; for (const triple of triples) { - await fetch(`${cortexBaseUrl}/api/v1/triples`, { + try { + const tripleRes = await fetch(`${cortexBaseUrl}/api/v1/triples`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(triple), + }); + if (!tripleRes.ok) errors.push(`triple store: ${tripleRes.statusText}`); + } catch (err) { + errors.push(`triple store: ${err instanceof Error ? err.message : String(err)}`); + } + } + + // Also store in Ineru STM for vector search + try { + const memRes = await fetch(`${cortexBaseUrl}/api/v1/memory/remember`, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(triple), + body: JSON.stringify({ + entry_type: category, + data: { content, tags }, + tags, + importance, + }), }); + if (!memRes.ok) errors.push(`ineru store: ${memRes.statusText}`); + } catch (err) { + errors.push(`ineru store: ${err instanceof Error ? err.message : String(err)}`); } - // Also store in Ineru STM for vector search - await fetch(`${cortexBaseUrl}/api/v1/memory/remember`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - entry_type: category, - data: { content, tags }, - tags, - importance, - }), - }); - + const summary = `Remembered: "${content.slice(0, 80)}${content.length > 80 ? "..." : ""}" [${category}]${tags.length > 0 ? ` #${tags.join(" #")}` : ""}`; return { content: [ { type: "text" as const, - text: `Remembered: "${content.slice(0, 80)}${content.length > 80 ? "..." : ""}" [${category}]${tags.length > 0 ? ` #${tags.join(" #")}` : ""}`, + text: errors.length > 0 ? `${summary}\nWarnings: ${errors.join("; ")}` : summary, }, ], }; @@ -117,40 +129,73 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { const limit = (params.limit as number) ?? 10; // Query Ineru recall endpoint - const recallRes = await fetch(`${cortexBaseUrl}/api/v1/memory/recall`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - text: query, - tags: tags ?? [], - entry_type: category, - limit, - }), - }); - - if (!recallRes.ok) { - // Fallback: query Cortex graph directly - const graphRes = await fetch( - `${cortexBaseUrl}/api/v1/triples?predicate=${encodeURIComponent(`${namespace}:memory:content`)}&limit=${limit}`, - ); - const graphData = (await graphRes.json()) as { - triples?: Array<{ object: string }>; - }; - const triples = graphData.triples ?? []; - + let recallRes: Response; + try { + recallRes = await fetch(`${cortexBaseUrl}/api/v1/memory/recall`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + text: query, + tags: tags ?? [], + entry_type: category, + limit, + }), + }); + } catch { return { content: [ { type: "text" as const, - text: - triples.length > 0 - ? triples.map((t, i) => `${i + 1}. ${t.object}`).join("\n") - : "No memories found.", + text: "Memory recall unavailable. Cortex may not be running.", }, ], }; } + if (!recallRes.ok) { + // Fallback: query Cortex graph directly + try { + const graphRes = await fetch( + `${cortexBaseUrl}/api/v1/triples?predicate=${encodeURIComponent(`${namespace}:memory:content`)}&limit=${limit}`, + ); + if (!graphRes.ok) { + return { + content: [ + { + type: "text" as const, + text: "Memory recall unavailable. Cortex may not be running.", + }, + ], + }; + } + const graphData = (await graphRes.json()) as { + triples?: Array<{ object: string }>; + }; + const triples = graphData.triples ?? []; + + return { + content: [ + { + type: "text" as const, + text: + triples.length > 0 + ? triples.map((t, i) => `${i + 1}. ${t.object}`).join("\n") + : "No memories found.", + }, + ], + }; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Memory recall unavailable. Cortex may not be running.", + }, + ], + }; + } + } + const memories = (await recallRes.json()) as Array<{ id: string; entry_type: string; @@ -201,11 +246,23 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { const text = params.text as string; const k = (params.k as number) ?? 5; - const recallRes = await fetch(`${cortexBaseUrl}/api/v1/memory/recall`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ text, limit: k }), - }); + let recallRes: Response; + try { + recallRes = await fetch(`${cortexBaseUrl}/api/v1/memory/recall`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ text, limit: k }), + }); + } catch { + return { + content: [ + { + type: "text" as const, + text: "Vector search unavailable. Cortex may not be running.", + }, + ], + }; + } if (!recallRes.ok) { return { @@ -254,19 +311,33 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { }), execute: async (_id: string, params: Record) => { const memoryId = params.id as string; - const res = await fetch(`${cortexBaseUrl}/api/v1/memory/${encodeURIComponent(memoryId)}`, { - method: "DELETE", - }); - return { - content: [ + try { + const res = await fetch( + `${cortexBaseUrl}/api/v1/memory/${encodeURIComponent(memoryId)}`, { - type: "text" as const, - text: res.ok - ? `Memory ${memoryId} forgotten.` - : `Failed to forget: ${res.statusText}`, + method: "DELETE", }, - ], - }; + ); + return { + content: [ + { + type: "text" as const, + text: res.ok + ? `Memory ${memoryId} forgotten.` + : `Failed to forget: ${res.statusText}`, + }, + ], + }; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Memory delete unavailable. Cortex may not be running.", + }, + ], + }; + } }, }, ]; diff --git a/extensions/mcp-server/setup-claude.ts b/extensions/mcp-server/setup-claude.ts index 3b714cd..776291d 100644 --- a/extensions/mcp-server/setup-claude.ts +++ b/extensions/mcp-server/setup-claude.ts @@ -39,15 +39,26 @@ export async function setupClaudeCodeMcp(opts: SetupClaudeOpts): Promise { // ── Claude Code ──────────────────────────────────────────────────── +function validateHost(host: string): boolean { + return /^[a-zA-Z0-9._-]+$/.test(host); +} + function setupCode(opts: SetupClaudeOpts): void { const transport = opts.transport ?? "stdio"; + if (!validateHost(opts.host)) { + console.error( + `Invalid host: "${opts.host}". Must contain only alphanumeric, dots, hyphens, or underscores.`, + ); + return; + } + try { if (transport === "stdio") { execSync("claude mcp add mayros -- mayros serve --stdio", { stdio: "inherit" }); } else { const url = `http://${opts.host}:${opts.port}/mcp`; - execSync(`claude mcp add mayros -s http --url ${url}`, { stdio: "inherit" }); + execSync(`claude mcp add mayros -s http --url ${JSON.stringify(url)}`, { stdio: "inherit" }); } console.log("Mayros registered with Claude Code."); } catch { @@ -126,11 +137,15 @@ function getDesktopConfigPath(): string | null { } function resolveNodePath(): string | null { + const os = platform(); try { - return execSync("which node", { encoding: "utf-8" }).trim(); + const cmd = os === "win32" ? "where node" : "which node"; + return execSync(cmd, { encoding: "utf-8" }).trim().split("\n")[0]; } catch { - // Fallback common paths - const candidates = ["/opt/homebrew/bin/node", "/usr/local/bin/node", "/usr/bin/node"]; + const candidates = + os === "win32" + ? ["C:\\Program Files\\nodejs\\node.exe", "C:\\Program Files (x86)\\nodejs\\node.exe"] + : ["/opt/homebrew/bin/node", "/usr/local/bin/node", "/usr/bin/node"]; return candidates.find((p) => existsSync(p)) ?? null; } } diff --git a/extensions/mcp-server/transport-http.ts b/extensions/mcp-server/transport-http.ts index b05cea8..6fe1f81 100644 --- a/extensions/mcp-server/transport-http.ts +++ b/extensions/mcp-server/transport-http.ts @@ -9,7 +9,7 @@ */ import { createServer, type Server, type IncomingMessage, type ServerResponse } from "node:http"; -import { randomUUID } from "node:crypto"; +import { randomUUID, timingSafeEqual } from "node:crypto"; import type { McpProtocolDispatcher } from "./protocol.js"; // ============================================================================ @@ -74,7 +74,7 @@ export class McpHttpTransport { /** Stop the HTTP server. */ async stop(): Promise { - // Close all active SSE sessions + // Close all active SSE sessions (legacy + modern) for (const [id, res] of this.sseSessions) { if (!res.destroyed) res.end(); this.sseSessions.delete(id); @@ -85,6 +85,8 @@ export class McpHttpTransport { resolve(); return; } + // Destroy all remaining keep-alive connections so server.close() resolves + this.server.closeAllConnections(); this.server.close(() => { this.server = null; resolve(); @@ -118,10 +120,15 @@ export class McpHttpTransport { return; } - // Auth check + // Auth check (timing-safe comparison) if (this.authToken) { const auth = req.headers.authorization; - if (!auth || auth !== `Bearer ${this.authToken}`) { + const expected = `Bearer ${this.authToken}`; + if ( + !auth || + auth.length !== expected.length || + !timingSafeEqual(Buffer.from(auth), Buffer.from(expected)) + ) { res.writeHead(401, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: "Unauthorized" })); return; @@ -211,12 +218,17 @@ export class McpHttpTransport { } private handleMcpSse(res: ServerResponse): void { + const sessionId = `mcp-sse-${randomUUID()}`; + res.writeHead(200, { "Content-Type": "text/event-stream", "Cache-Control": "no-cache", Connection: "keep-alive", }); + // Track session for cleanup on shutdown + this.sseSessions.set(sessionId, res); + // Send initial ping res.write("event: ping\ndata: {}\n\n"); @@ -231,6 +243,7 @@ export class McpHttpTransport { res.on("close", () => { clearInterval(keepAlive); + this.sseSessions.delete(sessionId); }); } diff --git a/src/cli/serve-cli.ts b/src/cli/serve-cli.ts index c675b3e..91f4613 100644 --- a/src/cli/serve-cli.ts +++ b/src/cli/serve-cli.ts @@ -129,15 +129,22 @@ export function registerServeCli(program: Command): void { await server.start(); - // Shutdown handler: stop server + sidecar - const shutdown = () => { - void (async () => { - if (sidecar) await sidecar.stop(); + // Shutdown handler: stop server + sidecar (both must run even if one fails) + let shuttingDown = false; + const shutdown = async () => { + if (shuttingDown) return; + shuttingDown = true; + try { await server.stop(); - })(); + } catch (err) { + process.stderr.write(`ERROR stopping server: ${err}\n`); + } + try { + if (sidecar) await sidecar.stop(); + } catch (err) { + process.stderr.write(`ERROR stopping sidecar: ${err}\n`); + } }; - process.on("SIGINT", shutdown); - process.on("SIGTERM", shutdown); if (transport !== "stdio") { const status = server.status(); @@ -148,9 +155,15 @@ export function registerServeCli(program: Command): void { ); await new Promise((resolve) => { - process.on("SIGINT", resolve); - process.on("SIGTERM", resolve); + const onSignal = () => { + void shutdown().finally(resolve); + }; + process.once("SIGINT", onSignal); + process.once("SIGTERM", onSignal); }); + } else { + process.once("SIGINT", () => void shutdown()); + process.once("SIGTERM", () => void shutdown()); } }); } From 6f2f23de1e695865325364eaad23c3f03205ab63 Mon Sep 17 00:00:00 2001 From: It Apilium Date: Thu, 12 Mar 2026 22:52:19 +0100 Subject: [PATCH 2/3] fix: resolve lint warnings in hardening code - Prefix unused variable with underscore in test - Add explicit unknown type annotation to catch clauses --- extensions/mcp-server/hardening.test.ts | 2 +- src/cli/serve-cli.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/mcp-server/hardening.test.ts b/extensions/mcp-server/hardening.test.ts index 0ed4583..a644424 100644 --- a/extensions/mcp-server/hardening.test.ts +++ b/extensions/mcp-server/hardening.test.ts @@ -65,7 +65,7 @@ describe("governance word-boundary matching", () => { it("DENY rule 'rm' should not match 'format'", async () => { const mod = await import("./governance-tools.js"); const tools = mod.createGovernanceTools(); - const tool = tools[0]; + const _tool = tools[0]; // Mock fs to return a policy with "- DENY: rm" vi.doMock("node:fs/promises", () => mockFs); diff --git a/src/cli/serve-cli.ts b/src/cli/serve-cli.ts index 91f4613..1c2abbf 100644 --- a/src/cli/serve-cli.ts +++ b/src/cli/serve-cli.ts @@ -136,13 +136,13 @@ export function registerServeCli(program: Command): void { shuttingDown = true; try { await server.stop(); - } catch (err) { - process.stderr.write(`ERROR stopping server: ${err}\n`); + } catch (err: unknown) { + process.stderr.write(`ERROR stopping server: ${String(err)}\n`); } try { if (sidecar) await sidecar.stop(); - } catch (err) { - process.stderr.write(`ERROR stopping sidecar: ${err}\n`); + } catch (err: unknown) { + process.stderr.write(`ERROR stopping sidecar: ${String(err)}\n`); } }; From d4de58dc5a415b6942f4a039c3f20d2184a1db23 Mon Sep 17 00:00:00 2001 From: It Apilium Date: Thu, 12 Mar 2026 23:19:11 +0100 Subject: [PATCH 3/3] fix: deep hardening for MCP server production readiness - CORS default-deny when allowedOrigins is empty - Cap SSE sessions at 50 to prevent connection exhaustion - Require Content-Type application/json on POST endpoints - Wrap .json() calls in try/catch for non-JSON 200 responses - Validate tags with Array.isArray, cap limit/k at 100 - Use execFileSync instead of execSync to avoid shell injection - Add port validation (NaN, range 1-65535) in setup-claude - Add host validation in config parser for all code paths - Cache shutdown promise to prevent concurrent signal races - Add skipSignalHandlers option to CortexSidecar - Log sidecar/agent errors to stderr instead of swallowing - Explicit process.exit after shutdown in both HTTP and stdio modes - Rewrite hardening tests to exercise production code directly (20 tests) --- extensions/mcp-server/config.ts | 5 + extensions/mcp-server/hardening.test.ts | 334 +++++++++++++++---- extensions/mcp-server/memory-tools.ts | 42 ++- extensions/mcp-server/setup-claude.ts | 15 +- extensions/mcp-server/transport-http.ts | 38 ++- extensions/memory-semantic/cortex-sidecar.ts | 27 +- src/cli/serve-cli.ts | 61 ++-- 7 files changed, 403 insertions(+), 119 deletions(-) diff --git a/extensions/mcp-server/config.ts b/extensions/mcp-server/config.ts index d16e281..ecb59b1 100644 --- a/extensions/mcp-server/config.ts +++ b/extensions/mcp-server/config.ts @@ -135,6 +135,11 @@ export const mcpServerConfigSchema = { } const host = typeof cfg.host === "string" ? cfg.host : DEFAULT_HOST; + if (!/^[a-zA-Z0-9._-]+$/.test(host)) { + throw new Error( + `Invalid host: "${host}". Must contain only alphanumeric, dots, hyphens, or underscores.`, + ); + } const auth = parseAuthConfig(cfg.auth); const capabilities = parseCapabilities(cfg.capabilities); diff --git a/extensions/mcp-server/hardening.test.ts b/extensions/mcp-server/hardening.test.ts index a644424..1e331d7 100644 --- a/extensions/mcp-server/hardening.test.ts +++ b/extensions/mcp-server/hardening.test.ts @@ -1,15 +1,19 @@ /** * Tests for production hardening fixes: * - Command injection prevention in setup-claude - * - Governance word-boundary matching + * - Governance word-boundary matching (via actual tool execution) * - Memory tools error handling when Cortex is down + * - Memory tools JSON parse resilience + * - Memory tools input validation (tags, limits) * - Cortex tools error handling when Cortex is down - * - Shutdown guard idempotency + * - Transport: CORS default-deny, SSE cap, Content-Type enforcement + * - Config: host validation + * - Shutdown guard (promise-cached pattern) */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -// ── setup-claude: host validation ────────────────────────────────── +// ── setup-claude: host + port validation ───────────────────────────── describe("setup-claude host validation", () => { it("rejects hosts with shell metacharacters", async () => { @@ -29,73 +33,106 @@ describe("setup-claude host validation", () => { expect(logs.some((l) => l.includes("Invalid host"))).toBe(true); }); - it("accepts valid hostnames", async () => { - // Should not throw for valid hosts (will fail at execSync but that's OK) - const mod = await import("./setup-claude.js"); - // We just verify no "Invalid host" error for valid hosts + it("rejects NaN port", async () => { const logs: string[] = []; const origError = console.error; console.error = (msg: string) => logs.push(msg); - // This will fail at execSync (claude not found) but should NOT reject the host + const mod = await import("./setup-claude.js"); await mod.setupClaudeCodeMcp({ - port: 19100, + port: NaN, host: "127.0.0.1", transport: "http", target: "code", }); console.error = origError; - expect(logs.some((l) => l.includes("Invalid host"))).toBe(false); + expect(logs.some((l) => l.includes("Invalid port"))).toBe(true); }); -}); -// ── governance: word-boundary matching ───────────────────────────── + it("accepts valid hostnames without error", async () => { + const mod = await import("./setup-claude.js"); + const errors: string[] = []; + const origError = console.error; + console.error = (msg: string) => errors.push(msg); -describe("governance word-boundary matching", () => { - let mockFs: { access: ReturnType; readFile: ReturnType }; + // Will fail at execFileSync (claude not found) but should NOT reject host/port + await mod.setupClaudeCodeMcp({ + port: 19100, + host: "127.0.0.1", + transport: "http", + target: "code", + }); - beforeEach(() => { - mockFs = { - access: vi.fn(), - readFile: vi.fn(), - }; + console.error = origError; + expect(errors.some((l) => l.includes("Invalid host"))).toBe(false); + expect(errors.some((l) => l.includes("Invalid port"))).toBe(false); }); +}); - it("DENY rule 'rm' should not match 'format'", async () => { - const mod = await import("./governance-tools.js"); - const tools = mod.createGovernanceTools(); - const _tool = tools[0]; - - // Mock fs to return a policy with "- DENY: rm" - vi.doMock("node:fs/promises", () => mockFs); - mockFs.access.mockResolvedValue(undefined); - mockFs.readFile.mockResolvedValue("- DENY: rm\n"); - - // Since we can't easily mock fs/promises inside the tool's dynamic import, - // test the regex pattern directly - const pattern = "rm"; - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const regex = new RegExp(`(?:^|[\\s/\\\\.:_-])${escaped}(?:$|[\\s/\\\\.:_-])`, "i"); +// ── config: host validation at parser level ────────────────────────── - expect(regex.test("format")).toBe(false); - expect(regex.test("rm -rf /")).toBe(true); - expect(regex.test("/bin/rm")).toBe(true); - expect(regex.test("perform")).toBe(false); - expect(regex.test("rm")).toBe(true); +describe("config host validation", () => { + it("rejects malicious host via config parser", async () => { + const { mcpServerConfigSchema } = await import("./config.js"); + expect(() => mcpServerConfigSchema.parse({ host: "$(whoami)" })).toThrow("Invalid host"); + }); - vi.doUnmock("node:fs/promises"); + it("accepts valid host via config parser", async () => { + const { mcpServerConfigSchema } = await import("./config.js"); + const config = mcpServerConfigSchema.parse({ host: "my-server.local" }); + expect(config.host).toBe("my-server.local"); }); +}); - it("DENY rule 'eval' should not match 'evaluate'", () => { - const pattern = "eval"; - const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const regex = new RegExp(`(?:^|[\\s/\\\\.:_-])${escaped}(?:$|[\\s/\\\\.:_-])`, "i"); +// ── governance: word-boundary matching (via actual tool) ───────────── - expect(regex.test("evaluate")).toBe(false); - expect(regex.test("eval()")).toBe(false); // () is not a boundary char - expect(regex.test("node -e eval")).toBe(true); - expect(regex.test("eval")).toBe(true); +describe("governance word-boundary matching", () => { + it("DENY rule 'rm' blocks 'rm -rf /' but not 'format'", async () => { + const { writeFile, mkdir } = await import("node:fs/promises"); + const { join } = await import("node:path"); + const { tmpdir } = await import("node:os"); + const { randomBytes } = await import("node:crypto"); + + // Create a temp MAYROS.md with a DENY rule + const dir = join(tmpdir(), `hardening-test-${randomBytes(4).toString("hex")}`); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, "MAYROS.md"), "- DENY: rm\n- DENY: eval\n"); + + const origCwd = process.cwd(); + process.chdir(dir); + + try { + const mod = await import("./governance-tools.js"); + const tools = mod.createGovernanceTools(); + const tool = tools[0]; + + // Should DENY "rm -rf /" + const denied = await tool.execute("id", { action: "shell_command", target: "rm -rf /" }); + expect((denied.content[0] as { text: string }).text).toContain("DENIED"); + + // Should ALLOW "format" (no substring match) + const allowed = await tool.execute("id", { action: "shell_command", target: "format disk" }); + expect((allowed.content[0] as { text: string }).text).toContain("ALLOWED"); + + // Should ALLOW "evaluate" (no substring match on "eval") + const evalAllowed = await tool.execute("id", { + action: "shell_command", + target: "evaluate expression", + }); + expect((evalAllowed.content[0] as { text: string }).text).toContain("ALLOWED"); + + // Should DENY standalone "eval" + const evalDenied = await tool.execute("id", { + action: "shell_command", + target: "node -e eval", + }); + expect((evalDenied.content[0] as { text: string }).text).toContain("DENIED"); + } finally { + process.chdir(origCwd); + const { rm } = await import("node:fs/promises"); + await rm(dir, { recursive: true, force: true }); + } }); }); @@ -113,7 +150,7 @@ describe("memory-tools with Cortex unreachable", () => { globalThis.fetch = origFetch; }); - it("mayros_remember returns warning when Cortex is down", async () => { + it("mayros_remember does not throw and returns warning", async () => { const mod = await import("./memory-tools.js"); const tools = mod.createMemoryTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -121,13 +158,15 @@ describe("memory-tools with Cortex unreachable", () => { }); const remember = tools.find((t) => t.name === "mayros_remember")!; + // Must not throw const result = await remember.execute("id", { content: "test fact", category: "fact" }); const text = (result.content[0] as { text: string }).text; expect(text).toContain("Remembered"); expect(text).toContain("Warnings"); + expect(text).toContain("fetch failed"); }); - it("mayros_recall returns friendly message when Cortex is down", async () => { + it("mayros_recall does not throw and returns unavailable", async () => { const mod = await import("./memory-tools.js"); const tools = mod.createMemoryTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -137,10 +176,10 @@ describe("memory-tools with Cortex unreachable", () => { const result = await recall.execute("id", { query: "test" }); const text = (result.content[0] as { text: string }).text; - expect(text).toContain("unavailable"); + expect(text).toContain("Memory recall unavailable"); }); - it("mayros_search returns friendly message when Cortex is down", async () => { + it("mayros_search does not throw and returns unavailable", async () => { const mod = await import("./memory-tools.js"); const tools = mod.createMemoryTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -150,10 +189,10 @@ describe("memory-tools with Cortex unreachable", () => { const result = await search.execute("id", { text: "test" }); const text = (result.content[0] as { text: string }).text; - expect(text).toContain("unavailable"); + expect(text).toContain("Vector search unavailable"); }); - it("mayros_forget returns friendly message when Cortex is down", async () => { + it("mayros_forget does not throw and returns unavailable", async () => { const mod = await import("./memory-tools.js"); const tools = mod.createMemoryTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -163,7 +202,104 @@ describe("memory-tools with Cortex unreachable", () => { const result = await forget.execute("id", { id: "mem-123" }); const text = (result.content[0] as { text: string }).text; - expect(text).toContain("unavailable"); + expect(text).toContain("Memory delete unavailable"); + }); +}); + +// ── memory-tools: JSON parse resilience ────────────────────────────── + +describe("memory-tools JSON parse resilience", () => { + let origFetch: typeof globalThis.fetch; + + beforeEach(() => { + origFetch = globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = origFetch; + }); + + it("mayros_recall handles non-JSON 200 response", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.reject(new SyntaxError("Unexpected token < in JSON")), + }); + + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const recall = tools.find((t) => t.name === "mayros_recall")!; + + const result = await recall.execute("id", { query: "test" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("failed"); + }); + + it("mayros_search handles non-JSON 200 response", async () => { + globalThis.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.reject(new SyntaxError("Unexpected token")), + }); + + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const search = tools.find((t) => t.name === "mayros_search")!; + + const result = await search.execute("id", { text: "test" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("failed"); + }); +}); + +// ── memory-tools: input validation ─────────────────────────────────── + +describe("memory-tools input validation", () => { + let origFetch: typeof globalThis.fetch; + + beforeEach(() => { + origFetch = globalThis.fetch; + globalThis.fetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + }); + + afterEach(() => { + globalThis.fetch = origFetch; + }); + + it("mayros_remember handles string tags param gracefully", async () => { + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const remember = tools.find((t) => t.name === "mayros_remember")!; + + // tags as string instead of array — should not throw TypeError + const result = await remember.execute("id", { content: "test", tags: "not-an-array" }); + const text = (result.content[0] as { text: string }).text; + expect(text).toContain("Remembered"); + }); + + it("mayros_recall caps limit", async () => { + const mockFetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + globalThis.fetch = mockFetch; + + const mod = await import("./memory-tools.js"); + const tools = mod.createMemoryTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const recall = tools.find((t) => t.name === "mayros_recall")!; + + await recall.execute("id", { query: "test", limit: 999999 }); + const calledBody = JSON.parse((mockFetch.mock.calls[0][1] as { body: string }).body) as { + limit: number; + }; + expect(calledBody.limit).toBeLessThanOrEqual(100); }); }); @@ -181,7 +317,7 @@ describe("cortex-tools with Cortex unreachable", () => { globalThis.fetch = origFetch; }); - it("mayros_cortex_query returns friendly message", async () => { + it("mayros_cortex_query does not throw", async () => { const mod = await import("./cortex-tools.js"); const tools = mod.createCortexTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -191,10 +327,10 @@ describe("cortex-tools with Cortex unreachable", () => { const result = await query.execute("id", { subject: "test" }); const text = (result.content[0] as { text: string }).text; - expect(text).toContain("unavailable"); + expect(text).toContain("Cortex query unavailable"); }); - it("mayros_cortex_store returns friendly message", async () => { + it("mayros_cortex_store does not throw", async () => { const mod = await import("./cortex-tools.js"); const tools = mod.createCortexTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -204,11 +340,13 @@ describe("cortex-tools with Cortex unreachable", () => { const result = await store.execute("id", { subject: "a", predicate: "b", object: "c" }); const text = (result.content[0] as { text: string }).text; - expect(text).toContain("unavailable"); + expect(text).toContain("Cortex store unavailable"); }); it("mayros_cortex_query caps limit at 500", async () => { - globalThis.fetch = origFetch; // restore for this test + const mockFetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); + globalThis.fetch = mockFetch; + const mod = await import("./cortex-tools.js"); const tools = mod.createCortexTools({ cortexBaseUrl: "http://127.0.0.1:99999", @@ -216,32 +354,84 @@ describe("cortex-tools with Cortex unreachable", () => { }); const query = tools.find((t) => t.name === "mayros_cortex_query")!; - // Mock fetch to capture the URL + await query.execute("id", { limit: 1000000 }); + const calledUrl = mockFetch.mock.calls[0][0] as string; + expect(calledUrl).toContain("limit=500"); + }); + + it("mayros_cortex_query caps limit at boundary (501 → 500)", async () => { const mockFetch = vi.fn().mockRejectedValue(new TypeError("fetch failed")); globalThis.fetch = mockFetch; - await query.execute("id", { limit: 1000000 }); + const mod = await import("./cortex-tools.js"); + const tools = mod.createCortexTools({ + cortexBaseUrl: "http://127.0.0.1:99999", + namespace: "test", + }); + const query = tools.find((t) => t.name === "mayros_cortex_query")!; + + await query.execute("id", { limit: 501 }); const calledUrl = mockFetch.mock.calls[0][0] as string; expect(calledUrl).toContain("limit=500"); }); }); -// ── shutdown guard ───────────────────────────────────────────────── +// ── shutdown guard: promise-cached pattern ──────────────────────────── describe("shutdown guard", () => { - it("shuttingDown flag prevents double execution", async () => { + it("cached promise prevents double execution", async () => { let callCount = 0; - let shuttingDown = false; - const shutdown = async () => { - if (shuttingDown) return; - shuttingDown = true; - callCount++; - await new Promise((r) => setTimeout(r, 10)); + // Mirror the production pattern from serve-cli.ts + let shutdownPromise: Promise | null = null; + const shutdown = (): Promise => { + if (shutdownPromise) return shutdownPromise; + shutdownPromise = (async () => { + callCount++; + await new Promise((r) => setTimeout(r, 10)); + })(); + return shutdownPromise; }; - // Simulate double SIGINT - await Promise.all([shutdown(), shutdown()]); + // Simulate double SIGINT — both should share the same promise + const [p1, p2] = [shutdown(), shutdown()]; + expect(p1).toBe(p2); // same promise reference + await Promise.all([p1, p2]); expect(callCount).toBe(1); }); }); + +// ── transport: CORS default-deny ───────────────────────────────────── + +describe("transport CORS", () => { + it("does not set CORS headers when allowedOrigins is empty", async () => { + const { McpHttpTransport } = await import("./transport-http.js"); + + // Create a minimal dispatcher mock (cast to satisfy TS) + const dispatcher = { + handleMessage: vi.fn().mockResolvedValue('{"jsonrpc":"2.0","result":{}}'), + } as Parameters[0] extends void + ? never + : // eslint-disable-next-line @typescript-eslint/no-explicit-any + any; + + // Use a high random port to avoid conflicts + const port = 19500 + Math.floor(Math.random() * 1000); + const transport = new McpHttpTransport({ + dispatcher, + port, + host: "127.0.0.1", + allowedOrigins: [], // empty = default = deny + }); + + await transport.start(); + try { + const res = await fetch(`http://127.0.0.1:${port}/health`, { + headers: { Origin: "https://evil.com" }, + }); + expect(res.headers.get("access-control-allow-origin")).toBeNull(); + } finally { + await transport.stop(); + } + }); +}); diff --git a/extensions/mcp-server/memory-tools.ts b/extensions/mcp-server/memory-tools.ts index 747267c..00ece1a 100644 --- a/extensions/mcp-server/memory-tools.ts +++ b/extensions/mcp-server/memory-tools.ts @@ -49,8 +49,8 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { execute: async (_id: string, params: Record) => { const content = params.content as string; const category = (params.category as string) ?? "general"; - const tags = (params.tags as string[]) ?? []; - const importance = (params.importance as number) ?? 0.7; + const tags = Array.isArray(params.tags) ? (params.tags as string[]) : []; + const importance = Number(params.importance) || 0.7; // Store as RDF triple in Cortex (timestamp + random suffix to avoid collisions) const subject = `${namespace}:memory:${Date.now()}-${randomBytes(4).toString("hex")}`; @@ -124,9 +124,9 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { }), execute: async (_id: string, params: Record) => { const query = params.query as string | undefined; - const tags = params.tags as string[] | undefined; + const tags = Array.isArray(params.tags) ? (params.tags as string[]) : undefined; const category = params.category as string | undefined; - const limit = (params.limit as number) ?? 10; + const limit = Math.min(Number(params.limit) || 10, 100); // Query Ineru recall endpoint let recallRes: Response; @@ -196,7 +196,7 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { } } - const memories = (await recallRes.json()) as Array<{ + let memories: Array<{ id: string; entry_type: string; data: { content?: string }; @@ -205,6 +205,18 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { relevance: number; source: string; }>; + try { + memories = (await recallRes.json()) as typeof memories; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Memory recall failed: invalid response from Cortex.", + }, + ], + }; + } if (memories.length === 0) { return { @@ -216,7 +228,7 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { .map( (m, i) => `${i + 1}. [${m.entry_type}] ${m.data.content ?? JSON.stringify(m.data)}` + - (m.tags.length > 0 ? ` #${m.tags.join(" #")}` : "") + + (m.tags?.length > 0 ? ` #${m.tags.join(" #")}` : "") + ` (relevance: ${(m.relevance * 100).toFixed(0)}%, source: ${m.source})`, ) .join("\n"); @@ -244,7 +256,7 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { }), execute: async (_id: string, params: Record) => { const text = params.text as string; - const k = (params.k as number) ?? 5; + const k = Math.min(Number(params.k) || 5, 100); let recallRes: Response; try { @@ -275,12 +287,24 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { }; } - const results = (await recallRes.json()) as Array<{ + let results: Array<{ data: { content?: string }; relevance: number; entry_type: string; tags: string[]; }>; + try { + results = (await recallRes.json()) as typeof results; + } catch { + return { + content: [ + { + type: "text" as const, + text: "Vector search failed: invalid response from Cortex.", + }, + ], + }; + } if (results.length === 0) { return { @@ -292,7 +316,7 @@ export function createMemoryTools(deps: MemoryToolDeps): AdaptableTool[] { .map( (r, i) => `${i + 1}. [${(r.relevance * 100).toFixed(0)}%] ${r.data.content ?? JSON.stringify(r.data)}` + - (r.tags.length > 0 ? ` #${r.tags.join(" #")}` : ""), + (r.tags?.length > 0 ? ` #${r.tags.join(" #")}` : ""), ) .join("\n"); diff --git a/extensions/mcp-server/setup-claude.ts b/extensions/mcp-server/setup-claude.ts index 776291d..1be7d9f 100644 --- a/extensions/mcp-server/setup-claude.ts +++ b/extensions/mcp-server/setup-claude.ts @@ -11,7 +11,7 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync } from "node:fs"; import { join, dirname } from "node:path"; -import { execSync } from "node:child_process"; +import { execSync, execFileSync } from "node:child_process"; import { homedir, platform } from "node:os"; // ── Types ────────────────────────────────────────────────────────── @@ -53,12 +53,21 @@ function setupCode(opts: SetupClaudeOpts): void { return; } + if (!Number.isFinite(opts.port) || opts.port < 1 || opts.port > 65535) { + console.error(`Invalid port: ${String(opts.port)}. Must be 1-65535.`); + return; + } + try { if (transport === "stdio") { - execSync("claude mcp add mayros -- mayros serve --stdio", { stdio: "inherit" }); + execFileSync("claude", ["mcp", "add", "mayros", "--", "mayros", "serve", "--stdio"], { + stdio: "inherit", + }); } else { const url = `http://${opts.host}:${opts.port}/mcp`; - execSync(`claude mcp add mayros -s http --url ${JSON.stringify(url)}`, { stdio: "inherit" }); + execFileSync("claude", ["mcp", "add", "mayros", "-s", "http", "--url", url], { + stdio: "inherit", + }); } console.log("Mayros registered with Claude Code."); } catch { diff --git a/extensions/mcp-server/transport-http.ts b/extensions/mcp-server/transport-http.ts index 6fe1f81..3fe9f79 100644 --- a/extensions/mcp-server/transport-http.ts +++ b/extensions/mcp-server/transport-http.ts @@ -31,6 +31,8 @@ export type HttpTransportOptions = { // Transport // ============================================================================ +const MAX_SSE_SESSIONS = 50; + export class McpHttpTransport { private readonly dispatcher: McpProtocolDispatcher; private readonly port: number; @@ -160,6 +162,12 @@ export class McpHttpTransport { // MCP endpoint if (url === "/mcp" && method === "POST") { + const ct = req.headers["content-type"] ?? ""; + if (!ct.includes("application/json")) { + res.writeHead(415, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "Content-Type must be application/json" })); + return; + } await this.handleMcpPost(req, res); return; } @@ -178,6 +186,12 @@ export class McpHttpTransport { // Legacy SSE session POST endpoint if (url.startsWith("/mcp/session/") && method === "POST") { + const ct = req.headers["content-type"] ?? ""; + if (!ct.includes("application/json")) { + res.writeHead(415, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "Content-Type must be application/json" })); + return; + } await this.handleLegacySsePost(url, req, res); return; } @@ -218,6 +232,11 @@ export class McpHttpTransport { } private handleMcpSse(res: ServerResponse): void { + if (this.sseSessions.size >= MAX_SSE_SESSIONS) { + res.writeHead(503, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "Too many SSE sessions" })); + return; + } const sessionId = `mcp-sse-${randomUUID()}`; res.writeHead(200, { @@ -248,6 +267,11 @@ export class McpHttpTransport { } private handleLegacySse(res: ServerResponse): void { + if (this.sseSessions.size >= MAX_SSE_SESSIONS) { + res.writeHead(503, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "Too many SSE sessions" })); + return; + } const sessionId = randomUUID(); const postUrl = `/mcp/session/${sessionId}`; @@ -323,11 +347,15 @@ export class McpHttpTransport { } private setCorsHeaders(req: IncomingMessage, res: ServerResponse): void { - const origin = req.headers.origin ?? "*"; - const allowed = - this.allowedOrigins.length === 0 || - this.allowedOrigins.includes("*") || - this.allowedOrigins.includes(origin); + const origin = req.headers.origin; + + // No Origin header = same-origin or non-browser request — always allowed + if (!origin) return; + + // If no origins configured, deny cross-origin requests (secure default) + if (this.allowedOrigins.length === 0) return; + + const allowed = this.allowedOrigins.includes("*") || this.allowedOrigins.includes(origin); if (allowed) { res.setHeader("Access-Control-Allow-Origin", origin); diff --git a/extensions/memory-semantic/cortex-sidecar.ts b/extensions/memory-semantic/cortex-sidecar.ts index 8b81286..76c0f82 100644 --- a/extensions/memory-semantic/cortex-sidecar.ts +++ b/extensions/memory-semantic/cortex-sidecar.ts @@ -21,6 +21,11 @@ import { CortexClient } from "./cortex-client.js"; export type SidecarStatus = "stopped" | "starting" | "running" | "failed"; +export type SidecarOptions = { + /** When true, skip registering process signal handlers (caller manages shutdown). */ + skipSignalHandlers?: boolean; +}; + export class CortexSidecar { private process: ChildProcess | null = null; private _status: SidecarStatus = "stopped"; @@ -30,11 +35,16 @@ export class CortexSidecar { private stopping = false; private lockPath: string | null = null; private stderrBuffer: string[] = []; + private readonly skipSignalHandlers: boolean; private static readonly MAX_RESTARTS = 3; private static readonly STDERR_BUFFER_SIZE = 50; - constructor(private readonly config: CortexConfig) { + constructor( + private readonly config: CortexConfig, + options?: SidecarOptions, + ) { this.client = new CortexClient(config); + this.skipSignalHandlers = options?.skipSignalHandlers ?? false; } get status(): SidecarStatus { @@ -346,12 +356,15 @@ export class CortexSidecar { this.restartCount = 0; // reset for future crashes // Register process signal handlers for graceful sidecar shutdown - const cleanup = () => { - void this.stop(); - }; - for (const signal of ["SIGTERM", "SIGINT", "beforeExit"] as const) { - process.once(signal, cleanup); - this.signalHandlers.set(signal, cleanup); + // (skipped when caller manages shutdown, e.g. serve-cli) + if (!this.skipSignalHandlers) { + const cleanup = () => { + void this.stop(); + }; + for (const signal of ["SIGTERM", "SIGINT", "beforeExit"] as const) { + process.once(signal, cleanup); + this.signalHandlers.set(signal, cleanup); + } } } else { this._status = "failed"; diff --git a/src/cli/serve-cli.ts b/src/cli/serve-cli.ts index 1c2abbf..1dee5c4 100644 --- a/src/cli/serve-cli.ts +++ b/src/cli/serve-cli.ts @@ -38,19 +38,23 @@ export function registerServeCli(program: Command): void { try { const { CortexSidecar } = (await import("../../extensions/memory-semantic/cortex-sidecar.js")) as { - CortexSidecar: new (cfg: unknown) => { + CortexSidecar: new ( + cfg: unknown, + opts?: { skipSignalHandlers?: boolean }, + ) => { start: () => Promise; stop: () => Promise; }; }; - const instance = new CortexSidecar(config.cortex); + // skipSignalHandlers: serve-cli manages shutdown, sidecar must not steal SIGINT/SIGTERM + const instance = new CortexSidecar(config.cortex, { skipSignalHandlers: true }); const started = await instance.start(); if (started) { sidecar = instance; process.stderr.write("Cortex sidecar started\n"); } - } catch { - // Cortex sidecar not available + } catch (err: unknown) { + process.stderr.write(`WARN: Cortex sidecar not available: ${String(err)}\n`); } // Load dedicated MCP tools @@ -94,8 +98,8 @@ export function registerServeCli(program: Command): void { identity: a.identity, origin: a.origin, })); - } catch { - // Agent discovery not available + } catch (err: unknown) { + process.stderr.write(`WARN: Agent discovery not available: ${String(err)}\n`); } const server = new McpServer({ @@ -129,21 +133,24 @@ export function registerServeCli(program: Command): void { await server.start(); - // Shutdown handler: stop server + sidecar (both must run even if one fails) - let shuttingDown = false; - const shutdown = async () => { - if (shuttingDown) return; - shuttingDown = true; - try { - await server.stop(); - } catch (err: unknown) { - process.stderr.write(`ERROR stopping server: ${String(err)}\n`); - } - try { - if (sidecar) await sidecar.stop(); - } catch (err: unknown) { - process.stderr.write(`ERROR stopping sidecar: ${String(err)}\n`); - } + // Shutdown handler: stop server + sidecar (both must run even if one fails). + // Cache the promise so concurrent signals reuse the same shutdown sequence. + let shutdownPromise: Promise | null = null; + const shutdown = (): Promise => { + if (shutdownPromise) return shutdownPromise; + shutdownPromise = (async () => { + try { + await server.stop(); + } catch (err: unknown) { + process.stderr.write(`ERROR stopping server: ${String(err)}\n`); + } + try { + if (sidecar) await sidecar.stop(); + } catch (err: unknown) { + process.stderr.write(`ERROR stopping sidecar: ${String(err)}\n`); + } + })(); + return shutdownPromise; }; if (transport !== "stdio") { @@ -161,9 +168,17 @@ export function registerServeCli(program: Command): void { process.once("SIGINT", onSignal); process.once("SIGTERM", onSignal); }); + process.exit(0); } else { - process.once("SIGINT", () => void shutdown()); - process.once("SIGTERM", () => void shutdown()); + const onSignal = () => { + void shutdown() + .catch((e: unknown) => { + process.stderr.write(`ERROR during shutdown: ${String(e)}\n`); + }) + .finally(() => process.exit(0)); + }; + process.once("SIGINT", onSignal); + process.once("SIGTERM", onSignal); } }); }