diff --git a/.env.example b/.env.example index d4a49662..9019f3dc 100644 --- a/.env.example +++ b/.env.example @@ -18,6 +18,10 @@ DEVSPACE_TOOL_MODE=full # off | changes | full. Defaults to changes. # changes creates one aggregate review widget via review_changes instead of per-tool iframes. DEVSPACE_WIDGETS=changes +# Optional cooperative-client guard based on initialize.clientInfo. +# Start with a denylist because some allowed clients may omit clientInfo. +# DEVSPACE_CLIENT_ACCESS_MODE=enforce +# DEVSPACE_DENIED_CLIENTS=codex DEVSPACE_LOG_LEVEL=info DEVSPACE_LOG_FORMAT=json DEVSPACE_LOG_REQUESTS=1 diff --git a/package.json b/package.json index 25f21059..3f4080d8 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "dev": "node scripts/dev-server.mjs", "postinstall": "node scripts/fix-node-pty-permissions.mjs", "start": "node dist/cli.js serve", - "test": "tsx src/config.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", + "test": "tsx src/client-access.test.ts && tsx src/client-access-http.test.ts && tsx src/config.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", "typecheck": "tsc -p tsconfig.json --noEmit" }, "keywords": [], diff --git a/src/client-access-http.test.ts b/src/client-access-http.test.ts new file mode 100644 index 00000000..30e37dd2 --- /dev/null +++ b/src/client-access-http.test.ts @@ -0,0 +1,81 @@ +import assert from "node:assert/strict"; +import { once } from "node:events"; +import type { AddressInfo } from "node:net"; +import { isInitializeRequest } from "@modelcontextprotocol/sdk/types.js"; +import express from "express"; +import type { ClientAccessConfig } from "./client-access.js"; +import { handleClientAccessInitialize } from "./server.js"; + +const config: ClientAccessConfig = { + mode: "enforce", + deniedClients: ["codex"], +}; + +const app = express(); +app.use(express.json()); + +let continuedRequests = 0; +app.post("/mcp", (req, res) => { + assert.equal(isInitializeRequest(req.body), true); + const { clientAccess } = handleClientAccessInitialize(res, req.body, config); + if (!clientAccess.allowed) return; + + continuedRequests += 1; + res.sendStatus(204); +}); + +const server = app.listen(0, "127.0.0.1"); +await once(server, "listening"); + +try { + const address = server.address() as AddressInfo; + const endpoint = `http://127.0.0.1:${address.port}/mcp`; + + const deniedResponse = await fetch(endpoint, { + method: "POST", + headers: { "content-type": "application/json", connection: "close" }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 0, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "codex-mcp-client", version: "0.145.0" }, + }, + }), + }); + + assert.equal(deniedResponse.status, 403); + assert.deepEqual(await deniedResponse.json(), { + jsonrpc: "2.0", + error: { code: -32003, message: "MCP client not allowed" }, + id: 0, + }); + assert.equal(continuedRequests, 0); + + const allowedResponse = await fetch(endpoint, { + method: "POST", + headers: { "content-type": "application/json", connection: "close" }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "", version: "" }, + }, + }), + }); + + assert.equal(allowedResponse.status, 204); + assert.equal(continuedRequests, 1); +} finally { + await new Promise((resolve, reject) => { + server.close((error) => { + if (error) reject(error); + else resolve(); + }); + }); +} diff --git a/src/client-access.test.ts b/src/client-access.test.ts new file mode 100644 index 00000000..8551c8e1 --- /dev/null +++ b/src/client-access.test.ts @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import { + evaluateClientAccess, + extractDeclaredClient, + type ClientAccessConfig, +} from "./client-access.js"; + +const enforceCodexDenylist: ClientAccessConfig = { + mode: "enforce", + deniedClients: ["codex"], +}; + +assert.deepEqual( + extractDeclaredClient({ + jsonrpc: "2.0", + id: 0, + method: "initialize", + params: { + clientInfo: { + name: "codex-mcp-client", + title: "Codex", + version: "0.108.0-alpha.12", + }, + }, + }), + { + name: "codex-mcp-client", + title: "Codex", + version: "0.108.0-alpha.12", + identities: ["codex-mcp-client", "codex"], + }, +); + +assert.deepEqual( + evaluateClientAccess(enforceCodexDenylist, { + name: "codex-mcp-client", + title: "Codex", + version: "0.145.0", + identities: ["codex-mcp-client", "codex"], + }), + { + allowed: false, + reason: "denied_client", + matchedClient: "codex", + }, +); + +assert.deepEqual( + evaluateClientAccess(enforceCodexDenylist, undefined), + { + allowed: true, + reason: "allowed", + }, +); + +assert.equal(extractDeclaredClient({ method: "tools/list", params: {} }), undefined); +assert.equal( + extractDeclaredClient({ method: "initialize", params: { clientInfo: { name: "" } } }), + undefined, +); diff --git a/src/client-access.ts b/src/client-access.ts new file mode 100644 index 00000000..0be6c629 --- /dev/null +++ b/src/client-access.ts @@ -0,0 +1,109 @@ +export type ClientAccessMode = "off" | "enforce"; + +export interface ClientAccessConfig { + mode: ClientAccessMode; + deniedClients: string[]; +} + +export interface DeclaredClient { + name: string; + title?: string; + version?: string; + identities: string[]; +} + +export interface ClientAccessDecision { + allowed: boolean; + reason: "disabled" | "allowed" | "denied_client"; + matchedClient?: string; +} + +export type JsonRpcId = string | number | null; + +export function extractDeclaredClient(body: unknown): DeclaredClient | undefined { + if (!isRecord(body) || body.method !== "initialize") return undefined; + const params = body.params; + if (!isRecord(params)) return undefined; + const clientInfo = params.clientInfo; + if (!isRecord(clientInfo)) return undefined; + + const name = sanitizeText(clientInfo.name); + if (!name) return undefined; + const title = sanitizeText(clientInfo.title); + const version = sanitizeText(clientInfo.version); + const identities = clientIdentities(name, title); + + return { + name, + ...(title ? { title } : {}), + ...(version ? { version } : {}), + identities, + }; +} + +export function extractJsonRpcId(body: unknown): JsonRpcId { + if (!isRecord(body)) return null; + const id = body.id; + return typeof id === "string" || typeof id === "number" ? id : null; +} + +export function evaluateClientAccess( + config: ClientAccessConfig, + client: DeclaredClient | undefined, +): ClientAccessDecision { + if (config.mode === "off") { + return { allowed: true, reason: "disabled" }; + } + + const identities = new Set(client?.identities ?? []); + const deniedClient = normalizedPolicyEntries(config.deniedClients).find((entry) => + identities.has(entry), + ); + if (deniedClient) { + return { + allowed: false, + reason: "denied_client", + matchedClient: deniedClient, + }; + } + + return { + allowed: true, + reason: "allowed", + }; +} + +function clientIdentities(name: string, title?: string): string[] { + const identities = new Set(); + const normalizedName = normalizeClientName(name); + if (normalizedName) identities.add(normalizedName); + + const combined = `${name} ${title ?? ""}`.toLowerCase(); + if (/\bcodex\b/.test(combined) || normalizedName.includes("codex")) identities.add("codex"); + if (combined.includes("chatgpt") || normalizedName.includes("chatgpt")) identities.add("chatgpt"); + + return [...identities]; +} + +function normalizedPolicyEntries(entries: string[]): string[] { + return [...new Set(entries.map(normalizeClientName).filter(Boolean))]; +} + +function normalizeClientName(value: string): string { + return value + .trim() + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); +} + +function sanitizeText(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const normalized = value.replace(/[\r\n\t]+/g, " ").trim(); + if (!normalized) return undefined; + return normalized.slice(0, 160); +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/config.test.ts b/src/config.test.ts index 0b4f99a8..5dd9538c 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -26,6 +26,21 @@ assert.equal(loadConfig(baseEnv).skillsEnabled, true); assert.equal(loadConfig(baseEnv).devspaceSkillsDir, join(emptyConfigDir, "skills")); assert.equal(loadConfig(baseEnv).devspaceAgentsDir, join(emptyConfigDir, "agents")); assert.equal(loadConfig(baseEnv).subagents, false); +assert.deepEqual(loadConfig(baseEnv).clientAccess, { + mode: "off", + deniedClients: [], +}); +assert.deepEqual( + loadConfig({ + ...baseEnv, + DEVSPACE_CLIENT_ACCESS_MODE: "enforce", + DEVSPACE_DENIED_CLIENTS: "codex", + }).clientAccess, + { + mode: "enforce", + deniedClients: ["codex"], + }, +); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_SKILLS: "0" }).skillsEnabled, false); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_SKILLS: "1" }).skillsEnabled, true); assert.equal( @@ -60,6 +75,14 @@ assert.throws( () => loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "invalid" }), /Invalid DEVSPACE_TOOL_MODE: invalid/, ); +assert.throws( + () => loadConfig({ ...baseEnv, DEVSPACE_CLIENT_ACCESS_MODE: "audit" }), + /Invalid DEVSPACE_CLIENT_ACCESS_MODE: audit/, +); +assert.throws( + () => loadConfig({ ...baseEnv, DEVSPACE_CLIENT_ACCESS_MODE: "" }), + /Invalid DEVSPACE_CLIENT_ACCESS_MODE:/, +); assert.deepEqual(loadConfig(baseEnv).logging, { level: "info", @@ -163,6 +186,10 @@ writeFileSync( allowedRoots: [process.cwd()], publicBaseUrl: "https://devspace.example.com", subagents: true, + clientAccess: { + mode: "enforce", + deniedClients: ["codex"], + }, }), ); writeFileSync( @@ -177,6 +204,10 @@ assert.equal(fileConfig.port, 8787); assert.equal(fileConfig.oauth.ownerToken, "persisted-owner-token-long-enough"); assert.equal(fileConfig.publicBaseUrl, "https://devspace.example.com"); assert.equal(fileConfig.subagents, true); +assert.deepEqual(fileConfig.clientAccess, { + mode: "enforce", + deniedClients: ["codex"], +}); assert.deepEqual(fileConfig.allowedHosts, [ "localhost", "127.0.0.1", diff --git a/src/config.ts b/src/config.ts index 4fc1bcbb..613fb29e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,6 +1,7 @@ import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { expandHomePath } from "./roots.js"; +import type { ClientAccessConfig, ClientAccessMode } from "./client-access.js"; import type { LoggingConfig, LogFormat, LogLevel } from "./logger.js"; import type { OAuthConfig } from "./oauth-provider.js"; import { devspaceAgentsDir, devspaceSkillsDir, loadDevspaceFiles } from "./user-config.js"; @@ -28,6 +29,7 @@ export interface ServerConfig { subagents: boolean; agentDir: string; logging: LoggingConfig; + clientAccess: ClientAccessConfig; } function parsePort(value: string | number | undefined): number { @@ -124,6 +126,31 @@ function parseStringList(value: string | undefined, fallback: string[]): string[ return entries && entries.length > 0 ? entries : fallback; } +function parseClientAccessMode(value: string | undefined): ClientAccessMode { + if (value === undefined || value === "off") return "off"; + if (value === "enforce") return value; + throw new Error(`Invalid DEVSPACE_CLIENT_ACCESS_MODE: ${value}`); +} + +function parseClientAccessConfig( + env: NodeJS.ProcessEnv, + fileConfig: + | { + mode?: ClientAccessMode; + deniedClients?: string[]; + } + | undefined, +): ClientAccessConfig { + const mode = parseClientAccessMode(env.DEVSPACE_CLIENT_ACCESS_MODE ?? fileConfig?.mode); + return { + mode, + deniedClients: + env.DEVSPACE_DENIED_CLIENTS !== undefined + ? parseStringList(env.DEVSPACE_DENIED_CLIENTS, []) + : [...(fileConfig?.deniedClients ?? [])], + }; +} + function parsePositiveInteger(value: string | undefined, fallback: number, name: string): number { if (!value) return fallback; @@ -236,6 +263,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig { : parseBoolean(env.DEVSPACE_SUBAGENTS), agentDir: resolve(expandHomePath(env.DEVSPACE_AGENT_DIR ?? files.config.agentDir ?? defaultAgentDir())), logging: parseLoggingConfig(env), + clientAccess: parseClientAccessConfig(env, files.config.clientAccess), }; } diff --git a/src/server.ts b/src/server.ts index 7dd9629e..f80b5a41 100644 --- a/src/server.ts +++ b/src/server.ts @@ -18,6 +18,15 @@ import express from "express"; import type { Request, Response } from "express"; import * as z from "zod/v4"; import { applyPatch } from "./apply-patch.js"; +import { + evaluateClientAccess, + extractDeclaredClient, + extractJsonRpcId, + type ClientAccessConfig, + type ClientAccessDecision, + type DeclaredClient, + type JsonRpcId, +} from "./client-access.js"; import { loadConfig, type ServerConfig, type WidgetMode } from "./config.js"; import { logEvent, @@ -282,14 +291,37 @@ function sendJsonRpcError( status: number, code: number, message: string, + id: JsonRpcId = null, ): void { res.status(status).json({ jsonrpc: "2.0", error: { code, message }, - id: null, + id, }); } +export function handleClientAccessInitialize( + res: Response, + body: unknown, + config: ClientAccessConfig, +): { + declaredClient: DeclaredClient | undefined; + clientAccess: ClientAccessDecision; +} { + const declaredClient = extractDeclaredClient(body); + const clientAccess = evaluateClientAccess(config, declaredClient); + if (!clientAccess.allowed) { + sendJsonRpcError( + res, + 403, + -32003, + "MCP client not allowed", + extractJsonRpcId(body), + ); + } + return { declaredClient, clientAccess }; +} + function requestLogFields(req: Request, config: ServerConfig): Record { return { ip: requestIp(req, config.logging.trustProxy), @@ -1735,6 +1767,38 @@ export function createServer(config = loadConfig()): RunningServer { return; } + if (initializeRequest) { + const { declaredClient, clientAccess } = handleClientAccessInitialize( + res, + req.body, + config.clientAccess, + ); + const clientFields = { + requestId, + clientName: declaredClient?.name, + clientTitle: declaredClient?.title, + clientVersion: declaredClient?.version, + clientIdentities: declaredClient?.identities ?? [], + oauthClientIdPrefix: req.auth.clientId?.slice(0, 8), + accessMode: config.clientAccess.mode, + accessReason: clientAccess.reason, + matchedClient: clientAccess.matchedClient, + ...requestLogFields(req, config), + }; + + logEvent( + config.logging, + clientAccess.reason === "allowed" || clientAccess.reason === "disabled" ? "info" : "warn", + "mcp_client_observed", + clientFields, + ); + + if (!clientAccess.allowed) { + logEvent(config.logging, "warn", "mcp_client_denied", clientFields); + return; + } + } + logEvent(config.logging, "debug", "mcp_request", { requestId, method: req.method, @@ -1839,6 +1903,12 @@ if (await isMainModule()) { console.log(`request logging: ${config.logging.requests ? "enabled" : "disabled"}`); console.log(`asset logging: ${config.logging.assets ? "enabled" : "disabled"}`); console.log(`trust proxy: ${config.logging.trustProxy ? "enabled" : "disabled"}`); + console.log( + `client access: ${config.clientAccess.mode}` + + (config.clientAccess.deniedClients.length > 0 + ? ` deny=${config.clientAccess.deniedClients.join(",")}` + : ""), + ); if (config.subagents) { console.log(`subagent providers: ${formatLocalAgentProviderAvailabilitySummary(localAgentProviders)}`); } diff --git a/src/user-config.ts b/src/user-config.ts index c8da90b5..79b7dea9 100644 --- a/src/user-config.ts +++ b/src/user-config.ts @@ -19,6 +19,10 @@ export interface DevspaceUserConfig { worktreeRoot?: string; agentDir?: string; subagents?: boolean; + clientAccess?: { + mode?: "off" | "enforce"; + deniedClients?: string[]; + }; } export interface DevspaceAuthConfig {