From 829c7dba474a8b9751c9a001eadf649b4aac15bf Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Sat, 25 Jul 2026 20:35:54 +0300 Subject: [PATCH] feat(mcp): require an ETAPI token on /mcp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP endpoint was unauthenticated, relying on a loopback source-IP check plus the SDK's DNS-rebinding Host allow-list. That had three problems: - Loopback is not a credential. Any local process (a package postinstall script, another user on a shared box) could read and rewrite every note. - The Host allow-list was the only thing between a malicious page and the database, so a single parsing bug or a proxy that rewrites Host lost everything. - Because req.ip honours trust proxy, a misconfigured trustedReverseProxy could forge X-Forwarded-For: 127.0.0.1 and walk straight in. It also made the endpoint unusable in the deployment people actually have: under Docker, host traffic arrives from the bridge gateway rather than 127.0.0.1, so port mapping alone never satisfied the check (#10646). Require an ETAPI token on every request instead, mirroring what the web clipper already does on the server build. This makes the rebinding protection redundant rather than merely inconvenient — rebinding borrows the victim browser's network position, and a bearer token is not ambient authority, since cross-origin JS cannot set Authorization without a CORS preflight we never approve. The Host allow-list is therefore dropped; it would only have blocked legitimate remote clients, whose Host is their own domain. Only the Authorization header authenticates, never a session cookie, which would make /mcp CSRF-able with full tool access. The 401 deliberately omits WWW-Authenticate: in MCP that header triggers OAuth discovery (RFC 9728) and must point at protected-resource metadata, so advertising one would send spec-compliant clients into a flow Trilium cannot serve. Adds a rate limiter mirroring the login one (15 min / 10 / skip successful) since token guessing is otherwise unbounded once reachable off-loopback. BREAKING CHANGE: existing MCP clients must send an ETAPI token. MCP is off by default, so this affects only users who explicitly enabled it. Co-Authored-By: Claude Opus 5 (1M context) --- .mcp.json | 5 +- CLAUDE.md | 1 + apps/server/src/app.ts | 5 +- apps/server/src/routes/mcp.spec.ts | 22 ++++ apps/server/src/routes/mcp.ts | 123 +++++++++++------- apps/server/src/routes/transport.spec.ts | 72 +++++++--- .../src/services/desktop_network_gate.ts | 6 +- 7 files changed, 163 insertions(+), 71 deletions(-) create mode 100644 apps/server/src/routes/mcp.spec.ts diff --git a/.mcp.json b/.mcp.json index 9fd17b8d67..4d457584a5 100644 --- a/.mcp.json +++ b/.mcp.json @@ -2,7 +2,10 @@ "mcpServers": { "trilium": { "type": "http", - "url": "http://localhost:8080/mcp" + "url": "http://localhost:8080/mcp", + "headers": { + "Authorization": "Bearer ${TRILIUM_ETAPI_TOKEN}" + } } } } diff --git a/CLAUDE.md b/CLAUDE.md index b4c2247edb..6c14ee2f50 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -341,6 +341,7 @@ Tools are defined using `defineTools()` in `apps/server/src/services/llm/tools/` ## MCP Server - Trilium exposes an MCP (Model Context Protocol) server at `http://localhost:8080/mcp`, configured in `.mcp.json` - The MCP server is **only available when the Trilium server is running** (`pnpm run server:start`) +- It requires an ETAPI token on every request — create one in Options → ETAPI and export it as `TRILIUM_ETAPI_TOKEN` before starting Claude Code (`.mcp.json` reads that variable). Without it the endpoint answers `401` - It provides tools for reading, searching, and modifying notes directly from the AI assistant - Use it to interact with actual note data when developing or debugging note-related features diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index c6ee5c457d..c93c6361af 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -97,8 +97,9 @@ export default async function buildApp() { app.use(express.urlencoded({ extended: false })); app.use(cookieParser()); - // MCP is registered before session/auth middleware — it uses its own - // localhost-only guard and does not require Trilium authentication. + // MCP is registered before session/auth middleware — it authenticates with its own + // guard (an ETAPI token, always required) and never uses the Trilium session, which + // would make the endpoint CSRF-able. mcpRoutes.register(app); // Desktop only: gate web access (SPA/login, /share, /api, static assets) behind diff --git a/apps/server/src/routes/mcp.spec.ts b/apps/server/src/routes/mcp.spec.ts new file mode 100644 index 0000000000..85bfe8f1f8 --- /dev/null +++ b/apps/server/src/routes/mcp.spec.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; + +import { resolveMcpAccess } from "./mcp.js"; + +describe("resolveMcpAccess", () => { + it("denies everything with 403 while MCP is disabled, token or not", () => { + for (const hasValidToken of [true, false]) { + expect(resolveMcpAccess({ mcpEnabled: false, hasValidToken })) + .toEqual({ type: "deny", status: 403, error: expect.stringContaining("disabled") }); + } + }); + + it("allows a valid token", () => { + expect(resolveMcpAccess({ mcpEnabled: true, hasValidToken: true })).toEqual({ type: "allow" }); + }); + + it("rejects a missing or invalid token with 401 and tells the user where to get one", () => { + const decision = resolveMcpAccess({ mcpEnabled: true, hasValidToken: false }); + expect(decision).toEqual({ type: "deny", status: 401, error: expect.stringContaining("ETAPI token") }); + expect(decision.type === "deny" && decision.error).toContain("Options > ETAPI"); + }); +}); diff --git a/apps/server/src/routes/mcp.ts b/apps/server/src/routes/mcp.ts index af86ff99fa..bb04356ea6 100644 --- a/apps/server/src/routes/mcp.ts +++ b/apps/server/src/routes/mcp.ts @@ -1,61 +1,89 @@ /** * MCP (Model Context Protocol) HTTP route handler. * - * Mounts the Streamable HTTP transport at `/mcp` with a localhost-only guard. - * No authentication is required — access is restricted to loopback addresses, - * with DNS-rebinding protection (Host-header allow-list) layered on top so a - * rebound attacker domain can't drive the transport from the victim's browser. + * Mounts the Streamable HTTP transport at `/mcp`. Every request must carry an ETAPI token + * in the `Authorization` header — there is no unauthenticated path, on any address. + * + * See {@link resolveMcpAccess} for why that replaces the previous loopback + DNS-rebinding + * arrangement outright rather than layering on top of it. */ import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; -import { options as optionService } from "@triliumnext/core"; +import { getLog, options as optionService } from "@triliumnext/core"; import type express from "express"; +import rateLimit from "express-rate-limit"; -import { getLog } from "@triliumnext/core"; +import etapiTokenService from "../services/etapi_tokens.js"; import { createMcpServer } from "../services/mcp/mcp_server.js"; -import port from "../services/port.js"; + +export function register(app: express.Application) { + // Token guessing is unbounded once the endpoint is reachable off-loopback, so cap + // failures per IP. `skipSuccessfulRequests` keeps a working client from ever counting + // against the limit — only 4xx/5xx responses do. Mirrors the login limiter. + const mcpRateLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 10, + skipSuccessfulRequests: true + }); + + app.post("/mcp", mcpRateLimiter, mcpGuard, handleMcpRequest); + app.get("/mcp", mcpRateLimiter, mcpGuard, handleMcpRequest); + app.delete("/mcp", mcpRateLimiter, mcpGuard, handleMcpRequest); + + getLog().info("MCP server registered at /mcp (ETAPI token required)"); +} + +export type McpAccessDecision = + | { type: "allow" } + | { type: "deny"; status: number; error: string }; /** - * Host header values a legitimate (loopback) MCP client may target. The SDK's - * DNS-rebinding protection rejects any other Host before a tool can run, so a - * rebound attacker domain pointed at 127.0.0.1 — which the browser sends as its - * own Host — cannot reach the transport even though it originates from loopback. + * Pure access decision for {@link mcpGuard}, extracted so the branching is unit-testable. * - * Both the port-qualified form (`localhost:8080`) and the bare form (`localhost`) - * are listed: a client reaching the server on a standard port (80/443) omits the - * port from the Host header. Bare loopback names still only ever resolve to - * loopback, so allowing them does not widen the DNS-rebinding surface. + * An ETAPI token is required unconditionally — loopback is *not* a credential. Any local + * process (a package postinstall script, another user on a shared box) can reach the + * loopback listener, so the previous "no token needed from 127.0.0.1" rule handed full + * read/write access over every note to anything running on the machine. It also meant a + * misconfigured `trustedReverseProxy` could forge `X-Forwarded-For: 127.0.0.1` and walk + * straight in. This mirrors what the web clipper already does on the server build + * (see routes.ts — `clipperMiddleware`). + * + * Requiring the token also makes the SDK's DNS-rebinding protection redundant rather than + * merely inconvenient: rebinding attacks work by borrowing the victim browser's *network + * position*, and a bearer token is not ambient authority — cross-origin JS cannot attach an + * `Authorization` header without a CORS preflight we never approve. See + * {@link handleMcpRequest} for why the Host allow-list is therefore gone. + * + * Note the 401 deliberately carries no `WWW-Authenticate` header. In MCP that header is + * the OAuth-discovery trigger (RFC 9728) and must point at protected-resource metadata; + * Trilium has no authorization server, so advertising one would send spec-compliant + * clients into a discovery flow that cannot succeed. */ -const MCP_ALLOWED_HOSTS = [ - "localhost", - `localhost:${port}`, - "127.0.0.1", - `127.0.0.1:${port}`, - "[::1]", - `[::1]:${port}` -]; +export function resolveMcpAccess(opts: { + mcpEnabled: boolean; + hasValidToken: boolean; +}): McpAccessDecision { + if (!opts.mcpEnabled) { + return { type: "deny", status: 403, error: "MCP server is disabled. Enable it in Options > AI / LLM." }; + } -function isLoopback(addr: string | undefined): boolean { - if (!addr) return false; - // IPv6 loopback - if (addr === "::1") return true; - // IPv4 loopback (127.0.0.0/8) - if (addr.startsWith("127.")) return true; - // IPv4-mapped IPv6 loopback - if (addr.startsWith("::ffff:127.")) return true; - return false; + if (!opts.hasValidToken) { + return { type: "deny", status: 401, error: "MCP requires an ETAPI token. Create one in Options > ETAPI and send it as 'Authorization: Bearer '." }; + } + + return { type: "allow" }; } function mcpGuard(req: express.Request, res: express.Response, next: express.NextFunction) { - if (optionService.getOptionOrNull("mcpEnabled") !== "true") { - res.status(403).json({ error: "MCP server is disabled. Enable it in Options > AI / LLM." }); - return; - } + const decision = resolveMcpAccess({ + mcpEnabled: optionService.getOptionOrNull("mcpEnabled") === "true", + // Only the Authorization header authenticates — never a session cookie. A cookie would + // make `/mcp` CSRF-able by any page the logged-in user visits, with full tool access. + hasValidToken: etapiTokenService.isValidAuthHeader(req.headers.authorization) + }); - // Use req.ip which respects trust proxy settings, falling back to socket address - const clientIp = req.ip || req.socket.remoteAddress; - if (!isLoopback(clientIp)) { - res.status(403).json({ error: "MCP is only available from localhost" }); + if (decision.type === "deny") { + res.status(decision.status).json({ error: decision.error }); return; } @@ -67,8 +95,11 @@ async function handleMcpRequest(req: express.Request, res: express.Response) { const server = createMcpServer(); const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined, // stateless - enableDnsRebindingProtection: true, - allowedHosts: MCP_ALLOWED_HOSTS + // Off because the guard above has already required a bearer token, which the + // browser in a rebinding attack cannot supply. Keeping a Host allow-list here + // would block legitimate remote clients — whose Host is their own domain — + // without denying an attacker anything the token doesn't already deny. + enableDnsRebindingProtection: false }); res.on("close", () => { @@ -86,12 +117,4 @@ async function handleMcpRequest(req: express.Request, res: express.Response) { } } -export function register(app: express.Application) { - app.post("/mcp", mcpGuard, handleMcpRequest); - app.get("/mcp", mcpGuard, handleMcpRequest); - app.delete("/mcp", mcpGuard, handleMcpRequest); - - getLog().info("MCP server registered at /mcp (localhost only)"); -} - export default { register }; diff --git a/apps/server/src/routes/transport.spec.ts b/apps/server/src/routes/transport.spec.ts index 676db2bc53..328445a951 100644 --- a/apps/server/src/routes/transport.spec.ts +++ b/apps/server/src/routes/transport.spec.ts @@ -4,8 +4,16 @@ import supertest from "supertest"; import { beforeAll, describe, expect, it, vi } from "vitest"; import { type ApiTestContext, bootLoggedInApp, createTextNote } from "../../spec/support/internal_api.js"; +import etapiTokenService from "../services/etapi_tokens.js"; import port from "../services/port.js"; +const initializeRequest = { + jsonrpc: "2.0", + method: "initialize", + id: 1, + params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "spec", version: "1" } } +}; + let ctx: ApiTestContext; let app: Application; @@ -129,35 +137,67 @@ describe("Route transport & middleware", () => { expect(res.body.error).toContain("disabled"); }); - it("reaches the MCP transport over loopback with a valid Host once enabled", async () => { + it("rejects an untokened request with 401 even over loopback, with no OAuth challenge", async () => { cls.init(() => optionService.setOption("mcpEnabled", "true")); - // supertest connects over loopback so the guard passes; a Host matching - // the configured port clears DNS-rebinding protection and the request - // is handed to the streamable transport (any status is fine — we only - // need the handler to execute past header validation). + // supertest dials loopback, which used to be sufficient on its own. It no longer + // is: any local process can reach the listener, so a token is always required. const res = await supertest(app) .post("/mcp") .set("Host", `localhost:${port}`) .set("Content-Type", "application/json") .set("Accept", "application/json, text/event-stream") - .send({ jsonrpc: "2.0", method: "initialize", id: 1, params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "spec", version: "1" } } }); - expect(res.status).toBeGreaterThanOrEqual(200); - expect(JSON.stringify(res.body)).not.toContain("Invalid Host header"); + .send(initializeRequest) + .expect(401); + expect(res.body.error).toContain("ETAPI token"); + // A WWW-Authenticate header would send spec-compliant MCP clients into an + // OAuth discovery flow Trilium cannot serve. + expect(res.headers["www-authenticate"]).toBeUndefined(); }); - it("rejects a forged Host header (DNS rebinding) with 403", async () => { + it("rejects a bogus token with 401", async () => { cls.init(() => optionService.setOption("mcpEnabled", "true")); - // A DNS-rebinding attacker reaches the loopback listener through the - // victim's browser (so the IP guard passes) but carries an attacker- - // controlled Host. It must be rejected before any MCP tool can run. const res = await supertest(app) .post("/mcp") - .set("Host", "attacker.example.com") + .set("Host", `localhost:${port}`) + .set("Authorization", "Bearer not_a_real_token") .set("Content-Type", "application/json") .set("Accept", "application/json, text/event-stream") - .send({ jsonrpc: "2.0", method: "initialize", id: 1, params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "spec", version: "1" } } }) - .expect(403); - expect(res.body?.error?.message ?? "").toContain("Invalid Host header"); + .send(initializeRequest) + .expect(401); + expect(res.body.error).toContain("ETAPI token"); + }); + + it("serves a token-authenticated caller, including on a foreign Host", async () => { + cls.init(() => optionService.setOption("mcpEnabled", "true")); + const { authToken } = cls.init(() => etapiTokenService.createToken("mcp transport spec")); + + // A remote client's Host is its own domain and could never satisfy a loopback + // allow-list; the token is what authorises it, so no Host check applies. + for (const host of [`localhost:${port}`, "trilium.example.com"]) { + const res = await supertest(app) + .post("/mcp") + .set("Host", host) + .set("Authorization", `Bearer ${authToken}`) + .set("Content-Type", "application/json") + .set("Accept", "application/json, text/event-stream") + .send(initializeRequest); + expect(res.status).toBe(200); + expect(JSON.stringify(res.body)).not.toContain("Invalid Host header"); + } + }); + + it("accepts the ETAPI Basic-auth form as well as Bearer", async () => { + cls.init(() => optionService.setOption("mcpEnabled", "true")); + const { authToken } = cls.init(() => etapiTokenService.createToken("mcp basic spec")); + // isValidAuthHeader also parses `Basic etapi:`, which some clients emit + // when given a username/password pair rather than a raw header. + const res = await supertest(app) + .post("/mcp") + .auth("etapi", authToken, { type: "basic" }) + .set("Content-Type", "application/json") + .set("Accept", "application/json, text/event-stream") + .send(initializeRequest); + expect(res.status).toBe(200); }); }); }); diff --git a/apps/server/src/services/desktop_network_gate.ts b/apps/server/src/services/desktop_network_gate.ts index 9deaf668c4..e6db094aee 100644 --- a/apps/server/src/services/desktop_network_gate.ts +++ b/apps/server/src/services/desktop_network_gate.ts @@ -8,8 +8,10 @@ import { isElectron } from "./utils.js"; * Paths that stay reachable on the desktop's loopback listener even when the user * has NOT enabled network access. These are same-machine integrations rather than * the web app: - * - `/mcp` — the MCP transport, which additionally self-restricts to loopback with - * DNS-rebinding protection (see routes/mcp.ts), so it never reaches the LAN. + * - `/mcp` — the MCP transport, which requires an ETAPI token on every request + * (see routes/mcp.ts), exactly like `/etapi`. The loopback Host check below still + * applies on top, so on desktop a token alone does not reach the LAN until the user + * opts into network access. * - `/api/clipper` — the web clipper endpoint the browser extension talks to. * - `/etapi` — the token-authenticated External API used by local automation. *