diff --git a/apps/e2e/demo-e2e-security/e2e/loopback-origin.e2e.spec.ts b/apps/e2e/demo-e2e-security/e2e/loopback-origin.e2e.spec.ts new file mode 100644 index 000000000..c99296a02 --- /dev/null +++ b/apps/e2e/demo-e2e-security/e2e/loopback-origin.e2e.spec.ts @@ -0,0 +1,77 @@ +/** + * Proposal 1.7: a loopback server refuses a request from another site's page. The browser sends that + * page's `Origin`; the `Host` is a loopback name, so the Host check alone let it through. A request + * with no `Origin` (CLI and agent clients) and one from the server's own origin are still served. + */ +import * as path from 'node:path'; + +import { expect, TestServer } from '@frontmcp/testing'; + +// Run from the app directory, whose tsconfig resolves @frontmcp/sdk to its source rather than a build. +const SERVER = { command: 'npx tsx src/loopback-origin.main.ts', cwd: path.join(__dirname, '..') }; + +const INITIALIZE = JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 'origin-e2e', version: '1.0' } }, +}); + +function initialize(server: TestServer, origin?: string): Promise { + return fetch(`${server.info.baseUrl}/`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + ...(origin ? { Origin: origin } : {}), + }, + body: INITIALIZE, + }); +} + +describe('Origin check on a loopback bind', () => { + let derived: TestServer; + let explicit: TestServer; + + beforeAll(async () => { + derived = await TestServer.start({ ...SERVER, project: 'demo-e2e-security', startupTimeout: 30_000 }); + explicit = await TestServer.start({ + ...SERVER, + project: 'demo-e2e-security', + env: { ALLOWED_ORIGINS: 'https://app.example.com' }, + startupTimeout: 30_000, + }); + }, 90_000); + + afterAll(async () => { + await derived?.stop(); + await explicit?.stop(); + }); + + it('refuses a cross-origin POST before it opens a session', async () => { + const res = await initialize(derived, 'https://evil.example'); + + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: 'Forbidden', message: 'Invalid Origin header' }); + expect(res.headers.get('mcp-session-id')).toBeNull(); + }); + + it('serves a request with no Origin', async () => { + const res = await initialize(derived); + + expect(res.status).toBe(200); + expect(res.headers.get('mcp-session-id')).toBeTruthy(); + }); + + it('serves a request from its own origin', async () => { + const res = await initialize(derived, `http://localhost:${derived.info.port}`); + + expect(res.status).toBe(200); + }); + + it('lets an explicit allowedOrigins decide instead', async () => { + expect((await initialize(explicit, 'https://app.example.com')).status).toBe(200); + expect((await initialize(explicit, `http://localhost:${explicit.info.port}`)).status).toBe(403); + expect((await initialize(explicit)).status).toBe(200); + }); +}); diff --git a/apps/e2e/demo-e2e-security/src/loopback-origin.main.ts b/apps/e2e/demo-e2e-security/src/loopback-origin.main.ts new file mode 100644 index 000000000..7b1e75d8e --- /dev/null +++ b/apps/e2e/demo-e2e-security/src/loopback-origin.main.ts @@ -0,0 +1,27 @@ +/** + * A public loopback server for the Origin check. `ALLOWED_ORIGINS` (comma-separated) sets an explicit + * `dnsRebindingProtection.allowedOrigins`; without it the allowed Origins are derived from the listener. + */ +import { App, FrontMcp, LogLevel, Tool, ToolContext } from '@frontmcp/sdk'; + +@Tool({ name: 'ping', inputSchema: {} }) +class PingTool extends ToolContext { + async execute() { + return { pong: true }; + } +} + +@App({ id: 'origin', name: 'Origin', tools: [PingTool] }) +class OriginApp {} + +const port = Number.parseInt(process.env['PORT'] ?? '3151', 10); +const allowedOrigins = (process.env['ALLOWED_ORIGINS'] ?? '').split(',').filter(Boolean); + +@FrontMcp({ + info: { name: 'Demo E2E Loopback Origin', version: '0.1.0' }, + apps: [OriginApp], + logging: { level: LogLevel.Warn }, + http: allowedOrigins.length ? { port, security: { dnsRebindingProtection: { allowedOrigins } } } : { port }, + auth: { mode: 'public' }, +}) +export default class Server {} diff --git a/apps/e2e/demo-e2e-standalone/e2e/cors-default.e2e.spec.ts b/apps/e2e/demo-e2e-standalone/e2e/cors-default.e2e.spec.ts index 5dde3b344..41c06aa75 100644 --- a/apps/e2e/demo-e2e-standalone/e2e/cors-default.e2e.spec.ts +++ b/apps/e2e/demo-e2e-standalone/e2e/cors-default.e2e.spec.ts @@ -4,11 +4,11 @@ * BREAKING in v1.7.0 (BC-034): omitting `http.cors` used to install permissive * CORS (`origin: true`); it now installs no CORS middleware at all. * - * The docs make a claim that is easy to get backwards, so it is pinned here: a - * cross-origin request is still DELIVERED and served normally — the server does - * not reject it — and the response simply carries no `Access-Control-Allow-Origin` - * header, which is what makes a browser refuse to let the calling page read it. - * CORS is a browser rule, not server-side access control. + * CORS is a browser rule, not server-side access control: the response carries no + * `Access-Control-Allow-Origin` header, which is what makes a browser refuse to + * let the calling page read it. Refusing the request is a separate check: this + * server binds loopback, where a request from another origin is refused outright + * (proposal 1.7) — a request with no `Origin` is still served. * * The browser half of that (refusing the read) is enforced by the browser vendor * from the absence of the header, so driving a real browser here would test @@ -26,18 +26,22 @@ test.describe('CORS default: no headers', () => { publicMode: true, }); - test('delivers and serves a cross-origin request, but sends no CORS headers', async ({ server }) => { + test('refuses a cross-origin request on a loopback bind, and sends no CORS headers', async ({ server }) => { const res = await fetch(`${server.info.baseUrl}/custom/ping`, { headers: { Origin: CROSS_ORIGIN }, }); - // Delivered and served — the server does not reject cross-origin callers. + expect(res.status).toBe(403); + expect(res.headers.get('access-control-allow-origin')).toBeNull(); + expect(res.headers.get('access-control-allow-credentials')).toBeNull(); + }); + + test('serves a request with no Origin and sends no CORS headers', async ({ server }) => { + const res = await fetch(`${server.info.baseUrl}/custom/ping`); + expect(res.status).toBe(200); expect(await res.json()).toMatchObject({ ok: true }); - - // ...but nothing tells the browser the calling page may read it. expect(res.headers.get('access-control-allow-origin')).toBeNull(); - expect(res.headers.get('access-control-allow-credentials')).toBeNull(); }); test('does not answer a preflight with CORS headers', async ({ server }) => { diff --git a/apps/e2e/demo-e2e-standalone/e2e/cors-explicit-origin.e2e.spec.ts b/apps/e2e/demo-e2e-standalone/e2e/cors-explicit-origin.e2e.spec.ts index 41f9b5b66..31a2a5b10 100644 --- a/apps/e2e/demo-e2e-standalone/e2e/cors-explicit-origin.e2e.spec.ts +++ b/apps/e2e/demo-e2e-standalone/e2e/cors-explicit-origin.e2e.spec.ts @@ -5,9 +5,9 @@ * file, so a CORS-configured server and a CORS-less one cannot share one. * * The contrast is the point: a configured origin gets an - * `Access-Control-Allow-Origin` header, an unconfigured one does not — and - * neither is REJECTED. CORS decides what a browser may read, never what the - * server will answer. + * `Access-Control-Allow-Origin` header and is served. An unconfigured one is + * refused, because this server binds loopback and checks `Origin` there + * (proposal 1.7); the CORS list is what admits the configured one. */ import { expect, test } from '@frontmcp/testing'; @@ -27,13 +27,12 @@ test.describe('CORS opt-in: explicit origin', () => { expect(res.headers.get('access-control-allow-origin')).toBe('https://allowed.example.com'); }); - test('never echoes an origin that is not on the list', async ({ server }) => { + test('refuses an origin that is not on the list, and never echoes it', async ({ server }) => { const res = await fetch(`${server.info.baseUrl}/custom/ping`, { headers: { Origin: 'https://evil.example.com' }, }); - // Still delivered — CORS never blocks the request, only the browser-side read. - expect(res.status).toBe(200); + expect(res.status).toBe(403); // A fixed `origin` string is sent on every response, so the header IS present; what matters is // that it names the allowed origin, never the caller's. The browser compares the two and // refuses the read on a mismatch. diff --git a/apps/e2e/demo-e2e-standalone/e2e/mcp-registry.e2e.spec.ts b/apps/e2e/demo-e2e-standalone/e2e/mcp-registry.e2e.spec.ts index df66d0167..03f612ddd 100644 --- a/apps/e2e/demo-e2e-standalone/e2e/mcp-registry.e2e.spec.ts +++ b/apps/e2e/demo-e2e-standalone/e2e/mcp-registry.e2e.spec.ts @@ -44,6 +44,14 @@ describe('MCP Registry documents', () => { expect((await fetch(`${server.info.baseUrl}/mcp/isolated/server-card`)).status).toBe(404); }); + it('refuses the card to a page on another origin, since the server binds loopback', async () => { + const card = await fetch(`${server.info.baseUrl}/mcp/server-card`, { + headers: { Origin: 'https://registry.example' }, + }); + + expect(card.status).toBe(403); + }); + it('lists the card in the AI Catalog', async () => { const catalog = await (await fetch(`${server.info.baseUrl}/.well-known/ai-catalog.json`)).json(); expect(catalog).toEqual({ diff --git a/apps/e2e/demo-e2e-stdio-transport/e2e/helpers/stdio-json-rpc.ts b/apps/e2e/demo-e2e-stdio-transport/e2e/helpers/stdio-json-rpc.ts index e0dffc473..25fb95789 100644 --- a/apps/e2e/demo-e2e-stdio-transport/e2e/helpers/stdio-json-rpc.ts +++ b/apps/e2e/demo-e2e-stdio-transport/e2e/helpers/stdio-json-rpc.ts @@ -30,10 +30,11 @@ export class StdioJsonRpcProcess { }); } - static start(entrypointPath: string, cwd: string): StdioJsonRpcProcess { - const child = spawn('npx', ['tsx', entrypointPath], { + /** Run the entrypoint in this process's node, so a signal reaches the server itself and not a launcher shell. */ + static start(entrypointPath: string, cwd: string, env: Record = {}): StdioJsonRpcProcess { + const child = spawn(process.execPath, ['--import', 'tsx', entrypointPath], { cwd, - env: process.env as Record, + env: { ...process.env, ...env } as Record, stdio: ['pipe', 'pipe', 'pipe'], }); child.stderr.resume(); @@ -66,8 +67,20 @@ export class StdioJsonRpcProcess { } } + private hasExited(): boolean { + return this.child.exitCode !== null || this.child.signalCode !== null; + } + + /** Send SIGTERM, leaving stdin open, and resolve with the exit code. */ + async terminate(): Promise { + if (this.hasExited()) return this.child.exitCode; + const exited = new Promise((resolve) => this.child.once('exit', (code) => resolve(code))); + this.child.kill('SIGTERM'); + return exited; + } + async stop(): Promise { - if (this.child.exitCode !== null) return; + if (this.hasExited()) return; const exited = new Promise((resolve) => this.child.once('close', resolve)); this.child.stdin.end(); this.child.kill('SIGTERM'); diff --git a/apps/e2e/demo-e2e-stdio-transport/e2e/stdio-dispose.e2e.spec.ts b/apps/e2e/demo-e2e-stdio-transport/e2e/stdio-dispose.e2e.spec.ts new file mode 100644 index 000000000..ee5496cf0 --- /dev/null +++ b/apps/e2e/demo-e2e-stdio-transport/e2e/stdio-dispose.e2e.spec.ts @@ -0,0 +1,43 @@ +/** + * E2E: SIGTERM on a stdio server disposes its scope, so `onDispose` callbacks run before it exits. + * They never ran under stdio: the handler shut the scope down and exited without disposing it. + */ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import { StdioJsonRpcProcess } from './helpers/stdio-json-rpc'; + +const projectPath = path.join(__dirname, '..'); + +describe('stdio shutdown', () => { + let directory: string; + let server: StdioJsonRpcProcess | undefined; + + beforeEach(() => { + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'stdio-dispose-')); + }); + + afterEach(async () => { + await server?.stop(); + server = undefined; + fs.rmSync(directory, { recursive: true, force: true }); + }); + + it('runs onDispose callbacks on SIGTERM, then exits 0', async () => { + const marker = path.join(directory, 'disposed'); + server = StdioJsonRpcProcess.start('src/stdio-dispose-entrypoint.ts', projectPath, { DISPOSE_MARKER: marker }); + const initialized = await server.request({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 'dispose-e2e', version: '1.0' } }, + }); + expect(initialized.result).toBeDefined(); + + const exitCode = await server.terminate(); + + expect(exitCode).toBe(0); + expect(fs.readFileSync(marker, 'utf8')).toBe('disposed'); + }, 60_000); +}); diff --git a/apps/e2e/demo-e2e-stdio-transport/src/stdio-dispose-entrypoint.ts b/apps/e2e/demo-e2e-stdio-transport/src/stdio-dispose-entrypoint.ts new file mode 100644 index 000000000..e5ae670bc --- /dev/null +++ b/apps/e2e/demo-e2e-stdio-transport/src/stdio-dispose-entrypoint.ts @@ -0,0 +1,29 @@ +/** + * Stdio entrypoint whose scope writes `DISPOSE_MARKER` from an `onDispose` callback, registered from a + * factory provider as the catalog shows. + */ +import { App, FrontMcpInstance, ScopeEntry } from '@frontmcp/sdk'; +import { writeFile } from '@frontmcp/utils'; + +import { serverConfig } from './config.js'; + +class DisposeMarker {} + +const disposeMarkerProvider = { + name: 'DisposeMarker', + provide: DisposeMarker, + inject: () => [ScopeEntry] as const, + useFactory: (scope: ScopeEntry) => { + const markerPath = process.env['DISPOSE_MARKER']; + if (markerPath) scope.onDispose(() => writeFile(markerPath, 'disposed')); + return new DisposeMarker(); + }, +}; + +@App({ name: 'Dispose', providers: [disposeMarkerProvider] }) +class DisposeApp {} + +FrontMcpInstance.runStdio({ ...serverConfig, apps: [...serverConfig.apps, DisposeApp] }).catch((err) => { + console.error('Failed to start stdio server:', err); + process.exit(1); +}); diff --git a/apps/e2e/demo-e2e-unix-socket/e2e/unix-socket.e2e.spec.ts b/apps/e2e/demo-e2e-unix-socket/e2e/unix-socket.e2e.spec.ts index 32b35db0d..04ca775d2 100644 --- a/apps/e2e/demo-e2e-unix-socket/e2e/unix-socket.e2e.spec.ts +++ b/apps/e2e/demo-e2e-unix-socket/e2e/unix-socket.e2e.spec.ts @@ -5,13 +5,16 @@ * Verifies the full MCP request lifecycle over Unix domain sockets. */ -import * as os from 'node:os'; +import { spawnSync } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; -import { randomUUID } from 'node:crypto'; + import { FrontMcpInstance } from '@frontmcp/sdk'; -import { serverConfig } from '../src/config'; + import { notesStore } from '../src/apps/notes/data/notes.store'; +import { serverConfig } from '../src/config'; import { httpOverSocket, UnixSocketMcpClient } from './helpers/unix-socket-client'; /** @@ -98,13 +101,10 @@ describe('Unix Socket Transport E2E', () => { expect(fs.existsSync(socketPath)).toBe(true); }); - it('should set socket file permissions to 0o660', async () => { + it('creates the socket owner-only (0600), so the group cannot connect', async () => { await startServer(); - const stats = fs.statSync(socketPath); - // Extract permission bits (last 9 bits) - const perms = stats.mode & 0o777; - expect(perms).toBe(0o660); + expect(fs.lstatSync(socketPath).mode & 0o777).toBe(0o600); }); it('should clean up socket file on close', async () => { @@ -118,19 +118,46 @@ describe('Unix Socket Transport E2E', () => { expect(fs.existsSync(socketPath)).toBe(false); }); - it('should handle stale socket file on restart', async () => { - // Create a stale socket file - fs.writeFileSync(socketPath, ''); - expect(fs.existsSync(socketPath)).toBe(true); + it('replaces a stale socket a crashed server left behind', async () => { + const script = `require('net').createServer().listen(${JSON.stringify(socketPath)}, () => process.kill(process.pid, 'SIGKILL'))`; + spawnSync(process.execPath, ['-e', script]); + expect(fs.lstatSync(socketPath).isSocket()).toBe(true); - // Starting should clean up the stale file and proceed await startServer(); - expect(fs.existsSync(socketPath)).toBe(true); - // Verify the server is actually functional const response = await httpOverSocket(socketPath, { path: '/health' }); expect(response.statusCode).toBe(200); }); + + it('refuses to remove a regular file at the socket path', async () => { + fs.writeFileSync(socketPath, 'not a socket'); + + await expect(startServer()).rejects.toThrow(/not a socket/); + expect(fs.readFileSync(socketPath, 'utf8')).toBe('not a socket'); + }); + + it('refuses to take over a live socket, and the first server keeps serving', async () => { + await startServer(); + + await expect(FrontMcpInstance.runUnixSocket({ ...serverConfig, socketPath })).rejects.toThrow(/already in use/); + + const response = await httpOverSocket(socketPath, { path: '/health' }); + expect(response.statusCode).toBe(200); + }); + + it('refuses a world-writable directory without the sticky bit', async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-')); + fs.chmodSync(directory, 0o777); + const unsafePath = path.join(directory, 's.sock'); + try { + await expect(FrontMcpInstance.runUnixSocket({ ...serverConfig, socketPath: unsafePath })).rejects.toThrow( + `${directory} (mode 0777)`, + ); + expect(fs.existsSync(unsafePath)).toBe(false); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } + }); }); // ─── Health Endpoint ─────────────────────────────────────────────────────── diff --git a/libs/cli/src/commands/build/exec/__tests__/generate-cli-entry.spec.ts b/libs/cli/src/commands/build/exec/__tests__/generate-cli-entry.spec.ts index f9f8517bd..5a978b206 100644 --- a/libs/cli/src/commands/build/exec/__tests__/generate-cli-entry.spec.ts +++ b/libs/cli/src/commands/build/exec/__tests__/generate-cli-entry.spec.ts @@ -979,6 +979,32 @@ describe('generateCliEntry', () => { expect(source).toContain('.pid'); }); + it('creates the sockets directory owner-only (0700)', () => { + const source = generateCliEntry(makeOptions()); + expect(source).toContain('fs.mkdirSync(socketDir, { recursive: true, mode: 0o700 })'); + expect(source).toContain('fs.chmodSync(socketDir, 0o700)'); + }); + + it('takes group and other write away from FRONTMCP_HOME, which could otherwise replace the sockets directory', () => { + const source = generateCliEntry(makeOptions()); + expect(source).toContain('if (homeMode & 0o022) fs.chmodSync(FRONTMCP_HOME, homeMode & ~0o022)'); + }); + + it('refuses a symlinked sockets directory before setting its mode', () => { + const source = generateCliEntry(makeOptions()); + const symlinkCheck = source.indexOf('fs.lstatSync(socketDir).isSymbolicLink()'); + expect(symlinkCheck).toBeGreaterThan(-1); + expect(source.indexOf('fs.chmodSync(socketDir, 0o700)')).toBeGreaterThan(symlinkCheck); + }); + + it('removes a leftover socket only once no daemon is running', () => { + const source = generateCliEntry(makeOptions()); + const runningCheck = source.indexOf("console.log('Daemon already running"); + const unlink = source.indexOf('fs.unlinkSync(socketPath)'); + expect(runningCheck).toBeGreaterThan(-1); + expect(unlink).toBeGreaterThan(runningCheck); + }); + it('should write the name and entry that frontmcp list and status read (#768)', () => { const source = generateCliEntry(makeOptions({ appName: 'my-server', selfContained: false })); expect(source).toContain('name: "my-server"'); diff --git a/libs/cli/src/commands/build/exec/cli-runtime/generate-cli-entry.ts b/libs/cli/src/commands/build/exec/cli-runtime/generate-cli-entry.ts index c78fe8dab..cf573f807 100644 --- a/libs/cli/src/commands/build/exec/cli-runtime/generate-cli-entry.ts +++ b/libs/cli/src/commands/build/exec/cli-runtime/generate-cli-entry.ts @@ -1823,14 +1823,19 @@ daemonCmd var socketDir = pathMod.join(FRONTMCP_HOME, 'sockets'); fs.mkdirSync(pidDir, { recursive: true }); fs.mkdirSync(logDir, { recursive: true }); - fs.mkdirSync(socketDir, { recursive: true }); + // The server refuses a socket others could replace, through its directory or any above it + fs.mkdirSync(socketDir, { recursive: true, mode: 0o700 }); + var homeMode = fs.statSync(FRONTMCP_HOME).mode & 0o7777; + if (homeMode & 0o022) fs.chmodSync(FRONTMCP_HOME, homeMode & ~0o022); + if (fs.lstatSync(socketDir).isSymbolicLink()) { + console.error('Refusing to use ' + socketDir + ': it is a symlink. Replace it with a directory only you can write to.'); + process.exit(1); + } + fs.chmodSync(socketDir, 0o700); var usePort = !!opts.port; var socketPath = pathMod.join(socketDir, ${JSON.stringify(appName)} + '.sock'); - // Clean up stale socket file (only relevant in socket mode) - if (!usePort) { try { fs.unlinkSync(socketPath); } catch (_) { /* ok */ } } - // Check if already running var pidPath = pathMod.join(pidDir, ${JSON.stringify(appName)} + '.pid'); try { @@ -1840,6 +1845,9 @@ daemonCmd return; } catch (_) { /* not running, proceed */ } + // Only once no daemon runs: removing the socket of a running one would cut it off + if (!usePort) { try { fs.unlinkSync(socketPath); } catch (_) { /* ok */ } } + var env = Object.assign({}, process.env, { FRONTMCP_DAEMON_IDLE_TIMEOUT: String(opts.idleTimeout) }); diff --git a/libs/cli/src/commands/pm/__tests__/paths.spec.ts b/libs/cli/src/commands/pm/__tests__/paths.spec.ts index 898d190d2..fe1cc6d9a 100644 --- a/libs/cli/src/commands/pm/__tests__/paths.spec.ts +++ b/libs/cli/src/commands/pm/__tests__/paths.spec.ts @@ -1,6 +1,8 @@ +import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { PM_DIRS, pidFilePath, logFilePath, errorLogFilePath, socketFilePath, appDir, registryPath } from '../paths'; + +import { appDir, errorLogFilePath, logFilePath, pidFilePath, PM_DIRS, registryPath, socketFilePath } from '../paths'; describe('pm.paths', () => { const home = os.homedir(); @@ -62,3 +64,80 @@ describe('pm.paths', () => { }); }); }); + +describe('pm sockets directory', () => { + const posixIt = process.platform === 'win32' ? it.skip : it; + let home: string; + + beforeEach(() => { + home = fs.mkdtempSync(path.join(os.tmpdir(), 'pm-home-')); + }); + + afterEach(() => { + fs.rmSync(home, { recursive: true, force: true }); + }); + + function pathsUnder(homeDir: string): typeof import('../paths') { + let loaded: typeof import('../paths') | undefined; + jest.isolateModules(() => { + jest.doMock('os', () => ({ ...jest.requireActual('os'), homedir: () => homeDir })); + loaded = require('../paths'); + }); + jest.dontMock('os'); + if (!loaded) throw new Error('paths module did not load'); + return loaded; + } + + posixIt('creates ~/.frontmcp/sockets owner-only (0700)', () => { + const paths = pathsUnder(home); + + paths.ensureSocketsDir(); + + expect(fs.statSync(paths.PM_DIRS.sockets).mode & 0o777).toBe(0o700); + }); + + posixIt('leaves the sockets directory mode to the commands that start a socket server', () => { + const paths = pathsUnder(home); + fs.mkdirSync(paths.PM_DIRS.sockets, { recursive: true, mode: 0o755 }); + fs.chmodSync(paths.PM_DIRS.sockets, 0o755); + + paths.ensurePmDirs(); + + expect(fs.statSync(paths.PM_DIRS.sockets).mode & 0o777).toBe(0o755); + }); + + posixIt( + 'takes group and other write away from ~/.frontmcp, which could otherwise replace the sockets directory', + () => { + const paths = pathsUnder(home); + fs.mkdirSync(paths.PM_DIRS.root, { recursive: true }); + fs.chmodSync(paths.PM_DIRS.root, 0o775); + + paths.ensureSocketsDir(); + + expect(fs.statSync(paths.PM_DIRS.root).mode & 0o777).toBe(0o755); + }, + ); + + posixIt('refuses a symlinked sockets directory, and leaves the directory it points to as it was', () => { + const paths = pathsUnder(home); + const elsewhere = path.join(home, 'elsewhere'); + fs.mkdirSync(elsewhere, { mode: 0o755 }); + fs.chmodSync(elsewhere, 0o755); + fs.mkdirSync(paths.PM_DIRS.root, { recursive: true }); + fs.symlinkSync(elsewhere, paths.PM_DIRS.sockets); + + expect(() => paths.ensureSocketsDir()).toThrow(paths.PM_DIRS.sockets); + expect(fs.statSync(elsewhere).mode & 0o777).toBe(0o755); + }); + + posixIt('tightens a sockets directory an earlier version created with the default mode', () => { + const paths = pathsUnder(home); + fs.mkdirSync(paths.PM_DIRS.sockets, { recursive: true, mode: 0o775 }); + fs.chmodSync(paths.PM_DIRS.sockets, 0o775); + + paths.ensureSocketsDir(); + + expect(fs.statSync(paths.PM_DIRS.sockets).mode & 0o777).toBe(0o700); + }); +}); diff --git a/libs/cli/src/commands/pm/__tests__/spawn.spec.ts b/libs/cli/src/commands/pm/__tests__/spawn.spec.ts index a9d7f1b6b..302278241 100644 --- a/libs/cli/src/commands/pm/__tests__/spawn.spec.ts +++ b/libs/cli/src/commands/pm/__tests__/spawn.spec.ts @@ -10,6 +10,8 @@ jest.mock('../log-utils', () => ({ const writePidFile = jest.fn(); jest.mock('../pidfile', () => ({ writePidFile: (...a: unknown[]) => writePidFile(...a), removePidFile: jest.fn() })); jest.mock('../health', () => ({ checkHealth: jest.fn() })); +const ensureSocketsDir = jest.fn(); +jest.mock('../paths', () => ({ ...jest.requireActual('../paths'), ensureSocketsDir: () => ensureSocketsDir() })); function fakeChild() { const child = new EventEmitter() as EventEmitter & { pid: number; stdout: null; stderr: null; kill: jest.Mock }; @@ -47,6 +49,17 @@ describe('Supervisor environment (#642)', () => { expect(env['FRONTMCP_SQLITE_PATH']).toBe('/tmp/a.sqlite'); }); + it('makes ~/.frontmcp/sockets owner-only before a start on the default socket path, and only then', async () => { + ensureSocketsDir.mockClear(); + for (const opts of [{ socket: true, socketPath: '/tmp/a.sock' }, { port: 4100 }, { socket: true }]) { + const sup = new Supervisor({ name: 'a', entry: 'main.ts', ...opts }); + await sup.start(); + await sup.stop(true); + } + + expect(ensureSocketsDir).toHaveBeenCalledTimes(1); + }); + it('records maxRestarts in the pid file so restart and service install can reuse it', async () => { const sup = new Supervisor({ name: 'a', entry: 'main.ts', maxRestarts: 7 }); await sup.start(); diff --git a/libs/cli/src/commands/pm/paths.ts b/libs/cli/src/commands/pm/paths.ts index 57be5f102..94b84df0c 100644 --- a/libs/cli/src/commands/pm/paths.ts +++ b/libs/cli/src/commands/pm/paths.ts @@ -4,10 +4,11 @@ */ import * as fs from 'fs'; -import * as path from 'path'; import * as os from 'os'; +import * as path from 'path'; const FRONTMCP_HOME = path.join(os.homedir(), '.frontmcp'); +const GROUP_OR_OTHER_WRITE = 0o022; export const PM_DIRS = { root: FRONTMCP_HOME, @@ -51,3 +52,16 @@ export function ensurePmDirs(): void { } } } + +/** The server refuses a socket others could replace, so ~/.frontmcp is not writable by others and its sockets are owner-only. */ +export function ensureSocketsDir(): void { + fs.mkdirSync(PM_DIRS.sockets, { recursive: true, mode: 0o700 }); + const rootMode = fs.statSync(PM_DIRS.root).mode & 0o7777; + if (rootMode & GROUP_OR_OTHER_WRITE) fs.chmodSync(PM_DIRS.root, rootMode & ~GROUP_OR_OTHER_WRITE); + if (fs.lstatSync(PM_DIRS.sockets).isSymbolicLink()) { + throw new Error( + `Refusing to use ${PM_DIRS.sockets}: it is a symlink. Replace it with a directory only you can write to.`, + ); + } + fs.chmodSync(PM_DIRS.sockets, 0o700); +} diff --git a/libs/cli/src/commands/pm/socket.ts b/libs/cli/src/commands/pm/socket.ts index 52b150879..5d7a71c05 100644 --- a/libs/cli/src/commands/pm/socket.ts +++ b/libs/cli/src/commands/pm/socket.ts @@ -1,5 +1,4 @@ import { spawn, type ChildProcess } from 'child_process'; -import * as os from 'os'; import * as path from 'path'; import { type ParsedArgs } from '../../core/args'; @@ -7,15 +6,9 @@ import { c } from '../../core/colors'; import { loadDevEnv } from '../../shared/env'; import { resolveEntry } from '../../shared/fs'; import { projectToolCommand, spawnTool } from '../../shared/tool-command'; +import { ensureSocketsDir, PM_DIRS } from './paths'; import { loadShipEnv } from './ship-env'; -function ensureDir(dir: string): void { - const fs = require('fs'); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } -} - function resolveSocketPath(opts: ParsedArgs, entryPath: string): string { if (opts.socket) { return path.resolve(opts.socket); @@ -23,9 +16,8 @@ function resolveSocketPath(opts: ParsedArgs, entryPath: string): string { // Default: ~/.frontmcp/sockets/{app-name}.sock const appName = path.basename(path.dirname(entryPath)); - const socketsDir = path.join(os.homedir(), '.frontmcp', 'sockets'); - ensureDir(socketsDir); - return path.join(socketsDir, `${appName}.sock`); + ensureSocketsDir(); + return path.join(PM_DIRS.sockets, `${appName}.sock`); } function resolveDbPath(opts: ParsedArgs): string | undefined { diff --git a/libs/cli/src/commands/pm/spawn.ts b/libs/cli/src/commands/pm/spawn.ts index 8b5f66ded..e3e878415 100644 --- a/libs/cli/src/commands/pm/spawn.ts +++ b/libs/cli/src/commands/pm/spawn.ts @@ -9,7 +9,7 @@ import { getSelfVersion } from '../../core/version'; import { projectToolCommand, spawnTool } from '../../shared/tool-command'; import { checkHealth } from './health'; import { createLogStreams } from './log-utils'; -import { socketFilePath } from './paths'; +import { ensureSocketsDir, socketFilePath } from './paths'; import { removePidFile, writePidFile } from './pidfile'; import { type StartOptions } from './types'; @@ -40,6 +40,7 @@ export class Supervisor { async start(): Promise { this.running = true; this.restartCount = 0; + if (this.resolvedSocketPath && !this.opts.socketPath) ensureSocketsDir(); await this.spawnChild(); } diff --git a/libs/sdk/src/common/types/options/http/interfaces.ts b/libs/sdk/src/common/types/options/http/interfaces.ts index b806ceb5e..b8ad02919 100644 --- a/libs/sdk/src/common/types/options/http/interfaces.ts +++ b/libs/sdk/src/common/types/options/http/interfaces.ts @@ -104,7 +104,8 @@ export interface HttpOptionsInterface { * Unix socket path for local-only server mode. * When set, the server listens on a Unix socket instead of a TCP port. * The entire HTTP feature set (streamable HTTP, SSE, elicitation, sessions) - * works unchanged over Unix sockets. + * works unchanged over Unix sockets. On POSIX the socket is created 0600, in a + * directory only the owner can write to (or a sticky one, such as /tmp). */ socketPath?: string; @@ -259,7 +260,10 @@ export interface SecurityOptions { enabled?: boolean; /** Allowed Host header values (e.g., ['localhost:3001', 'api.example.com']) */ allowedHosts?: string[]; - /** Allowed Origin header values (e.g., ['https://app.example.com']) */ + /** + * Allowed Origin header values (e.g., ['https://app.example.com']). A request with no Origin is always allowed. + * Unset on a loopback bind: the server's own origins, its issuer and the `cors.origin` list. + */ allowedOrigins?: string[]; }; } diff --git a/libs/sdk/src/errors/index.ts b/libs/sdk/src/errors/index.ts index 83c93d41a..eedcacad3 100644 --- a/libs/sdk/src/errors/index.ts +++ b/libs/sdk/src/errors/index.ts @@ -207,6 +207,10 @@ export { TransportServiceNotAvailableError, SessionClaimConflictError, SessionOwnerUnreachableError, + UnsafeSocketDirectoryError, + SocketInUseError, + SocketPathConflictError, + SocketPermissionError, } from './transport.errors'; // Export auth internal errors diff --git a/libs/sdk/src/errors/transport.errors.ts b/libs/sdk/src/errors/transport.errors.ts index c20632388..9397b6f3e 100644 --- a/libs/sdk/src/errors/transport.errors.ts +++ b/libs/sdk/src/errors/transport.errors.ts @@ -121,3 +121,46 @@ export class SessionOwnerUnreachableError extends PublicMcpError { this.reason = reason; } } + +function octalMode(mode: number): string { + return `0${(mode & 0o7777).toString(8)}`; +} + +/** + * Thrown at startup when another user could replace a Unix socket: its directory, or one above it, + * belongs to another user (not root), or the group or others can write to it and it has no sticky bit. + */ +export class UnsafeSocketDirectoryError extends InternalMcpError { + constructor(directory: string, mode: number, ownerUid?: number) { + const reason = + ownerUid === undefined + ? 'the group or others can write to it and it has no sticky bit' + : `it belongs to another user (uid ${ownerUid})`; + super( + `Refusing to create a Unix socket under ${directory} (mode ${octalMode(mode)}): ${reason}. ` + + 'Use a directory only you can write to, such as ~/.frontmcp/sockets.', + 'UNSAFE_SOCKET_DIRECTORY', + ); + } +} + +/** Thrown at startup when a server is already listening on the Unix socket path. */ +export class SocketInUseError extends InternalMcpError { + constructor(socketPath: string) { + super(`Unix socket ${socketPath} is already in use by a running server.`, 'SOCKET_IN_USE'); + } +} + +/** Thrown at startup when the Unix socket path holds something the server must not remove. */ +export class SocketPathConflictError extends InternalMcpError { + constructor(socketPath: string, reason: string) { + super(`Refusing to replace ${socketPath}: ${reason}.`, 'SOCKET_PATH_CONFLICT'); + } +} + +/** Thrown at startup when a Unix socket cannot be made owner-only (0600). */ +export class SocketPermissionError extends InternalMcpError { + constructor(socketPath: string, detail: string) { + super(`Could not make Unix socket ${socketPath} owner-only (0600): ${detail}.`, 'SOCKET_PERMISSION'); + } +} diff --git a/libs/sdk/src/front-mcp/__tests__/graceful-shutdown.spec.ts b/libs/sdk/src/front-mcp/__tests__/graceful-shutdown.spec.ts index 1aee87b06..95b4e0ca6 100644 --- a/libs/sdk/src/front-mcp/__tests__/graceful-shutdown.spec.ts +++ b/libs/sdk/src/front-mcp/__tests__/graceful-shutdown.spec.ts @@ -212,6 +212,28 @@ describe('FrontMcpInstance.runUnixSocket() shutdown', () => { } }); + it('shuts down what it built when the start is refused, then rethrows', async () => { + const socketPath = path.join(os.tmpdir(), `fmcp-${randomUUID().slice(0, 8)}.sock`); + const occupant = net.createServer(); + await new Promise((resolve) => occupant.listen(socketPath, resolve)); + const shutdown = jest.spyOn(FrontMcpInstance.prototype, 'shutdown'); + try { + await expect( + FrontMcpInstance.runUnixSocket({ + info: { name: 'refused-socket', version: '1.0.0' }, + apps: [DemoApp], + logging: { level: LogLevel.Off }, + socketPath, + }), + ).rejects.toThrow('already in use'); + + expect(shutdown).toHaveBeenCalledTimes(1); + } finally { + shutdown.mockRestore(); + await new Promise((resolve) => occupant.close(() => resolve())); + } + }); + it('leaves the socket of a replacement that binds the same path while it shuts down', async () => { const socketPath = path.join(os.tmpdir(), `fmcp-${randomUUID().slice(0, 8)}.sock`); const replacement = net.createServer(); diff --git a/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose-deadline.spec.ts b/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose-deadline.spec.ts new file mode 100644 index 000000000..9fa80b9c6 --- /dev/null +++ b/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose-deadline.spec.ts @@ -0,0 +1,110 @@ +/** + * An `onDispose` callback that never settles cannot keep a stdio server alive: the process exits 1 + * once the stdio shutdown deadline passes, as the HTTP and socket servers do. + */ +import 'reflect-metadata'; + +import { App, LogLevel, Tool, ToolContext, type ScopeEntry } from '../../common'; +import { FrontMcpInstance } from '../front-mcp'; +import { STDIO_SHUTDOWN_DEADLINE_MS } from '../shutdown-signals'; + +jest.mock('@frontmcp/protocol', () => { + const actual = jest.requireActual('@frontmcp/protocol'); + class IdleMcpServer { + setRequestHandler() { + return undefined; + } + setNotificationHandler() { + return undefined; + } + async connect() { + return undefined; + } + async close() { + return undefined; + } + } + class IdleStdioServerTransport { + async close() { + return undefined; + } + } + return { ...actual, McpServer: IdleMcpServer, StdioServerTransport: IdleStdioServerTransport }; +}); + +@Tool({ name: 'noop', inputSchema: {} }) +class NoopTool extends ToolContext { + async execute() { + return {}; + } +} + +@App({ id: 'hang', name: 'Hang', tools: [NoopTool] }) +class HangApp {} + +function flush(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + +describe('runStdio shutdown deadline', () => { + const savedConsole = { ...console }; + const savedStdioEnv = process.env['FRONTMCP_STDIO']; + const savedLogsMax = process.env['FRONTMCP_LOGS_MAX']; + const signalsBefore = process.listeners('SIGTERM'); + + afterAll(() => { + jest.useRealTimers(); + Object.assign(console, savedConsole); + if (savedStdioEnv === undefined) delete process.env['FRONTMCP_STDIO']; + else process.env['FRONTMCP_STDIO'] = savedStdioEnv; + if (savedLogsMax === undefined) delete process.env['FRONTMCP_LOGS_MAX']; + else process.env['FRONTMCP_LOGS_MAX'] = savedLogsMax; + for (const signal of ['SIGINT', 'SIGTERM'] as const) { + for (const listener of process.listeners(signal)) { + if (!signalsBefore.includes(listener)) process.removeListener(signal, listener); + } + } + }); + + it('exits 1 at the deadline when an onDispose callback hangs', async () => { + process.env['FRONTMCP_LOGS_MAX'] = '0'; + const exit = jest.spyOn(process, 'exit').mockImplementation((() => undefined) as never); + const getPrimaryScope = FrontMcpInstance.prototype.getPrimaryScope; + let scope: ScopeEntry | undefined; + const capture = jest.spyOn(FrontMcpInstance.prototype, 'getPrimaryScope').mockImplementation(function ( + this: FrontMcpInstance, + ) { + scope = getPrimaryScope.call(this); + return scope; + }); + try { + await FrontMcpInstance.runStdio({ + info: { name: 'stdio-hang', version: '1.0.0' }, + apps: [HangApp], + logging: { level: LogLevel.Off, enableConsole: false }, + }); + capture.mockRestore(); + if (!scope) throw new Error('runStdio served no scope'); + let entered = false; + scope.onDispose( + () => + new Promise(() => { + entered = true; + }), + ); + const onSigterm = process.listeners('SIGTERM').find((listener) => !signalsBefore.includes(listener)); + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + + onSigterm?.('SIGTERM'); + for (let i = 0; i < 100 && !entered; i++) await flush(); + expect(entered).toBe(true); + jest.advanceTimersByTime(STDIO_SHUTDOWN_DEADLINE_MS - 1); + expect(exit).not.toHaveBeenCalled(); + jest.advanceTimersByTime(1); + + expect(exit).toHaveBeenCalledWith(1); + } finally { + exit.mockRestore(); + } + }); +}); diff --git a/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose.spec.ts b/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose.spec.ts new file mode 100644 index 000000000..969f8885c --- /dev/null +++ b/libs/sdk/src/front-mcp/__tests__/run-stdio-dispose.spec.ts @@ -0,0 +1,110 @@ +/** + * `runStdio` shuts down like the HTTP and socket servers: SIGTERM/SIGINT shut every scope down and + * then dispose it, so `scope.onDispose()` callbacks run under stdio too. They never did: the stdio + * handler called `scope.shutdown()` and exited. + */ +import 'reflect-metadata'; + +import { App, LogLevel, Tool, ToolContext, type ScopeEntry } from '../../common'; +import { FrontMcpInstance } from '../front-mcp'; + +const mockServerClose = jest.fn(); + +jest.mock('@frontmcp/protocol', () => { + const actual = jest.requireActual('@frontmcp/protocol'); + class IdleMcpServer { + setRequestHandler() { + return undefined; + } + setNotificationHandler() { + return undefined; + } + async connect() { + return undefined; + } + async close() { + mockServerClose(); + } + } + class IdleStdioServerTransport { + async close() { + return undefined; + } + } + return { ...actual, McpServer: IdleMcpServer, StdioServerTransport: IdleStdioServerTransport }; +}); + +@Tool({ name: 'noop', inputSchema: {} }) +class NoopTool extends ToolContext { + async execute() { + return {}; + } +} + +@App({ id: 'dispose', name: 'Dispose', tools: [NoopTool] }) +class DisposeApp {} + +describe('runStdio shutdown', () => { + const savedConsole = { ...console }; + const savedStdioEnv = process.env['FRONTMCP_STDIO']; + const savedLogsMax = process.env['FRONTMCP_LOGS_MAX']; + const signalsBefore = process.listeners('SIGTERM'); + let exit: jest.SpyInstance; + + beforeAll(() => { + process.env['FRONTMCP_LOGS_MAX'] = '0'; + exit = jest.spyOn(process, 'exit').mockImplementation((() => undefined) as never); + }); + + afterAll(() => { + exit.mockRestore(); + Object.assign(console, savedConsole); + if (savedStdioEnv === undefined) delete process.env['FRONTMCP_STDIO']; + else process.env['FRONTMCP_STDIO'] = savedStdioEnv; + if (savedLogsMax === undefined) delete process.env['FRONTMCP_LOGS_MAX']; + else process.env['FRONTMCP_LOGS_MAX'] = savedLogsMax; + for (const signal of ['SIGINT', 'SIGTERM'] as const) { + for (const listener of process.listeners(signal)) { + if (!signalsBefore.includes(listener)) process.removeListener(signal, listener); + } + } + }); + + it('awaits every onDispose callback on SIGTERM, then closes the server and exits 0', async () => { + const getPrimaryScope = FrontMcpInstance.prototype.getPrimaryScope; + let scope: ScopeEntry | undefined; + const capture = jest.spyOn(FrontMcpInstance.prototype, 'getPrimaryScope').mockImplementation(function ( + this: FrontMcpInstance, + ) { + scope = getPrimaryScope.call(this); + return scope; + }); + await FrontMcpInstance.runStdio({ + info: { name: 'stdio-dispose', version: '1.0.0' }, + apps: [DisposeApp], + logging: { level: LogLevel.Off, enableConsole: false }, + }); + capture.mockRestore(); + if (!scope) throw new Error('runStdio served no scope'); + + const order: string[] = []; + scope.onDispose(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + order.push('slow'); + }); + scope.onDispose(() => { + order.push('fast'); + }); + const onSigterm = process.listeners('SIGTERM').find((listener) => !signalsBefore.includes(listener)); + + onSigterm?.('SIGTERM'); + const deadline = Date.now() + 5_000; + while (exit.mock.calls.length === 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 10)); + } + + expect(order).toEqual(['fast', 'slow']); + expect(mockServerClose).toHaveBeenCalledTimes(1); + expect(exit).toHaveBeenCalledWith(0); + }); +}); diff --git a/libs/sdk/src/front-mcp/__tests__/run-stdio-rollback-deadline.spec.ts b/libs/sdk/src/front-mcp/__tests__/run-stdio-rollback-deadline.spec.ts new file mode 100644 index 000000000..63d6d1275 --- /dev/null +++ b/libs/sdk/src/front-mcp/__tests__/run-stdio-rollback-deadline.spec.ts @@ -0,0 +1,105 @@ +/** + * A stdio start that fails after its scopes are live rolls them back within the stdio shutdown deadline: + * an `onDispose` callback that never settles cannot hold back the start error. + */ +import 'reflect-metadata'; + +import { App, LogLevel, Tool, ToolContext } from '../../common'; +import { FrontMcpInstance } from '../front-mcp'; +import { STDIO_SHUTDOWN_DEADLINE_MS } from '../shutdown-signals'; + +jest.mock('@frontmcp/protocol', () => { + const actual = jest.requireActual('@frontmcp/protocol'); + class RefusingMcpServer { + setRequestHandler() { + return undefined; + } + setNotificationHandler() { + return undefined; + } + async connect() { + throw new Error('connect refused'); + } + async close() { + return undefined; + } + } + class IdleStdioServerTransport { + async close() { + return undefined; + } + } + return { ...actual, McpServer: RefusingMcpServer, StdioServerTransport: IdleStdioServerTransport }; +}); + +@Tool({ name: 'noop', inputSchema: {} }) +class NoopTool extends ToolContext { + async execute() { + return {}; + } +} + +@App({ id: 'rollback', name: 'Rollback', tools: [NoopTool] }) +class RollbackApp {} + +function flush(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + +describe('runStdio startup rollback deadline', () => { + const savedConsole = { ...console }; + const savedStdioEnv = process.env['FRONTMCP_STDIO']; + const savedLogsMax = process.env['FRONTMCP_LOGS_MAX']; + + afterAll(() => { + jest.useRealTimers(); + Object.assign(console, savedConsole); + if (savedStdioEnv === undefined) delete process.env['FRONTMCP_STDIO']; + else process.env['FRONTMCP_STDIO'] = savedStdioEnv; + if (savedLogsMax === undefined) delete process.env['FRONTMCP_LOGS_MAX']; + else process.env['FRONTMCP_LOGS_MAX'] = savedLogsMax; + }); + + it('rejects with the start error at the deadline when an onDispose callback hangs', async () => { + process.env['FRONTMCP_LOGS_MAX'] = '0'; + let entered = false; + const getPrimaryScope = FrontMcpInstance.prototype.getPrimaryScope; + const capture = jest.spyOn(FrontMcpInstance.prototype, 'getPrimaryScope').mockImplementation(function ( + this: FrontMcpInstance, + ) { + const scope = getPrimaryScope.call(this); + scope?.onDispose( + () => + new Promise(() => { + entered = true; + }), + ); + jest.useFakeTimers({ doNotFake: ['setImmediate', 'nextTick', 'queueMicrotask'] }); + return scope; + }); + try { + let outcome: unknown; + const run = FrontMcpInstance.runStdio({ + info: { name: 'stdio-rollback', version: '1.0.0' }, + apps: [RollbackApp], + logging: { level: LogLevel.Off, enableConsole: false }, + }).catch((error: unknown) => { + outcome = error; + }); + + for (let i = 0; i < 200 && !entered; i++) await flush(); + expect(entered).toBe(true); + jest.advanceTimersByTime(STDIO_SHUTDOWN_DEADLINE_MS - 1); + await flush(); + expect(outcome).toBeUndefined(); + jest.advanceTimersByTime(1); + for (let i = 0; i < 20 && outcome === undefined; i++) await flush(); + + expect(outcome).toEqual(new Error('connect refused')); + await run; + } finally { + capture.mockRestore(); + jest.useRealTimers(); + } + }); +}); diff --git a/libs/sdk/src/front-mcp/front-mcp.ts b/libs/sdk/src/front-mcp/front-mcp.ts index 1df7e58e6..db4e7753c 100644 --- a/libs/sdk/src/front-mcp/front-mcp.ts +++ b/libs/sdk/src/front-mcp/front-mcp.ts @@ -28,7 +28,7 @@ import { computeTaskCapabilities } from '../task/task-scope.helper'; import type { FetchHandlerCtx, WebFetchHandler } from '../transport/web-fetch-handler'; import { markDirectOnly } from './direct-only.flag'; import { createMcpGlobalProviders } from './front-mcp.providers'; -import { DEFAULT_SHUTDOWN_DEADLINE_MS, exitOnShutdownSignals } from './shutdown-signals'; +import { DEFAULT_SHUTDOWN_DEADLINE_MS, exitOnShutdownSignals, STDIO_SHUTDOWN_DEADLINE_MS } from './shutdown-signals'; /** * A `@FrontMcp`-decorated server class, or the raw config object it wraps. @@ -88,21 +88,24 @@ export class FrontMcpInstance implements FrontMcpInterface { /** Shut down what a failed start built, for at most the signal path's deadline, then fail with its error. */ private async releaseFailedStart(error: unknown): Promise { + await this.releaseWithin(DEFAULT_SHUTDOWN_DEADLINE_MS); + throw error; + } + + /** Shut down what a failed start built, waiting at most `deadlineMs` for it. */ + private async releaseWithin(deadlineMs: number): Promise { let deadline: ReturnType | undefined; const outlasted = new Promise((resolve) => { - deadline = setTimeout(() => resolve(true), DEFAULT_SHUTDOWN_DEADLINE_MS); + deadline = setTimeout(() => resolve(true), deadlineMs); }); const released = this.shutdown().then( () => false, () => false, ); if (await Promise.race([released, outlasted])) { - this.log?.error( - `Releasing a server that failed to start did not finish within ${DEFAULT_SHUTDOWN_DEADLINE_MS} ms`, - ); + this.log?.error(`Releasing a server that failed to start did not finish within ${deadlineMs} ms`); } clearTimeout(deadline); - throw error; } async initialize(): Promise { @@ -154,6 +157,15 @@ export class FrontMcpInstance implements FrontMcpInterface { } } + /** Start, or shut down what was built when the start fails (a refused socket, a port in use), then rethrow. */ + private async startOrRelease(): Promise { + try { + await this.start(); + } catch (error) { + return this.releaseFailedStart(error); + } + } + /** Where this server serves MCP, as reported to the `frontmcp dev --stdio` bridge. */ private devListenerInfo(server: FrontMcpServer): { port?: number; socketPath?: string; path: string } { const http = (server as Partial).config; @@ -342,7 +354,7 @@ export class FrontMcpInstance implements FrontMcpInterface { const frontMcp = new FrontMcpInstance(parsedConfig); await frontMcp.ready; - await frontMcp.start(); + await frontMcp.startOrRelease(); exitOnShutdownSignals(() => frontMcp.shutdown(), { logger: frontMcp.log }); frontMcp.log?.info('FrontMCP bootstrap complete'); return frontMcp; @@ -656,7 +668,7 @@ export class FrontMcpInstance implements FrontMcpInterface { const frontMcp = new FrontMcpInstance(parsedConfig); await frontMcp.ready; - await frontMcp.start(); + await frontMcp.startOrRelease(); frontMcp.log?.info(`MCP server listening on unix://${socketPath}`); // The graceful shutdown of the TCP server (#712). Closing the server removes its socket file, and the next @@ -796,11 +808,7 @@ export class FrontMcpInstance implements FrontMcpInterface { await frontMcp.ready; // Scopes are now live (sqlite/redis/channels/jobs) — shut them down if a // later startup step throws. - startupCleanups.push(async () => { - for (const s of frontMcp.getScopes()) { - await (s as Scope).shutdown(); - } - }); + startupCleanups.push(() => frontMcp.releaseWithin(STDIO_SHUTDOWN_DEADLINE_MS)); // The scope holding the server's own apps, not a standalone app's const primaryScope = frontMcp.getPrimaryScope(); @@ -908,29 +916,19 @@ export class FrontMcpInstance implements FrontMcpInterface { scope.notifications.subscribeAllChannels(sessionId, channelNames); } - // Graceful shutdown: cleanup resources then exit with code 0. - // A ref'd timer keeps the event loop alive while async cleanup runs — - // without it, closing the transport removes the last ref (stdin) and the - // process exits via the raw signal before process.exit(0) is reached. - // This is the same pattern used by Express/Fastify for graceful shutdown. - let shuttingDown = false; - const shutdownHandler = async () => { - if (shuttingDown) return; // prevent re-entry from second signal - shuttingDown = true; - const deadline = setTimeout(() => process.exit(1), 5000); - try { + // The graceful shutdown of the HTTP and socket servers (#712): every scope is shut down, then + // disposed, so `onDispose` callbacks run under stdio too. + exitOnShutdownSignals( + async () => { scope.notifications.unregisterServer(sessionId); - await scope.shutdown(); - await mcpServer.close(); - } catch (err) { - console.error('Error closing MCP server:', err); - } finally { - clearTimeout(deadline); - } - process.exit(0); - }; - process.on('SIGINT', shutdownHandler); - process.on('SIGTERM', shutdownHandler); + try { + await frontMcp.shutdown(); + } finally { + await mcpServer.close(); + } + }, + { logger: frontMcp.log, deadlineMs: STDIO_SHUTDOWN_DEADLINE_MS }, + ); } finally { if (!startupSucceeded) { // Roll back resources the partial startup created (newest-first), then diff --git a/libs/sdk/src/front-mcp/shutdown-signals.ts b/libs/sdk/src/front-mcp/shutdown-signals.ts index 4de1fec67..3f724032c 100644 --- a/libs/sdk/src/front-mcp/shutdown-signals.ts +++ b/libs/sdk/src/front-mcp/shutdown-signals.ts @@ -2,6 +2,9 @@ const SHUTDOWN_SIGNALS = ['SIGTERM', 'SIGINT'] as const; export const DEFAULT_SHUTDOWN_DEADLINE_MS = 10_000; +/** A stdio client waits on the process to exit, so stdio keeps its shorter deadline. */ +export const STDIO_SHUTDOWN_DEADLINE_MS = 5_000; + interface SignalTarget { on(signal: string, listener: () => void): unknown; removeListener(signal: string, listener: () => void): unknown; diff --git a/libs/sdk/src/server/adapters/__tests__/express.host.adapter.spec.ts b/libs/sdk/src/server/adapters/__tests__/express.host.adapter.spec.ts index e5c60121c..8849700c6 100644 --- a/libs/sdk/src/server/adapters/__tests__/express.host.adapter.spec.ts +++ b/libs/sdk/src/server/adapters/__tests__/express.host.adapter.spec.ts @@ -689,6 +689,147 @@ describe('ExpressHostAdapter — derived host allow-list scope', () => { }); }); +/** + * Proposal 1.7: a loopback server checks `Origin` too. A page on another site can POST to + * 127.0.0.1 without any rebinding (the Host is a loopback name), and only its `Origin` tells it apart. + */ +describe('ExpressHostAdapter — derived Origin check on a loopback bind', () => { + type HostValidation = (req: never, res: never, next: () => void) => void; + + function check( + adapter: ExpressHostAdapter, + headers: Record, + ): 'allowed' | { status: number; body: unknown } { + const validate = (adapter as unknown as { hostValidation?: HostValidation }).hostValidation; + if (!validate) return 'allowed'; + let outcome: 'allowed' | { status: number; body: unknown } = 'allowed'; + const res = { status: (status: number) => ({ json: (body: unknown) => (outcome = { status, body }) }) }; + validate({ headers } as never, res as never, () => undefined); + return outcome; + } + + function listening(adapter: ExpressHostAdapter, port: number, bindAddress: string): ExpressHostAdapter { + (adapter as unknown as { enableDerivedHostValidation(p: number, b?: string): void }).enableDerivedHostValidation( + port, + bindAddress, + ); + return adapter; + } + + const host = 'localhost:3000'; + + it('refuses a request from another origin with the response an explicit allowedOrigins gives', () => { + const derived = listening(new ExpressHostAdapter(), 3000, '127.0.0.1'); + const explicit = new ExpressHostAdapter({ + security: { dnsRebindingProtection: { allowedOrigins: ['https://app.example.com'] } }, + }); + + const refused = check(derived, { host, origin: 'https://evil.example' }); + + expect(refused).toEqual({ status: 403, body: { error: 'Forbidden', message: 'Invalid Origin header' } }); + expect(check(explicit, { host, origin: 'https://evil.example' })).toEqual(refused); + }); + + it('allows a request with no Origin, and one from the server itself', () => { + const adapter = listening(new ExpressHostAdapter(), 3000, '127.0.0.1'); + + expect(check(adapter, { host })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://localhost:3000' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://127.0.0.1:3000' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://[::1]:3000' })).toBe('allowed'); + }); + + it('refuses Origin null and matches the spellings of its own origin', () => { + const adapter = listening(new ExpressHostAdapter(), 3000, '127.0.0.1'); + + expect(check(adapter, { host, origin: 'null' })).toMatchObject({ status: 403 }); + expect(check(adapter, { host, origin: 'HTTP://LOCALHOST:3000' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://[0:0:0:0:0:0:0:1]:3000' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'https://127.0.0.1:3000' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://127.0.0.2:3000' })).toMatchObject({ status: 403 }); + }); + + it('matches a default port only for its own scheme', () => { + const adapter = listening(new ExpressHostAdapter(), 80, '127.0.0.1'); + + expect(check(adapter, { host: 'localhost', origin: 'http://localhost' })).toBe('allowed'); + expect(check(adapter, { host: 'localhost', origin: 'https://localhost' })).toMatchObject({ status: 403 }); + }); + + it('refuses a loopback page served on another port', () => { + const adapter = listening(new ExpressHostAdapter(), 3000, '127.0.0.1'); + + expect(check(adapter, { host, origin: 'http://localhost:8080' })).toMatchObject({ status: 403 }); + }); + + it('allows the origins an explicit CORS list names', () => { + const adapter = listening(new ExpressHostAdapter({ cors: { origin: ['https://app.example.com'] } }), 3000, '::1'); + + expect(check(adapter, { host, origin: 'https://app.example.com' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'https://evil.example' })).toMatchObject({ status: 403 }); + }); + + it.each([true, '*', () => undefined])('checks no Origin when CORS admits any origin (%p)', (origin) => { + const adapter = listening(new ExpressHostAdapter({ cors: { origin: origin as never } }), 3000, '127.0.0.1'); + + expect(check(adapter, { host, origin: 'https://evil.example' })).toBe('allowed'); + }); + + it('lets an explicit allowedOrigins win over the derived list', () => { + const adapter = listening( + new ExpressHostAdapter({ security: { dnsRebindingProtection: { allowedOrigins: ['https://app.example.com'] } } }), + 3000, + '127.0.0.1', + ); + + expect(check(adapter, { host, origin: 'https://app.example.com' })).toBe('allowed'); + expect(check(adapter, { host, origin: 'http://localhost:3000' })).toMatchObject({ status: 403 }); + }); + + it('leaves Origin to the operator when the Host list is explicit', () => { + const adapter = listening( + new ExpressHostAdapter({ security: { dnsRebindingProtection: { allowedHosts: [host] } } }), + 3000, + '127.0.0.1', + ); + + expect(check(adapter, { host, origin: 'https://evil.example' })).toBe('allowed'); + }); + + it('checks no Origin on a routable bind', () => { + const adapter = listening( + new ExpressHostAdapter({ listen: { issuer: 'https://mcp.example.com' } }), + 3000, + '0.0.0.0', + ); + + expect(check(adapter, { host: 'mcp.example.com', origin: 'https://evil.example' })).toBe('allowed'); + }); +}); + +describe('ExpressHostAdapter.start() bind fallback', () => { + it('listens on loopback when called without an address', async () => { + const opened: http.Server[] = []; + const realCreateServer = http.createServer.bind(http); + const spy = jest.spyOn(http, 'createServer').mockImplementation(((...args: never[]) => { + const created = (realCreateServer as (...a: never[]) => http.Server)(...args); + opened.push(created); + return created; + }) as never); + const log = jest.spyOn(console, 'log').mockImplementation(() => undefined); + try { + const adapter = new ExpressHostAdapter(); + await adapter.start(0); + + expect((opened[0].address() as import('node:net').AddressInfo).address).toBe('127.0.0.1'); + await adapter.stop(0); + } finally { + spy.mockRestore(); + log.mockRestore(); + } + }); +}); + describe('ExpressHostAdapter.stop() (#712)', () => { async function startAdapter(): Promise<{ adapter: ExpressHostAdapter; server: http.Server }> { const opened: http.Server[] = []; diff --git a/libs/sdk/src/server/adapters/__tests__/express.host.adapter.unix-socket.spec.ts b/libs/sdk/src/server/adapters/__tests__/express.host.adapter.unix-socket.spec.ts new file mode 100644 index 000000000..776d57289 --- /dev/null +++ b/libs/sdk/src/server/adapters/__tests__/express.host.adapter.unix-socket.spec.ts @@ -0,0 +1,358 @@ +/** + * Proposal 1.7: a Unix socket is the only boundary a socket server has, so it is created owner-only + * with no window, in a directory nobody else can write to, and a start never removes a live socket, + * a file that is not a socket, or another user's socket. + */ +import { spawnSync } from 'node:child_process'; +import cluster from 'node:cluster'; +import * as fs from 'node:fs'; +import * as http from 'node:http'; +import * as net from 'node:net'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import * as utils from '@frontmcp/utils'; + +import { + SocketInUseError, + SocketPathConflictError, + SocketPermissionError, + UnsafeSocketDirectoryError, +} from '../../../errors/transport.errors'; +import { ExpressHostAdapter } from '../express.host.adapter'; + +jest.mock('../base.host.adapter', () => { + class HostServerAdapter {} + return { HostServerAdapter }; +}); + +jest.mock('@frontmcp/utils', () => { + const actual = jest.requireActual('@frontmcp/utils'); + return { ...actual, chmod: jest.fn(actual.chmod), lstat: jest.fn(actual.lstat), stat: jest.fn(actual.stat) }; +}); + +const actualUtils = jest.requireActual('@frontmcp/utils'); +const chmodMock = utils.chmod as jest.MockedFunction; +const lstatMock = utils.lstat as jest.MockedFunction; +const statMock = utils.stat as jest.MockedFunction; + +const posixIt = process.platform === 'win32' ? it.skip : it; + +function health(socketPath: string): Promise { + return new Promise((resolve, reject) => { + http + .get({ socketPath, path: '/health', agent: false }, (res) => { + res.resume(); + resolve(res.statusCode ?? 0); + }) + .once('error', reject); + }); +} + +function leaveStaleSocket(socketPath: string): void { + const script = `require('net').createServer().listen(${JSON.stringify(socketPath)}, () => process.kill(process.pid, 'SIGKILL'))`; + spawnSync(process.execPath, ['-e', script]); +} + +describe('ExpressHostAdapter on a Unix socket', () => { + let directory: string; + let socketPath: string; + let adapter: ExpressHostAdapter | undefined; + let log: jest.SpyInstance; + + beforeEach(() => { + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'fmcp-')); + socketPath = path.join(directory, 's.sock'); + chmodMock.mockClear(); + log = jest.spyOn(console, 'log').mockImplementation(() => undefined); + }); + + afterEach(async () => { + await adapter?.stop(0); + adapter = undefined; + log.mockRestore(); + chmodMock.mockImplementation(actualUtils.chmod); + lstatMock.mockImplementation(actualUtils.lstat); + statMock.mockImplementation(actualUtils.stat); + fs.rmSync(directory, { recursive: true, force: true }); + }); + + async function start(): Promise { + const started = new ExpressHostAdapter(); + started.registerRoute('GET', '/health', (_req, res) => res.status(200).json({ ok: true })); + await started.start(socketPath); + adapter = started; + return started; + } + + posixIt('creates the socket owner-only (0600)', async () => { + await start(); + + expect(fs.lstatSync(socketPath).mode & 0o777).toBe(0o600); + expect(await health(socketPath)).toBe(200); + }); + + posixIt('binds owner-only before any chmod, so there is no window', async () => { + chmodMock.mockResolvedValue(undefined); + lstatMock.mockImplementation(async (p) => { + const stats = await actualUtils.lstat(p); + return p === socketPath ? Object.assign(stats, { mode: (stats.mode & ~0o777) | 0o600 }) : stats; + }); + + await start(); + + expect(fs.lstatSync(socketPath).mode & 0o077).toBe(0); + }); + + posixIt('restores the process umask after binding', async () => { + const before = process.umask(); + + await start(); + + expect(process.umask()).toBe(before); + }); + + posixIt('fails startup and closes the listener when chmod fails', async () => { + chmodMock.mockRejectedValue(Object.assign(new Error('EPERM'), { code: 'EPERM' })); + + await expect(start()).rejects.toBeInstanceOf(SocketPermissionError); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('fails startup when the socket does not end up 0600', async () => { + lstatMock.mockImplementation(async (p) => { + const stats = await actualUtils.lstat(p); + return Object.assign(stats, { mode: (stats.mode & ~0o777) | 0o660 }); + }); + + await expect(start()).rejects.toThrow(/0660/); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('refuses a directory the group or others can write, naming the path and the mode', async () => { + fs.chmodSync(directory, 0o777); + + const failure = start(); + + await expect(failure).rejects.toBeInstanceOf(UnsafeSocketDirectoryError); + await expect(failure).rejects.toThrow(directory); + await expect(failure).rejects.toThrow('0777'); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('accepts a world-writable directory with the sticky bit, as /tmp is', async () => { + fs.chmodSync(directory, 0o1777); + + await start(); + + expect(await health(socketPath)).toBe(200); + }); + + function ownDirectoryAs(uid: number): void { + statMock.mockImplementation(async (p) => { + const stats = await actualUtils.stat(p); + return p === directory ? Object.assign(stats, { uid }) : stats; + }); + } + + posixIt('refuses a sticky directory another user owns, since its owner can replace the socket', async () => { + fs.chmodSync(directory, 0o1777); + ownDirectoryAs(fs.statSync(directory).uid + 1); + + const failure = start(); + + await expect(failure).rejects.toBeInstanceOf(UnsafeSocketDirectoryError); + await expect(failure).rejects.toThrow('another user'); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('accepts a sticky directory root owns, as /tmp is', async () => { + fs.chmodSync(directory, 0o1777); + ownDirectoryAs(0); + + await start(); + + expect(await health(socketPath)).toBe(200); + }); + + function nestSocketIn(name: string): string { + const nested = path.join(directory, name); + fs.mkdirSync(nested, { mode: 0o700 }); + socketPath = path.join(nested, 's.sock'); + return nested; + } + + posixIt('refuses a socket whose parent directory the group or others can write, naming that directory', async () => { + nestSocketIn('private'); + fs.chmodSync(directory, 0o777); + + const failure = start(); + + await expect(failure).rejects.toBeInstanceOf(UnsafeSocketDirectoryError); + await expect(failure).rejects.toThrow(`under ${directory} (mode 0777)`); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('refuses a socket whose parent directory another user owns', async () => { + nestSocketIn('private'); + ownDirectoryAs(fs.statSync(directory).uid + 1); + + await expect(start()).rejects.toThrow('another user'); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('refuses a socket reached through a symlink whose target sits in a directory others can write', async () => { + const shared = path.join(directory, 'shared'); + fs.mkdirSync(path.join(shared, 'socks'), { recursive: true, mode: 0o700 }); + fs.chmodSync(shared, 0o777); + fs.symlinkSync(path.join(shared, 'socks'), path.join(directory, 'link')); + socketPath = path.join(directory, 'link', 's.sock'); + + const failure = start(); + + await expect(failure).rejects.toBeInstanceOf(UnsafeSocketDirectoryError); + await expect(failure).rejects.toThrow(`under ${path.join(fs.realpathSync(directory), 'shared')} (mode 0777)`); + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('replaces a stale socket a crashed server left behind', async () => { + leaveStaleSocket(socketPath); + expect(fs.lstatSync(socketPath).isSocket()).toBe(true); + + await start(); + + expect(await health(socketPath)).toBe(200); + }); + + posixIt('refuses to take over a live socket, and the server on it keeps serving', async () => { + const first = net.createServer((socket) => { + socket.on('error', () => undefined); + socket.end('first'); + }); + await new Promise((resolve) => first.listen(socketPath, resolve)); + try { + await expect(start()).rejects.toBeInstanceOf(SocketInUseError); + + const reply = await new Promise((resolve) => { + const client = net.connect(socketPath); + let data = ''; + client.on('data', (chunk) => (data += chunk)); + client.on('end', () => resolve(data)); + }); + expect(reply).toBe('first'); + } finally { + await new Promise((resolve) => first.close(() => resolve())); + } + }); + + posixIt('refuses to remove a regular file at the path', async () => { + fs.writeFileSync(socketPath, 'keep me'); + + await expect(start()).rejects.toBeInstanceOf(SocketPathConflictError); + expect(fs.readFileSync(socketPath, 'utf8')).toBe('keep me'); + }); + + posixIt("refuses to remove another user's socket", async () => { + leaveStaleSocket(socketPath); + lstatMock.mockImplementation(async (p) => { + const stats = await actualUtils.lstat(p); + return Object.assign(stats, { uid: stats.uid + 1 }); + }); + + await expect(start()).rejects.toThrow(/another user/); + expect(fs.existsSync(socketPath)).toBe(true); + }); + + posixIt('reports a probe failure other than a refused connection instead of unlinking', async () => { + leaveStaleSocket(socketPath); + const connect = jest.spyOn(net, 'connect').mockImplementation(() => { + const socket = new net.Socket(); + process.nextTick(() => socket.emit('error', Object.assign(new Error('EACCES'), { code: 'EACCES' }))); + return socket; + }); + try { + await expect(start()).rejects.toThrow(/EACCES/); + expect(fs.existsSync(socketPath)).toBe(true); + } finally { + connect.mockRestore(); + } + }); + + posixIt('still ends up 0600 where the umask cannot be set (a worker thread)', async () => { + const umask = jest.spyOn(process, 'umask').mockImplementation(((mask?: number) => { + if (mask !== undefined) throw new Error('ERR_WORKER_UNSUPPORTED_OPERATION'); + return 0o022; + }) as typeof process.umask); + try { + await start(); + } finally { + umask.mockRestore(); + } + + expect(fs.lstatSync(socketPath).mode & 0o777).toBe(0o600); + }); + + posixIt('refuses a directory others can enter where the umask cannot be set, before binding', async () => { + fs.chmodSync(directory, 0o755); + const umask = jest.spyOn(process, 'umask').mockImplementation(((mask?: number) => { + if (mask !== undefined) throw new Error('ERR_WORKER_UNSUPPORTED_OPERATION'); + return 0o022; + }) as typeof process.umask); + try { + const failure = start(); + + await expect(failure).rejects.toBeInstanceOf(SocketPermissionError); + await expect(failure).rejects.toThrow('0755'); + } finally { + umask.mockRestore(); + } + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('refuses a directory others can enter in a cluster worker, whose socket the primary binds', async () => { + fs.chmodSync(directory, 0o755); + const isWorker = Object.getOwnPropertyDescriptor(cluster, 'isWorker'); + Object.defineProperty(cluster, 'isWorker', { value: true, configurable: true }); + try { + await expect(start()).rejects.toBeInstanceOf(SocketPermissionError); + } finally { + if (isWorker) Object.defineProperty(cluster, 'isWorker', isWorker); + } + expect(fs.existsSync(socketPath)).toBe(false); + }); + + posixIt('goes on when the stale socket disappears before the probe', async () => { + leaveStaleSocket(socketPath); + const connect = jest.spyOn(net, 'connect').mockImplementation(() => { + const socket = new net.Socket(); + fs.unlinkSync(socketPath); + process.nextTick(() => socket.emit('error', Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))); + return socket; + }); + try { + await start(); + } finally { + connect.mockRestore(); + } + + expect(await health(socketPath)).toBe(200); + }); + + posixIt('reports an lstat failure other than a missing path', async () => { + lstatMock.mockRejectedValue(Object.assign(new Error('EACCES'), { code: 'EACCES' })); + + await expect(start()).rejects.toThrow('EACCES'); + }); + + it('applies none of this to a Windows named pipe', async () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { value: 'win32' }); + fs.chmodSync(directory, 0o777); + try { + await start(); + expect(chmodMock).not.toHaveBeenCalled(); + } finally { + if (platform) Object.defineProperty(process, 'platform', platform); + } + }); +}); diff --git a/libs/sdk/src/server/adapters/express.host.adapter.ts b/libs/sdk/src/server/adapters/express.host.adapter.ts index f00bcfcaa..1dbb10a49 100644 --- a/libs/sdk/src/server/adapters/express.host.adapter.ts +++ b/libs/sdk/src/server/adapters/express.host.adapter.ts @@ -4,8 +4,6 @@ import * as http from 'node:http'; import cors from 'cors'; import express from 'express'; -import { fileExists, unlink } from '@frontmcp/utils'; - import { type CorsOptions, type HttpMethod, @@ -18,7 +16,21 @@ import { PayloadTooLargeError } from '../../errors/mcp.error'; import { applyMachineIdHeader } from '../../ha/ha-headers'; import { resolveSecurityHeaders } from '../middleware/csp.middleware'; import { createHostValidationMiddleware } from '../middleware/host-validation.middleware'; -import { allowedHostsFromEnv, deriveAllowedHosts, shouldEnforceDerivedHosts } from '../security/resolve-allowed-hosts'; +import { + allowedHostsFromEnv, + corsAllowedOrigins, + deriveAllowedHosts, + deriveAllowedOrigins, + isLoopbackAddress, + shouldEnforceDerivedHosts, +} from '../security/resolve-allowed-hosts'; +import { + assertSafeSocketDirectory, + clearStaleSocket, + enforceOwnerOnly, + hasSocketFileMode, + listenOwnerOnly, +} from '../security/unix-socket-guard'; import { HostServerAdapter } from './base.host.adapter'; const SERVER_DRAIN_MS = 5_000; @@ -195,7 +207,8 @@ export class ExpressHostAdapter extends HostServerAdapter { * * ON by default since v1.7.2 (BC-035). `allowedHosts` falls back to the names * this process actually answers to — the loopback aliases with and without - * the bound port, the bound NIC address, and the configured issuer host. + * the bound port, the bound NIC address, and the configured issuer host. On a + * loopback bind `allowedOrigins` falls back to its own origins and the CORS list. * * Escape hatches, in precedence order: explicit * `security.dnsRebindingProtection.allowedHosts`, the @@ -236,8 +249,8 @@ export class ExpressHostAdapter extends HostServerAdapter { // derive from, and therefore no protection on exactly the loopback // listener DNS rebinding targets. const derivation = { - // `start()` binds `bindAddress ?? '0.0.0.0'`; mirror that exactly. - bindAddress: bindAddress ?? '0.0.0.0', + // `start()` binds `bindAddress ?? '127.0.0.1'`; mirror that exactly. + bindAddress: bindAddress ?? '127.0.0.1', port: portOrSocketPath, issuer: options?.listen?.issuer, }; @@ -260,7 +273,14 @@ export class ExpressHostAdapter extends HostServerAdapter { // skip it rather than fail closed on a valid config. if (!allowedHosts.length) return undefined; - return createHostValidationMiddleware({ enabled: true, allowedHosts }); + // A page on another site reaches a loopback server under a loopback Host; only its Origin differs. + const corsOrigins = corsAllowedOrigins(options?.cors); + const allowedOrigins = + isLoopbackAddress(derivation.bindAddress) && corsOrigins + ? deriveAllowedOrigins({ port: derivation.port, issuer: derivation.issuer, extraOrigins: corsOrigins }) + : undefined; + + return createHostValidationMiddleware({ enabled: true, allowedHosts, allowedOrigins }); }; } @@ -329,26 +349,28 @@ export class ExpressHostAdapter extends HostServerAdapter { server.keepAliveTimeout = 75_000; if (typeof portOrSocketPath === 'string') { - // Unix socket mode - clean up stale socket file before listening - await this.cleanupStaleSocket(portOrSocketPath); + const ownerOnly = hasSocketFileMode(); + if (ownerOnly) { + await assertSafeSocketDirectory(portOrSocketPath); + await clearStaleSocket(portOrSocketPath); + } this.enableDerivedHostValidation(portOrSocketPath); await new Promise((resolve, reject) => { server.on('error', reject); - server.listen(portOrSocketPath, () => { - // Set socket file permissions (owner + group read/write) - // Using node:fs chmodSync directly - no chmod equivalent in @frontmcp/utils - try { - const fs = require('node:fs'); - fs.chmodSync(portOrSocketPath, 0o660); - } catch { - // chmod may fail on some platforms, non-critical - } - console.log(`MCP HTTP (Express) on unix://${portOrSocketPath}`); - resolve(); - }); + if (ownerOnly) listenOwnerOnly(server, portOrSocketPath, resolve); + else server.listen(portOrSocketPath, resolve); }); + if (ownerOnly) { + try { + await enforceOwnerOnly(portOrSocketPath); + } catch (error) { + await this.stop(0); + throw error; + } + } + console.log(`MCP HTTP (Express) on unix://${portOrSocketPath}`); } else { - const host = bindAddress ?? '0.0.0.0'; + const host = bindAddress ?? '127.0.0.1'; await new Promise((resolve, reject) => { server.on('error', reject); server.listen(portOrSocketPath, host, () => { @@ -385,14 +407,4 @@ export class ExpressHostAdapter extends HostServerAdapter { clearTimeout(forceClose); } } - - private async cleanupStaleSocket(socketPath: string): Promise { - try { - if (await fileExists(socketPath)) { - await unlink(socketPath); - } - } catch { - // Ignore cleanup errors - listen will fail if socket is still in use - } - } } diff --git a/libs/sdk/src/server/security/__tests__/resolve-allowed-hosts.spec.ts b/libs/sdk/src/server/security/__tests__/resolve-allowed-hosts.spec.ts index 95fea335d..13cfd08ac 100644 --- a/libs/sdk/src/server/security/__tests__/resolve-allowed-hosts.spec.ts +++ b/libs/sdk/src/server/security/__tests__/resolve-allowed-hosts.spec.ts @@ -10,7 +10,9 @@ */ import { allowedHostsFromEnv, + corsAllowedOrigins, deriveAllowedHosts, + deriveAllowedOrigins, isLoopbackAddress, isRoutableBind, shouldEnforceDerivedHosts, @@ -164,3 +166,50 @@ describe('allowedHostsFromEnv', () => { expect(allowedHostsFromEnv({ FRONTMCP_ALLOWED_HOSTS: ' , , ' })).toBeUndefined(); }); }); + +describe('deriveAllowedOrigins', () => { + it('names http and https on every loopback spelling with the bound port', () => { + expect(deriveAllowedOrigins({ port: 3000 })).toEqual([ + 'http://localhost:3000', + 'http://127.0.0.1:3000', + 'http://[::1]:3000', + 'https://localhost:3000', + 'https://127.0.0.1:3000', + 'https://[::1]:3000', + ]); + }); + + it('adds the issuer origin and the extra origins', () => { + expect( + deriveAllowedOrigins({ + port: 3000, + issuer: 'https://mcp.example.com/base', + extraOrigins: [' https://app.example.com ', ''], + }), + ).toEqual(expect.arrayContaining(['https://mcp.example.com', 'https://app.example.com'])); + }); + + it('ignores an unparseable issuer', () => { + expect(deriveAllowedOrigins({ port: 3000, issuer: 'not a url' })).toHaveLength(6); + }); +}); + +describe('corsAllowedOrigins', () => { + it('admits no extra origin when CORS is off', () => { + expect(corsAllowedOrigins(undefined)).toEqual([]); + expect(corsAllowedOrigins(false)).toEqual([]); + expect(corsAllowedOrigins({ origin: false })).toEqual([]); + }); + + it('lists the origins a string or an array names', () => { + expect(corsAllowedOrigins({ origin: 'https://app.example.com' })).toEqual(['https://app.example.com']); + expect(corsAllowedOrigins({ origin: ['https://a.example', 'https://b.example'] })).toEqual([ + 'https://a.example', + 'https://b.example', + ]); + }); + + it.each([true, '*', ['https://a.example', '*'], () => undefined])('admits any origin for %p', (origin) => { + expect(corsAllowedOrigins({ origin: origin as never })).toBeUndefined(); + }); +}); diff --git a/libs/sdk/src/server/security/__tests__/security-audit.spec.ts b/libs/sdk/src/server/security/__tests__/security-audit.spec.ts index c67feb009..0b6337d0b 100644 --- a/libs/sdk/src/server/security/__tests__/security-audit.spec.ts +++ b/libs/sdk/src/server/security/__tests__/security-audit.spec.ts @@ -160,6 +160,13 @@ describe('auditSecurityDefaults()', () => { const finding = dnsFinding({ resolvedBindAddress: '127.0.0.1' }); expect(finding?.code).toBe('DNS_REBINDING_PROTECTED'); expect(finding?.message).toContain('loopback'); + expect(finding?.message).toContain('Host and Origin headers are checked'); + }); + + it('says Origin is not checked on a loopback bind when CORS admits every origin', () => { + delete process.env['FRONTMCP_ALLOWED_HOSTS']; + const finding = dnsFinding({ resolvedBindAddress: '127.0.0.1', cors: { origin: true } }); + expect(finding?.message).toContain('Origin is not checked'); }); it('says a Unix socket has no Host to check', () => { diff --git a/libs/sdk/src/server/security/resolve-allowed-hosts.ts b/libs/sdk/src/server/security/resolve-allowed-hosts.ts index 711988300..3103ea4d2 100644 --- a/libs/sdk/src/server/security/resolve-allowed-hosts.ts +++ b/libs/sdk/src/server/security/resolve-allowed-hosts.ts @@ -7,6 +7,8 @@ * binds: the names a client can legitimately use to reach *this* process. */ +import type { CorsOptions } from '../../common/types/options/http/interfaces'; + /** Loopback spellings a local client may use, in `Host`-header form. */ const LOOPBACK_NAMES = ['localhost', '127.0.0.1', '[::1]']; @@ -138,6 +140,53 @@ export function deriveAllowedHosts(input: DeriveAllowedHostsInput): string[] { return [...hosts]; } +/** The origins an explicit CORS config admits, or `undefined` when it admits any origin. */ +export function corsAllowedOrigins(cors: CorsOptions | false | undefined): string[] | undefined { + const origin = cors === false ? undefined : cors?.origin; + if (origin === undefined || origin === false) return []; + if (typeof origin === 'string') return origin === '*' ? undefined : [origin]; + if (Array.isArray(origin)) return origin.includes('*') ? undefined : origin; + return undefined; +} + +export interface DeriveAllowedOriginsInput { + /** Listening TCP port. */ + port?: number; + /** Configured issuer / public URL; its origin is the server's own. */ + issuer?: string; + /** Origins the CORS config admits. */ + extraOrigins?: string[]; +} + +/** + * Derive the `Origin` values a loopback-bound server accepts: its own pages under every loopback + * spelling, its issuer, and the origins CORS admits. A request with no `Origin` is never checked. + */ +export function deriveAllowedOrigins(input: DeriveAllowedOriginsInput): string[] { + const origins = new Set(); + const port = input.port ? `:${input.port}` : ''; + for (const scheme of ['http', 'https']) { + for (const name of LOOPBACK_NAMES) origins.add(`${scheme}://${name}${port}`); + } + const issuerOrigin = originFromUrl(input.issuer); + if (issuerOrigin) origins.add(issuerOrigin); + for (const extra of input.extraOrigins ?? []) { + const trimmed = extra.trim(); + if (trimmed) origins.add(trimmed); + } + return [...origins]; +} + +function originFromUrl(url: string | undefined): string | undefined { + if (!url) return undefined; + try { + const { origin } = new URL(url); + return origin === 'null' ? undefined : origin; + } catch { + return undefined; + } +} + /** * Whether a DERIVED (not operator-supplied) host list may be enforced. * diff --git a/libs/sdk/src/server/security/security-audit.ts b/libs/sdk/src/server/security/security-audit.ts index 8566206f9..44afaaf21 100644 --- a/libs/sdk/src/server/security/security-audit.ts +++ b/libs/sdk/src/server/security/security-audit.ts @@ -9,7 +9,7 @@ import type { CorsOptions } from '../../common'; import { resolveHttpCors } from '../middleware/cors-env'; -import { allowedHostsFromEnv, isRoutableBind } from './resolve-allowed-hosts'; +import { allowedHostsFromEnv, corsAllowedOrigins, isRoutableBind } from './resolve-allowed-hosts'; /** * Security configuration for the audit. @@ -81,8 +81,8 @@ export function auditSecurityDefaults(config: SecurityAuditConfig, isProduction: level: 'info', code: 'CORS_DISABLED', message: - 'No CORS headers are sent. Cross-origin requests still reach the server — a browser just ' + - 'will not let the calling page read the response. CORS is not server-side access control.', + 'No CORS headers are sent, so a browser will not let a page on another origin read the response. ' + + 'CORS is not server-side access control: on a routable bind, cross-origin requests still reach the server.', }); } else if (corsOrigin === true) { findings.push({ @@ -195,7 +195,11 @@ function auditDnsRebinding(config: SecurityAuditConfig, bindAddress: string): Se return { level: 'info', code: 'DNS_REBINDING_PROTECTED', - message: `DNS rebinding protection is enabled: Host headers are checked against the loopback names for ${bindAddress}.`, + message: corsAllowedOrigins(resolveHttpCors(config.cors)) + ? `DNS rebinding protection is enabled: Host and Origin headers are checked against the loopback names for ${bindAddress}. ` + + 'A request with no Origin header is allowed.' + : `DNS rebinding protection is enabled: Host headers are checked against the loopback names for ${bindAddress}. ` + + 'Origin is not checked: CORS admits every origin.', }; } diff --git a/libs/sdk/src/server/security/unix-socket-guard.ts b/libs/sdk/src/server/security/unix-socket-guard.ts new file mode 100644 index 000000000..a00ca0ddc --- /dev/null +++ b/libs/sdk/src/server/security/unix-socket-guard.ts @@ -0,0 +1,130 @@ +// A socket server has no Host or Origin to check: the socket file's permissions are its only boundary. +import cluster from 'node:cluster'; +import type * as http from 'node:http'; +import * as net from 'node:net'; +import * as path from 'node:path'; + +import { chmod, lstat, realpath, stat, unlink } from '@frontmcp/utils'; + +import { + SocketInUseError, + SocketPathConflictError, + SocketPermissionError, + UnsafeSocketDirectoryError, +} from '../../errors/transport.errors'; + +const OWNER_ONLY_MODE = 0o600; +// Clears group and other bits only: the umask is process-wide, so a directory created meanwhile stays usable. +const OWNER_ONLY_UMASK = 0o077; +const GROUP_OR_OTHER_WRITE = 0o022; +const GROUP_OR_OTHER_ACCESS = 0o077; +const STICKY_BIT = 0o1000; +const ROOT_UID = 0; + +/** A win32 socket path names a named pipe, which has no file mode. */ +export function hasSocketFileMode(): boolean { + return process.platform !== 'win32'; +} + +function errorCode(error: unknown): string | undefined { + return (error as NodeJS.ErrnoException | undefined)?.code; +} + +/** Refuse a socket another user could replace through its directory or any above it, or reach before it is 0600. */ +export async function assertSafeSocketDirectory(socketPath: string): Promise { + const socketDirectory = path.dirname(path.resolve(socketPath)); + const realSocketDirectory = await realpath(socketDirectory); + await assertNoOtherUserCanReplace(socketDirectory); + if (realSocketDirectory !== socketDirectory) await assertNoOtherUserCanReplace(realSocketDirectory); + if (canBindOwnerOnly()) return; + const { mode } = await stat(socketDirectory); + if (mode & GROUP_OR_OTHER_ACCESS) { + throw new SocketPermissionError( + socketPath, + `the umask cannot be set here (a worker thread, or a cluster worker whose primary binds the socket), so its ` + + `directory ${socketDirectory} must be owner-only (0700), not 0${(mode & 0o777).toString(8)}`, + ); + } +} + +/** Refuse a directory, or one above it, that another user (not root) owns or the group or others can write without the sticky bit. */ +async function assertNoOtherUserCanReplace(start: string): Promise { + for (let directory = start; ; directory = path.dirname(directory)) { + const { mode, uid } = await stat(directory); + if (uid !== ROOT_UID && uid !== process.getuid?.()) throw new UnsafeSocketDirectoryError(directory, mode, uid); + if (mode & GROUP_OR_OTHER_WRITE && !(mode & STICKY_BIT)) throw new UnsafeSocketDirectoryError(directory, mode); + if (path.dirname(directory) === directory) return; + } +} + +/** Whether this thread binds the socket under the umask it sets: a worker thread cannot set it, and a cluster primary binds a worker's socket. */ +function canBindOwnerOnly(): boolean { + if (cluster.isWorker) return false; + const previousUmask = setUmask(OWNER_ONLY_UMASK); + if (previousUmask === undefined) return false; + setUmask(previousUmask); + return true; +} + +function probe(socketPath: string): Promise<'live' | 'stale' | 'gone'> { + return new Promise((resolve, reject) => { + const socket = net.connect(socketPath); + socket.once('connect', () => { + socket.destroy(); + resolve('live'); + }); + socket.once('error', (error) => { + const code = errorCode(error); + if (code === 'ECONNREFUSED') resolve('stale'); + else if (code === 'ENOENT') resolve('gone'); + else reject(new SocketPathConflictError(socketPath, `connecting to it failed (${code ?? error.message})`)); + }); + }); +} + +/** Remove a socket a crashed server left behind, and refuse anything else at the path. */ +export async function clearStaleSocket(socketPath: string): Promise { + let stats: Awaited>; + try { + stats = await lstat(socketPath); + } catch (error) { + if (errorCode(error) === 'ENOENT') return; + throw error; + } + if (!stats.isSocket()) throw new SocketPathConflictError(socketPath, 'it is not a socket'); + if (stats.uid !== process.getuid?.()) { + throw new SocketPathConflictError(socketPath, `it belongs to another user (uid ${stats.uid})`); + } + const state = await probe(socketPath); + if (state === 'live') throw new SocketInUseError(socketPath); + if (state === 'stale') await unlink(socketPath); +} + +/** Listen with an owner-only umask, so the socket is never reachable by others, even briefly. */ +export function listenOwnerOnly(server: http.Server, socketPath: string, onListening: () => void): void { + const previousUmask = setUmask(OWNER_ONLY_UMASK); + try { + server.listen(socketPath, onListening); + } finally { + if (previousUmask !== undefined) setUmask(previousUmask); + } +} + +function setUmask(mask: number): number | undefined { + try { + return process.umask(mask); + } catch { + return undefined; + } +} + +/** Set the socket to 0600 and check that it took. */ +export async function enforceOwnerOnly(socketPath: string): Promise { + try { + await chmod(socketPath, OWNER_ONLY_MODE); + } catch (error) { + throw new SocketPermissionError(socketPath, `chmod failed (${errorCode(error) ?? String(error)})`); + } + const mode = (await lstat(socketPath)).mode & 0o777; + if (mode !== OWNER_ONLY_MODE) throw new SocketPermissionError(socketPath, `its mode is 0${mode.toString(8)}`); +} diff --git a/libs/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index 8318d6927..dc2f5d101 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -134,6 +134,27 @@ treats `host` and `host:80`/`host:443` as equal. A loopback listener is reachabl names, so the derived list is exact and is enforced as-is; a routable bind enforces nothing derived until you name the public host, and the bound NIC address then joins the list alongside it. +**A loopback bind checks `Origin` too.** A page on another site can POST to `127.0.0.1` without +any rebinding — the `Host` is then a loopback name, and only the page's `Origin` gives it away. So +with no `allowedOrigins` configured, a loopback server also derives the origins it accepts: +`http://` and `https://` on `localhost`, `127.0.0.1` and `[::1]` with the bound port, the issuer's +origin, and the origins your `cors.origin` list names. A request from any other origin gets the +same `403 {"error":"Forbidden","message":"Invalid Origin header"}` an explicit list gives. A request +with **no** `Origin` (CLI and agent clients) is still served. `cors: { origin: true }` (or `'*'`, or +an origin function) admits every origin, so no `Origin` is checked then. An explicit +`allowedOrigins` replaces the derived list. An explicit `allowedHosts` (or `FRONTMCP_ALLOWED_HOSTS`) +replaces the derived protection as a whole, `Origin` included, so set `allowedOrigins` next to it to +keep checking `Origin`. A routable bind derives no origins. On a loopback server, a page on another +origin also gets `403` for the server card, which otherwise answers any origin. + +```typescript +// A browser app on another local port talks to a loopback server: name it in CORS. +http: { + port: 3000, + cors: { origin: ['http://localhost:5173'] }, +} +``` + **Deployments behind a proxy need one line of config.** A routable bind (`0.0.0.0`, `::`, a specific NIC) is reached under a hostname the process cannot know, so a derived list is not enforced there — FrontMCP logs a warning and leaves host checking off rather than 403-ing a proxied deployment on a @@ -169,16 +190,19 @@ To turn it off entirely: `dnsRebindingProtection: { enabled: false }`. It wins o turn protection back on. A request with **no** `Origin` header is allowed through — non-browser clients never send one, and a -rebound page always does. Rejecting the absent case breaks every CLI client and adds nothing. +browser sends one on every cross-site `POST`. Rejecting the absent case breaks every CLI client and adds nothing. ## CORS Configuration ### No CORS Headers (Default) -When `cors` is not specified, the server sends **no CORS headers**. A cross-origin request still -reaches the server and is served normally — the browser simply refuses to let the calling page read -the response. Non-browser clients are unaffected: CORS is a browser rule, **not server-side access -control**. If you need to keep callers out, use authentication. +When `cors` is not specified, the server sends **no CORS headers** (except on the public server card, +`/server-card`, which answers any origin). On a loopback bind, the derived +`Origin` check ([DNS Rebinding Protection](#dns-rebinding-protection)) refuses a request from another +site with `403`. Otherwise a cross-origin request reaches the server and is served normally — the +browser simply refuses to let the calling page read the response. Non-browser clients are +unaffected: CORS is a browser rule, **not server-side access control**. If you need to keep callers +out, use authentication. `cors: {}` and `cors: { origin: false }` behave identically to omitting the option — the middleware is installed only when `origin` is set to something other than `false`. @@ -459,6 +483,29 @@ http: { - Use for local daemons, CLI tools, and process manager integrations - Combine with `sqlite` for fully local deployments +The socket file is the only boundary a socket server has, so startup is strict about it (POSIX; +Windows named pipes are not checked): + +- The socket is created **owner-only (`0600`)** — the umask is tightened around `listen`, so it is + never reachable by others, even briefly. Only the user running the server can connect; the group + can no longer connect (it was `0660`). If the mode cannot be set, startup fails with + `SocketPermissionError`. Where the umask cannot be set (a worker thread, or a cluster worker, + whose socket the primary binds), the socket's directory must be owner-only (`0700`) instead, or + startup fails with `SocketPermissionError` before binding. +- The directory, and every directory above it (along the path as given and along the path its + symlinks lead to), must belong to you (or root), and must not be writable by the group or others + unless it has the sticky bit (as `/tmp` does): otherwise another user could rename it and put + their own socket at the path. The sticky bit does not bind a directory's owner, so a sticky + directory another user owns is refused too. Otherwise startup fails with + `UnsafeSocketDirectoryError`, which names the directory and its mode. `frontmcp socket` and + `frontmcp start --socket` create `~/.frontmcp/sockets` as `0700` and take group and other write + away from `~/.frontmcp`. Prefer it (or another directory only you can write to) over `/tmp`: + while your server is down, another user can create a socket at a predictable `/tmp` path and your + clients connect to theirs. +- A socket a crashed server left behind is removed and replaced. A live socket (a server answers on + it) fails with `SocketInUseError` and that server keeps serving. A path that is not a socket, or a + socket another user owns, fails with `SocketPathConflictError` and is left in place. + ## Verification ```bash @@ -498,7 +545,7 @@ curl --unix-socket /tmp/my-mcp-server.sock http://localhost/ ### CORS -- [ ] `cors` is set only if a browser on another origin needs access — omitting it sends no headers +- [ ] `cors` is set only if a browser on another origin needs access — omitting it sends no headers, and a loopback server refuses that origin - [ ] If `credentials: true`, `origin` lists explicit allowed origins (not `true` or `*`) - [ ] `maxAge` is set to a reasonable value for production (e.g., `86400` for 24 hours) - [ ] Dynamic origin function handles `undefined` origin (non-browser requests) @@ -519,17 +566,20 @@ curl --unix-socket /tmp/my-mcp-server.sock http://localhost/ ## Troubleshooting -| Problem | Cause | Solution | -| ----------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | -| `EADDRINUSE` on startup | Another process is already using the configured port | Change the port, stop the other process, or use `port: 0` for a random available port | -| CORS errors in the browser console | Origin not included in the `cors.origin` list or `credentials: true` with wildcard origin | Add the frontend origin to the `origin` array and ensure credentials and origin settings are compatible | -| Unix socket file not created | Missing write permissions on the target directory or stale socket file from a previous run | Check directory permissions and remove the stale `.sock` file before restarting | -| Routes return 404 after setting `entryPath` | Client is still requesting the root path without the prefix | Update client base URL to include the entry path (e.g., `http://localhost:3000/api/mcp`) | -| Server binds but external clients cannot connect | The default bind address is `127.0.0.1`, which a published container port cannot reach | Set `FRONTMCP_BIND_ADDRESS=all` in the container (or `http.security.bindAddress: 'all'` in the config) | -| `413 Payload Too Large` with JSON-RPC envelope | Request body exceeded `bodyLimit` (default `'4mb'`) | Raise `http.bodyLimit` to fit the payload, or move large blobs to a separate upload endpoint | -| Server throws at startup mentioning a "reserved" path | A custom `http.routes` path collides with the MCP entry path, `/oauth/*`, `/.well-known/*`, `/health`, or `/metrics` | Rename the custom route to a non-reserved path | -| Custom route returns JSON when HTML/bytes expected | The Express adapter defaults responses to `application/json` | Set `res.setHeader('Content-Type', ...)` (or `res.type(...)`) in the handler before sending the body | -| Custom `auth: true` route always returns `401` | Auth mode has no anonymous fallback and the request carries no valid bearer token | Send a valid `Authorization: Bearer `, or use `auth` mode `public`/transparent-anon for open routes | +| Problem | Cause | Solution | +| ----------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| `EADDRINUSE` on startup | Another process is already using the configured port | Change the port, stop the other process, or use `port: 0` for a random available port | +| CORS errors in the browser console | Origin not included in the `cors.origin` list or `credentials: true` with wildcard origin | Add the frontend origin to the `origin` array and ensure credentials and origin settings are compatible | +| Unix socket file not created | Missing write permissions on the target directory | Check directory permissions; a stale `.sock` file from a crashed run is replaced automatically | +| `UnsafeSocketDirectoryError` at startup | The socket's directory, or one above it, belongs to another user, or is writable by the group or others and has no sticky bit | Use a directory only you can write to (`chmod 700 `), such as `~/.frontmcp/sockets` | +| `SocketInUseError` / `SocketPathConflictError` | A server already listens on the socket, or the path holds a regular file or another user's socket | Stop the other server or pick another `socketPath`; FrontMCP never removes these itself | +| `403 Invalid Origin header` from a browser app | A loopback server refuses an `Origin` that is not its own and not in `cors.origin` | Add the app's origin to `cors.origin` (or set `dnsRebindingProtection.allowedOrigins`) | +| Routes return 404 after setting `entryPath` | Client is still requesting the root path without the prefix | Update client base URL to include the entry path (e.g., `http://localhost:3000/api/mcp`) | +| Server binds but external clients cannot connect | The default bind address is `127.0.0.1`, which a published container port cannot reach | Set `FRONTMCP_BIND_ADDRESS=all` in the container (or `http.security.bindAddress: 'all'` in the config) | +| `413 Payload Too Large` with JSON-RPC envelope | Request body exceeded `bodyLimit` (default `'4mb'`) | Raise `http.bodyLimit` to fit the payload, or move large blobs to a separate upload endpoint | +| Server throws at startup mentioning a "reserved" path | A custom `http.routes` path collides with the MCP entry path, `/oauth/*`, `/.well-known/*`, `/health`, or `/metrics` | Rename the custom route to a non-reserved path | +| Custom route returns JSON when HTML/bytes expected | The Express adapter defaults responses to `application/json` | Set `res.setHeader('Content-Type', ...)` (or `res.type(...)`) in the handler before sending the body | +| Custom `auth: true` route always returns `401` | Auth mode has no anonymous fallback and the request carries no valid bearer token | Send a valid `Authorization: Bearer `, or use `auth` mode `public`/transparent-anon for open routes | ## Examples diff --git a/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md b/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md index 365626130..18b697e32 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md +++ b/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md @@ -145,6 +145,8 @@ frontmcp socket ./src/main.ts -b --db ~/.my-tool/data.db The daemon accepts JSON-RPC requests over HTTP via the Unix socket, making it ideal for local MCP clients (Claude Code, IDE extensions) that need persistent tool access without a TCP port. +The socket is the daemon's only boundary, so it is created owner-only (`0600`): only the user running the daemon can connect. `~/.frontmcp/sockets` is created `0700` and `~/.frontmcp` loses group and other write, and startup refuses a socket whose directory, or any directory above it (through symlinks too), another user (other than root) owns or the group or others can write to (unless it has the sticky bit, as `/tmp` does). A start replaces a stale socket a crashed daemon left behind, but refuses a live one (another daemon is answering on it), a regular file, or another user's socket instead of removing it. + ## Process Management Manage long-running MCP server processes with built-in supervisor commands: diff --git a/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md b/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md index 9ea51b7d6..ce8c1e0fd 100644 --- a/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md +++ b/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md @@ -155,7 +155,7 @@ cross-cutting concern, it's wrong — rework it through a hookable flow. Besides flow hooks, the scope (`ScopeEntry`) takes two lifecycle callbacks: - **`scope.onServerStarted(callback: () => void | Promise): void`** runs `callback` after the HTTP server starts listening, in `FrontMcpInstance.start()`. Callbacks run in registration order, each awaited. It never fires in direct mode (`create()`, `createDirect()`, `connect()`), where no HTTP server starts. -- **`scope.onDispose(callback: () => void | Promise): () => void`** runs `callback` when the scope is disposed, in direct mode too. Callbacks run once, in reverse order of registration; the returned function removes the callback. +- **`scope.onDispose(callback: () => void | Promise): () => void`** runs `callback` when the scope is disposed: on `SIGTERM`/`SIGINT` for an HTTP, Unix socket or stdio server, and in direct mode too. Callbacks run once, in reverse order of registration, each awaited; the shutdown deadline (5s under stdio, 10s otherwise) still exits a process whose callback hangs. The returned function removes the callback. Use them for warming caches, starting background indexing, or releasing timers and connections. A plugin reaches the scope from a factory provider: `ScopeEntry` is a DI token, so `inject: () => [ScopeEntry]` hands the factory the scope it is built in. The plugin's providers are built while the server starts up, before it listens, so the callback is registered in time. diff --git a/libs/skills/catalog/frontmcp-production-readiness/SKILL.md b/libs/skills/catalog/frontmcp-production-readiness/SKILL.md index 440c4451d..2f162a8a5 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/SKILL.md +++ b/libs/skills/catalog/frontmcp-production-readiness/SKILL.md @@ -151,11 +151,11 @@ Each reference has matching examples under [`examples//`](./examples/ ### `production-cli-daemon` -| Example | Level | Description | -| -------------------------------------------------------------------------------------------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [`daemon-socket-config`](./examples/production-cli-daemon/daemon-socket-config.md) | Basic | Shows how to configure a FrontMCP server as a long-running local daemon with Unix socket transport, process management, and SQLite storage. | -| [`graceful-shutdown-cleanup`](./examples/production-cli-daemon/graceful-shutdown-cleanup.md) | Intermediate | Shows how to implement graceful shutdown for a daemon process, including completing in-flight requests, closing database connections, removing the socket file, and cleaning up the PID file. | -| [`security-and-permissions`](./examples/production-cli-daemon/security-and-permissions.md) | Advanced | Shows how to secure a local daemon with restrictive socket permissions, XDG-compliant config storage, and file-based secret management. | +| Example | Level | Description | +| -------------------------------------------------------------------------------------------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [`daemon-socket-config`](./examples/production-cli-daemon/daemon-socket-config.md) | Basic | Shows how to configure a FrontMCP server as a long-running local daemon with Unix socket transport, process management, and SQLite storage. | +| [`graceful-shutdown-cleanup`](./examples/production-cli-daemon/graceful-shutdown-cleanup.md) | Intermediate | Shows how to implement graceful shutdown for a daemon process, including completing in-flight requests, closing database connections, and cleaning up the PID file while the framework removes the socket file. | +| [`security-and-permissions`](./examples/production-cli-daemon/security-and-permissions.md) | Advanced | Shows how to secure a local daemon with restrictive socket permissions, XDG-compliant config storage, and file-based secret management. | ### `production-cloudflare` diff --git a/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/graceful-shutdown-cleanup.md b/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/graceful-shutdown-cleanup.md index 62a24fcde..66a20d571 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/graceful-shutdown-cleanup.md +++ b/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/graceful-shutdown-cleanup.md @@ -2,7 +2,7 @@ name: graceful-shutdown-cleanup reference: production-cli-daemon level: intermediate -description: Shows how to layer daemon-specific cleanup (socket file, PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler. +description: Shows how to layer daemon-specific cleanup (PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler, leaving the socket file to the framework. tags: - production - unix-socket @@ -13,7 +13,7 @@ tags: features: - The framework already wires SIGTERM/SIGINT — daemon cleanup attaches _additional_ listeners and does not call `process.exit()` - Using `server.dispose()` (the only real method) instead of fictional `server.close()` - - Removing the Unix socket file to prevent stale `.sock` files on restart + - Leaving the Unix socket file to the framework, which removes it on close and replaces a stale one at startup - Cleaning up the PID file on shutdown - Using `@frontmcp/utils` (`unlink`, `fileExists`, `ensureDir`) for file operations - Providers initialize in the constructor — there is no `onInit` / `onDestroy` @@ -21,9 +21,9 @@ features: # Daemon Graceful Shutdown with Socket Cleanup -Shows how to layer daemon-specific cleanup (socket file, PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler. +Shows how to layer daemon-specific cleanup (PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler, leaving the socket file to the framework. -> The FrontMCP framework already installs `SIGTERM` / `SIGINT` handlers that call `scope.shutdown()` and `mcpServer.close()` (see `libs/sdk/src/front-mcp/front-mcp.ts`). **Do not install your own** competing shutdown handler — only register _additional_ cleanup hooks (e.g. for socket / PID files) that run before the framework exits. +> The FrontMCP framework already installs `SIGTERM` / `SIGINT` handlers that shut down and dispose every scope (so `scope.onDispose()` callbacks run) and close the server (see `libs/sdk/src/front-mcp/front-mcp.ts`). Closing the server removes its socket file, and the next start replaces a stale one, so do not unlink the socket yourself: a signal listener runs while requests are still draining. **Do not install your own** competing shutdown handler — only register _additional_ cleanup hooks (e.g. for a PID file) that run before the framework exits. > > `FrontMcpServerInstance` exposes `dispose()` only — there is **no** `server.close()` method. @@ -39,14 +39,9 @@ import { fileExists, unlink } from '@frontmcp/utils'; * the framework owns that. */ export function setupDaemonSideEffectCleanup(): void { - const socketPath = '/tmp/my-daemon.sock'; const pidFile = `${process.env.HOME}/.config/my-daemon/daemon.pid`; const cleanup = async () => { - if (await fileExists(socketPath)) { - await unlink(socketPath); - console.log(`[daemon] Socket removed: ${socketPath}`); - } if (await fileExists(pidFile)) { await unlink(pidFile); console.log(`[daemon] PID file removed: ${pidFile}`); @@ -104,7 +99,7 @@ export const sqliteStoreProvider = AsyncProvider({ - The framework already wires SIGTERM/SIGINT — daemon cleanup attaches _additional_ listeners and does not call `process.exit()` - Using `server.dispose()` (the only real method) instead of fictional `server.close()` -- Removing the Unix socket file to prevent stale `.sock` files on restart +- Leaving the Unix socket file to the framework, which removes it on close and replaces a stale one at startup - Cleaning up the PID file on shutdown - Using `@frontmcp/utils` (`unlink`, `fileExists`, `ensureDir`) for file operations - Providers initialize in the constructor — there is no `onInit` / `onDestroy` diff --git a/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/security-and-permissions.md b/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/security-and-permissions.md index c47592038..c3cf9d163 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/security-and-permissions.md +++ b/libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-daemon/security-and-permissions.md @@ -20,11 +20,10 @@ Shows how to secure a local daemon with restrictive socket permissions, XDG-comp ```typescript // src/lifecycle/daemon-security.ts -import { chmod } from 'fs/promises'; import * as os from 'os'; import * as path from 'path'; -import { ensureDir, fileExists, readFile, stat, writeFile } from '@frontmcp/utils'; +import { chmod, ensureDir, fileExists, readFile, stat, writeFile } from '@frontmcp/utils'; // XDG Base Directory compliance function getConfigDir(appName: string): string { @@ -84,8 +83,18 @@ export async function loadSecrets(configDir: string): Promise See all examples in [`examples/production-cli-daemon/`](../examples/production-cli-daemon/) diff --git a/libs/skills/catalog/skills-manifest.json b/libs/skills/catalog/skills-manifest.json index 7b0692434..af0215455 100644 --- a/libs/skills/catalog/skills-manifest.json +++ b/libs/skills/catalog/skills-manifest.json @@ -3001,13 +3001,13 @@ }, { "name": "graceful-shutdown-cleanup", - "description": "Shows how to layer daemon-specific cleanup (socket file, PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler.", + "description": "Shows how to layer daemon-specific cleanup (PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler, leaving the socket file to the framework.", "level": "intermediate", "tags": ["production", "unix-socket", "cli", "database", "daemon", "graceful"], "features": [ "The framework already wires SIGTERM/SIGINT — daemon cleanup attaches _additional_ listeners and does not call `process.exit()`", "Using `server.dispose()` (the only real method) instead of fictional `server.close()`", - "Removing the Unix socket file to prevent stale `.sock` files on restart", + "Leaving the Unix socket file to the framework, which removes it on close and replaces a stale one at startup", "Cleaning up the PID file on shutdown", "Using `@frontmcp/utils` (`unlink`, `fileExists`, `ensureDir`) for file operations", "Providers initialize in the constructor — there is no `onInit` / `onDestroy`" diff --git a/libs/utils/src/fs/fs.spec.ts b/libs/utils/src/fs/fs.spec.ts index 33e870781..b18428829 100644 --- a/libs/utils/src/fs/fs.spec.ts +++ b/libs/utils/src/fs/fs.spec.ts @@ -4,6 +4,7 @@ import * as path from 'path'; import { access, + chmod, copyFile, cp, ensureDir, @@ -12,6 +13,7 @@ import { fileExistsSync, getSpawnFn, isDirEmpty, + lstat, mkdir, mkdtemp, readdir, @@ -384,6 +386,49 @@ describe('FS Utils', () => { }); }); + describe('lstat', () => { + const symlinkIt = process.platform === 'win32' ? it.skip : it; + + symlinkIt('describes a symlink itself, not its target, where creating one needs no elevated rights', async () => { + const target = path.join(tempDir, 'target.txt'); + const link = path.join(tempDir, 'link.txt'); + await fs.promises.writeFile(target, 'content'); + await fs.promises.symlink(target, link); + + const stats = await lstat(link); + + expect(stats.isSymbolicLink()).toBe(true); + }); + + it('should throw for non-existing path', async () => { + await expect(lstat(path.join(tempDir, 'nonexistent'))).rejects.toThrow(/ENOENT/); + }); + }); + + describe('chmod', () => { + const posixIt = process.platform === 'win32' ? it.skip : it; + + it('resolves for an existing file', async () => { + const filePath = path.join(tempDir, 'chmod-any.txt'); + await fs.promises.writeFile(filePath, 'content'); + + await expect(chmod(filePath, 0o600)).resolves.toBeUndefined(); + }); + + posixIt('sets the permission bits, which Windows cannot set apart for owner, group and others', async () => { + const filePath = path.join(tempDir, 'chmod.txt'); + await fs.promises.writeFile(filePath, 'content'); + + await chmod(filePath, 0o600); + + expect((await fs.promises.stat(filePath)).mode & 0o777).toBe(0o600); + }); + + it('should throw for non-existing path', async () => { + await expect(chmod(path.join(tempDir, 'nonexistent'), 0o600)).rejects.toThrow(/ENOENT/); + }); + }); + describe('copyFile', () => { it('should copy file', async () => { const srcPath = path.join(tempDir, 'src.txt'); diff --git a/libs/utils/src/fs/fs.ts b/libs/utils/src/fs/fs.ts index e4f81cb14..65b739576 100644 --- a/libs/utils/src/fs/fs.ts +++ b/libs/utils/src/fs/fs.ts @@ -298,6 +298,39 @@ export async function stat(p: string): Promise { return fsp.stat(p); } +/** + * Get file/directory stats without following a symlink. + * + * **Node.js only** - throws an error if called in browser. + * + * @param p - Path to file, directory, socket or symlink + * @returns Stats of the entry itself (isSocket(), isSymbolicLink(), mode, uid, ...) + * + * @example + * const stats = await lstat('/tmp/app.sock'); + * if (stats.isSocket()) { ... } + */ +export async function lstat(p: string): Promise { + const fsp = getFsp(); + return fsp.lstat(p); +} + +/** + * Change the permission bits of a file or directory. + * + * **Node.js only** - throws an error if called in browser. + * + * @param p - Path to file or directory + * @param mode - Permission bits, e.g. 0o600 + * + * @example + * await chmod('/tmp/app.sock', 0o600); + */ +export async function chmod(p: string, mode: number): Promise { + const fsp = getFsp(); + await fsp.chmod(p, mode); +} + /** Link type accepted by {@link symlink}; only meaningful on Windows. */ export type SymlinkType = 'dir' | 'file' | 'junction'; diff --git a/libs/utils/src/fs/index.ts b/libs/utils/src/fs/index.ts index 0c85a3a3a..2d075843f 100644 --- a/libs/utils/src/fs/index.ts +++ b/libs/utils/src/fs/index.ts @@ -8,6 +8,8 @@ export { rename, unlink, stat, + lstat, + chmod, copyFile, cp, readdir, diff --git a/libs/utils/src/index.ts b/libs/utils/src/index.ts index bf100c814..42881bba3 100644 --- a/libs/utils/src/index.ts +++ b/libs/utils/src/index.ts @@ -58,6 +58,8 @@ export { rename, unlink, stat, + lstat, + chmod, copyFile, cp, readdir,