From b88cbc9a8fb1d4acd45cabc0963a1bb6bd90f30a Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 03:30:55 +0200 Subject: [PATCH 1/6] fix(sdk): check Origin on loopback, create Unix sockets owner-only, dispose scopes on stdio shutdown - A loopback bind with derived DNS-rebinding protection also derives its allowed Origins (its own, the issuer's, the cors.origin list). A request with no Origin is still served; an explicit allowedOrigins still wins. - ExpressHostAdapter.start() falls back to 127.0.0.1 instead of 0.0.0.0. - A Unix socket is bound under an owner-only umask, set to 0600 and verified. Startup refuses a group- or other-writable directory without the sticky bit, a live socket, a non-socket and another user's socket; only a stale socket is replaced. bootstrap() and runUnixSocket() release what they built on a failed start. - pm and the generated daemon create ~/.frontmcp/sockets as 0700. - runStdio shutdown runs FrontMcpInstance.shutdown(), so every scope is shut down and disposed and onDispose callbacks run, within the 5 s stdio deadline. - @frontmcp/utils gains lstat() and chmod(). --- .../e2e/loopback-origin.e2e.spec.ts | 77 +++++++ .../src/loopback-origin.main.ts | 27 +++ .../e2e/cors-default.e2e.spec.ts | 24 +- .../e2e/cors-explicit-origin.e2e.spec.ts | 11 +- .../e2e/helpers/stdio-json-rpc.ts | 11 +- .../e2e/stdio-dispose.e2e.spec.ts | 43 ++++ .../src/stdio-dispose-entrypoint.ts | 29 +++ .../e2e/unix-socket.e2e.spec.ts | 57 +++-- .../exec/__tests__/generate-cli-entry.spec.ts | 14 ++ .../exec/cli-runtime/generate-cli-entry.ts | 10 +- .../src/commands/pm/__tests__/paths.spec.ts | 46 +++- libs/cli/src/commands/pm/paths.ts | 9 +- libs/cli/src/commands/pm/socket.ts | 14 +- .../common/types/options/http/interfaces.ts | 8 +- libs/sdk/src/errors/index.ts | 4 + libs/sdk/src/errors/transport.errors.ts | 39 ++++ .../run-stdio-dispose-deadline.spec.ts | 110 +++++++++ .../__tests__/run-stdio-dispose.spec.ts | 110 +++++++++ libs/sdk/src/front-mcp/front-mcp.ts | 56 +++-- libs/sdk/src/front-mcp/shutdown-signals.ts | 3 + .../__tests__/express.host.adapter.spec.ts | 114 ++++++++++ .../express.host.adapter.unix-socket.spec.ts | 215 ++++++++++++++++++ .../server/adapters/express.host.adapter.ts | 76 ++++--- .../__tests__/resolve-allowed-hosts.spec.ts | 49 ++++ .../security/__tests__/security-audit.spec.ts | 7 + .../server/security/resolve-allowed-hosts.ts | 49 ++++ .../sdk/src/server/security/security-audit.ts | 12 +- .../src/server/security/unix-socket-guard.ts | 97 ++++++++ .../references/configure-http.md | 41 +++- .../references/build-for-cli.md | 2 + .../references/create-plugin-hooks.md | 2 +- .../frontmcp-production-readiness/SKILL.md | 10 +- .../graceful-shutdown-cleanup.md | 15 +- .../references/common-checklist.md | 3 +- .../references/production-cli-daemon.md | 14 +- libs/skills/catalog/skills-manifest.json | 4 +- libs/utils/src/fs/fs.spec.ts | 34 +++ libs/utils/src/fs/fs.ts | 33 +++ libs/utils/src/fs/index.ts | 2 + libs/utils/src/index.ts | 2 + 40 files changed, 1337 insertions(+), 146 deletions(-) create mode 100644 apps/e2e/demo-e2e-security/e2e/loopback-origin.e2e.spec.ts create mode 100644 apps/e2e/demo-e2e-security/src/loopback-origin.main.ts create mode 100644 apps/e2e/demo-e2e-stdio-transport/e2e/stdio-dispose.e2e.spec.ts create mode 100644 apps/e2e/demo-e2e-stdio-transport/src/stdio-dispose-entrypoint.ts create mode 100644 libs/sdk/src/front-mcp/__tests__/run-stdio-dispose-deadline.spec.ts create mode 100644 libs/sdk/src/front-mcp/__tests__/run-stdio-dispose.spec.ts create mode 100644 libs/sdk/src/server/adapters/__tests__/express.host.adapter.unix-socket.spec.ts create mode 100644 libs/sdk/src/server/security/unix-socket-guard.ts 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-stdio-transport/e2e/helpers/stdio-json-rpc.ts b/apps/e2e/demo-e2e-stdio-transport/e2e/helpers/stdio-json-rpc.ts index e0dffc473..517ab2d34 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,10 @@ export class StdioJsonRpcProcess { }); } - static start(entrypointPath: string, cwd: string): StdioJsonRpcProcess { + static start(entrypointPath: string, cwd: string, env: Record = {}): StdioJsonRpcProcess { const child = spawn('npx', ['tsx', entrypointPath], { cwd, - env: process.env as Record, + env: { ...process.env, ...env } as Record, stdio: ['pipe', 'pipe', 'pipe'], }); child.stderr.resume(); @@ -66,6 +66,13 @@ export class StdioJsonRpcProcess { } } + /** Send SIGTERM, leaving stdin open, and resolve with the exit code. */ + async terminate(): Promise { + 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; const exited = new Promise((resolve) => this.child.once('close', resolve)); 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..45bd45550 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,20 @@ 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('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..7646799c6 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,13 @@ 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 directory others can write to + fs.mkdirSync(socketDir, { recursive: true, mode: 0o700 }); + 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 +1839,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..4b3545048 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,45 @@ 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.ensurePmDirs(); + + expect(fs.statSync(paths.PM_DIRS.sockets).mode & 0o777).toBe(0o700); + }); + + 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/paths.ts b/libs/cli/src/commands/pm/paths.ts index 57be5f102..e7a158b9e 100644 --- a/libs/cli/src/commands/pm/paths.ts +++ b/libs/cli/src/commands/pm/paths.ts @@ -4,8 +4,8 @@ */ 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'); @@ -50,4 +50,11 @@ export function ensurePmDirs(): void { fs.mkdirSync(dir, { recursive: true }); } } + ensureSocketsDir(); +} + +/** The server refuses a socket directory others can write to, so this one is owner-only (0700). */ +export function ensureSocketsDir(): void { + fs.mkdirSync(PM_DIRS.sockets, { recursive: true, mode: 0o700 }); + 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/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..594174420 100644 --- a/libs/sdk/src/errors/transport.errors.ts +++ b/libs/sdk/src/errors/transport.errors.ts @@ -121,3 +121,42 @@ export class SessionOwnerUnreachableError extends PublicMcpError { this.reason = reason; } } + +function octalMode(mode: number): string { + return `0${(mode & 0o7777).toString(8)}`; +} + +/** + * Thrown at startup when a Unix socket's directory can be written by the group or others and has no + * sticky bit, so another user could replace the socket. + */ +export class UnsafeSocketDirectoryError extends InternalMcpError { + constructor(directory: string, mode: number) { + super( + `Refusing to create a Unix socket in ${directory} (mode ${octalMode(mode)}): the group or others can write ` + + 'to it and it has no sticky bit. 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__/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/front-mcp.ts b/libs/sdk/src/front-mcp/front-mcp.ts index 1df7e58e6..11de917ec 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. @@ -154,6 +154,16 @@ 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) { + await this.shutdown().catch(() => undefined); + throw 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 +352,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 +666,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 +806,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.shutdown()); // The scope holding the server's own apps, not a standalone app's const primaryScope = frontMcp.getPrimaryScope(); @@ -908,29 +914,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..773f1de04 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,120 @@ 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 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('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..1f4996951 --- /dev/null +++ b/libs/sdk/src/server/adapters/__tests__/express.host.adapter.unix-socket.spec.ts @@ -0,0 +1,215 @@ +/** + * 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 * 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) }; +}); + +const actualUtils = jest.requireActual('@frontmcp/utils'); +const chmodMock = utils.chmod as jest.MockedFunction; +const lstatMock = utils.lstat 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); + 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); + + await start(); + + expect(fs.lstatSync(socketPath).mode & 0o777).toBe(0o600); + }); + + 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); + }); + + 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.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(); + } + }); + + 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..a59065259 --- /dev/null +++ b/libs/sdk/src/server/security/unix-socket-guard.ts @@ -0,0 +1,97 @@ +// A socket server has no Host or Origin to check: the socket file's permissions are its only boundary. +import type * as http from 'node:http'; +import * as net from 'node:net'; +import * as path from 'node:path'; + +import { chmod, lstat, stat, unlink } from '@frontmcp/utils'; + +import { + SocketInUseError, + SocketPathConflictError, + SocketPermissionError, + UnsafeSocketDirectoryError, +} from '../../errors/transport.errors'; + +const OWNER_ONLY_MODE = 0o600; +const OWNER_ONLY_UMASK = 0o177; +const GROUP_OR_OTHER_WRITE = 0o022; +const STICKY_BIT = 0o1000; + +/** 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 directory the group or others can write to, unless it has the sticky bit (as /tmp does). */ +export async function assertSafeSocketDirectory(socketPath: string): Promise { + const directory = path.dirname(path.resolve(socketPath)); + const { mode } = await stat(directory); + if (mode & GROUP_OR_OTHER_WRITE && !(mode & STICKY_BIT)) throw new UnsafeSocketDirectoryError(directory, mode); +} + +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..4d2796a01 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -134,6 +134,24 @@ 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. A routable bind derives no origins. + +```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,7 +187,7 @@ 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 page always does. Rejecting the absent case breaks every CLI client and adds nothing. ## CORS Configuration @@ -459,6 +477,20 @@ 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`. +- The directory must not be writable by the group or others, unless it has the sticky bit (as + `/tmp` does). Otherwise startup fails with `UnsafeSocketDirectoryError`, which names the directory + and its mode. `frontmcp socket` and `frontmcp start --socket` create `~/.frontmcp/sockets` as `0700`. +- 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 +530,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) @@ -523,7 +555,10 @@ curl --unix-socket /tmp/my-mcp-server.sock http://localhost/ | ----------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | | `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 | +| 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 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 | 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..b80d6f32d 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 startup refuses a socket directory 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 cca52d962..58d12d4dd 100644 --- a/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md +++ b/libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md @@ -118,7 +118,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/references/common-checklist.md b/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md index a09efd56b..029ea7c65 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md +++ b/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md @@ -40,7 +40,8 @@ These checks apply to ALL deployment targets. Run them first, then proceed to yo - [ ] Startup logs show no `DNS-rebinding protection is not enforcing a Host allow-list` warning, and the production audit reports `[Security] DNS_REBINDING_PROTECTED` (not `DNS_REBINDING_NOT_ENFORCED`) - [ ] Include the port when the public URL uses a non-default one (`api.example.com:8443`) -- [ ] `allowedOrigins` is set when a browser client connects, so a foreign `Origin` is refused +- [ ] On a routable bind, `allowedOrigins` is set when a browser client connects, so a foreign `Origin` is refused. + A loopback bind derives its own origins (plus the `cors.origin` list) and refuses any other `Origin` ### Input Validation diff --git a/libs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.md b/libs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.md index 7556db6a7..8e29d1e7d 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.md +++ b/libs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.md @@ -22,7 +22,9 @@ Checklist for deploying FrontMCP as a long-running local MCP server managed by t - [ ] Unix socket path is configured: `http: { socketPath: '/tmp/my-app.sock' }` - [ ] Socket file is cleaned up on shutdown (no stale `.sock` files) - [ ] Stdio transport works as fallback: `frontmcp socket ./src/main.ts` -- [ ] Socket permissions are restrictive (only the owning user can connect) +- [ ] Socket permissions are restrictive: FrontMCP creates the socket `0600` (only the owning user can connect) and fails startup if it cannot +- [ ] The socket directory is writable only by the owner (or is sticky, like `/tmp`); startup refuses any other with `UnsafeSocketDirectoryError` +- [ ] A second daemon on the same socket path fails with `SocketInUseError` and the first keeps serving ## Service Registration @@ -64,10 +66,10 @@ On a Unix socket (`runUnixSocket()`, or `bootstrap()` under `FRONTMCP_DAEMON_SOC ## Examples -| 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 layer daemon-specific cleanup (socket file, PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler. | -| [`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 layer daemon-specific cleanup (PID file) **on top of** the framework's built-in SIGTERM/SIGINT handler, leaving the socket file to the framework. | +| [`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. | > 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 65a310d09..a3279ade2 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..f74a3b84e 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,38 @@ describe('FS Utils', () => { }); }); + describe('lstat', () => { + it('describes a symlink itself, not its target', 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', () => { + it('sets the permission bits', 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..96c5fd5cd 100644 --- a/libs/utils/src/fs/fs.ts +++ b/libs/utils/src/fs/fs.ts @@ -299,6 +299,39 @@ export async function stat(p: string): Promise { } /** Link type accepted by {@link symlink}; only meaningful on Windows. */ +/** + * 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); +} + 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, From fd0f6e63cb05540e26d651e70e439b536e0f6d78 Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 03:41:26 +0200 Subject: [PATCH 2/6] fix(sdk): bound the release of a failed start by the shutdown deadline - bootstrap() and runUnixSocket() release a failed start through releaseFailedStart(), so a hanging dispose cannot hold back the start error. - The stdio e2e helper treats a child killed by a signal as exited. - The catalog no longer claims a browser sends Origin on every request. - Cover the umask fallback, a socket that vanishes before the probe, an lstat failure, and the release of a refused socket start. --- .../e2e/helpers/stdio-json-rpc.ts | 7 +++- .../__tests__/graceful-shutdown.spec.ts | 22 +++++++++++ libs/sdk/src/front-mcp/front-mcp.ts | 3 +- .../express.host.adapter.unix-socket.spec.ts | 37 +++++++++++++++++++ .../references/configure-http.md | 2 +- 5 files changed, 67 insertions(+), 4 deletions(-) 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 517ab2d34..4ef5dfbec 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 @@ -66,15 +66,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/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/front-mcp.ts b/libs/sdk/src/front-mcp/front-mcp.ts index 11de917ec..578c6c2b4 100644 --- a/libs/sdk/src/front-mcp/front-mcp.ts +++ b/libs/sdk/src/front-mcp/front-mcp.ts @@ -159,8 +159,7 @@ export class FrontMcpInstance implements FrontMcpInterface { try { await this.start(); } catch (error) { - await this.shutdown().catch(() => undefined); - throw error; + return this.releaseFailedStart(error); } } 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 index 1f4996951..1a42afbdb 100644 --- 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 @@ -201,6 +201,43 @@ describe('ExpressHostAdapter on a Unix socket', () => { } }); + 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('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' }); diff --git a/libs/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index 4d2796a01..e21915bf6 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -187,7 +187,7 @@ 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 -browser 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 From 8fb696b46fa9671be73192bae0de6d36bef3a251 Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 04:08:06 +0200 Subject: [PATCH 3/6] fix(sdk): refuse a socket directory another user owns and bound the stdio startup rollback - A Unix socket directory another user (not root) owns is refused with UnsafeSocketDirectoryError: the sticky bit does not bind a directory's owner, who can still remove or rename the socket. - The owner-only umask around listen is 0o077, so a directory another thread creates in that instant keeps its owner's execute bit; the socket is still set to 0600 and verified. - A stdio start that fails after its scopes are live rolls them back within the 5 s stdio deadline, so a hanging onDispose cannot hold back the error. - Pin the Origin edge cases (null, case, IPv6 spellings, default ports), that an explicit allowedHosts leaves Origin to the operator, and that a loopback server refuses its server card to a page on another origin. - Catalog: directory ownership, the explicit-hosts precedence, the server card, and the daemon examples use ~/.frontmcp/sockets and chmod from @frontmcp/utils instead of /tmp and fs/promises. - Restore the SymlinkType doc comment the lstat/chmod insertion displaced. --- .../e2e/mcp-registry.e2e.spec.ts | 8 ++ libs/sdk/src/errors/transport.errors.ts | 14 ++- .../run-stdio-rollback-deadline.spec.ts | 105 ++++++++++++++++++ libs/sdk/src/front-mcp/front-mcp.ts | 15 ++- .../__tests__/express.host.adapter.spec.ts | 27 +++++ .../express.host.adapter.unix-socket.spec.ts | 37 +++++- .../src/server/security/unix-socket-guard.ts | 9 +- .../references/configure-http.md | 17 ++- .../references/build-for-cli.md | 2 +- .../security-and-permissions.md | 10 +- .../references/production-cli-daemon.md | 4 +- libs/utils/src/fs/fs.ts | 2 +- 12 files changed, 221 insertions(+), 29 deletions(-) create mode 100644 libs/sdk/src/front-mcp/__tests__/run-stdio-rollback-deadline.spec.ts 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/libs/sdk/src/errors/transport.errors.ts b/libs/sdk/src/errors/transport.errors.ts index 594174420..71a864d2c 100644 --- a/libs/sdk/src/errors/transport.errors.ts +++ b/libs/sdk/src/errors/transport.errors.ts @@ -127,14 +127,18 @@ function octalMode(mode: number): string { } /** - * Thrown at startup when a Unix socket's directory can be written by the group or others and has no - * sticky bit, so another user could replace the socket. + * Thrown at startup when another user could replace a Unix socket in its directory: the directory + * 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) { + 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 in ${directory} (mode ${octalMode(mode)}): the group or others can write ` + - 'to it and it has no sticky bit. Use a directory only you can write to, such as ~/.frontmcp/sockets.', + `Refusing to create a Unix socket in ${directory} (mode ${octalMode(mode)}): ${reason}. ` + + 'Use a directory only you can write to, such as ~/.frontmcp/sockets.', 'UNSAFE_SOCKET_DIRECTORY', ); } 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 578c6c2b4..db4e7753c 100644 --- a/libs/sdk/src/front-mcp/front-mcp.ts +++ b/libs/sdk/src/front-mcp/front-mcp.ts @@ -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 { @@ -805,7 +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(() => frontMcp.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(); 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 773f1de04..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 @@ -739,6 +739,23 @@ describe('ExpressHostAdapter — derived Origin check on a loopback bind', () => 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'); @@ -769,6 +786,16 @@ describe('ExpressHostAdapter — derived Origin check on a loopback bind', () => 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' } }), 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 index 1a42afbdb..783324b0b 100644 --- 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 @@ -27,12 +27,13 @@ jest.mock('../base.host.adapter', () => { jest.mock('@frontmcp/utils', () => { const actual = jest.requireActual('@frontmcp/utils'); - return { ...actual, chmod: jest.fn(actual.chmod), lstat: jest.fn(actual.lstat) }; + 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; @@ -71,6 +72,7 @@ describe('ExpressHostAdapter on a Unix socket', () => { log.mockRestore(); chmodMock.mockImplementation(actualUtils.chmod); lstatMock.mockImplementation(actualUtils.lstat); + statMock.mockImplementation(actualUtils.stat); fs.rmSync(directory, { recursive: true, force: true }); }); @@ -91,10 +93,14 @@ describe('ExpressHostAdapter on a Unix socket', () => { 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 & 0o777).toBe(0o600); + expect(fs.lstatSync(socketPath).mode & 0o077).toBe(0); }); posixIt('restores the process umask after binding', async () => { @@ -141,6 +147,33 @@ describe('ExpressHostAdapter on a Unix socket', () => { 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); + }); + posixIt('replaces a stale socket a crashed server left behind', async () => { leaveStaleSocket(socketPath); expect(fs.lstatSync(socketPath).isSocket()).toBe(true); diff --git a/libs/sdk/src/server/security/unix-socket-guard.ts b/libs/sdk/src/server/security/unix-socket-guard.ts index a59065259..3579e216f 100644 --- a/libs/sdk/src/server/security/unix-socket-guard.ts +++ b/libs/sdk/src/server/security/unix-socket-guard.ts @@ -13,9 +13,11 @@ import { } from '../../errors/transport.errors'; const OWNER_ONLY_MODE = 0o600; -const OWNER_ONLY_UMASK = 0o177; +// 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 STICKY_BIT = 0o1000; +const ROOT_UID = 0; /** A win32 socket path names a named pipe, which has no file mode. */ export function hasSocketFileMode(): boolean { @@ -26,10 +28,11 @@ function errorCode(error: unknown): string | undefined { return (error as NodeJS.ErrnoException | undefined)?.code; } -/** Refuse a directory the group or others can write to, unless it has the sticky bit (as /tmp does). */ +/** Refuse a directory another user owns (the sticky bit does not bind its owner) or can write without the sticky bit. */ export async function assertSafeSocketDirectory(socketPath: string): Promise { const directory = path.dirname(path.resolve(socketPath)); - const { mode } = await stat(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); } diff --git a/libs/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index e21915bf6..90b1a5d02 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -142,7 +142,10 @@ origin, and the origins your `cors.origin` list names. A request from any other 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. A routable bind derives no origins. +`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. @@ -484,9 +487,13 @@ Windows named pipes are not checked): 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`. -- The directory must not be writable by the group or others, unless it has the sticky bit (as - `/tmp` does). Otherwise startup fails with `UnsafeSocketDirectoryError`, which names the directory - and its mode. `frontmcp socket` and `frontmcp start --socket` create `~/.frontmcp/sockets` as `0700`. +- The directory 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). 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`. 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. @@ -556,7 +563,7 @@ curl --unix-socket /tmp/my-mcp-server.sock http://localhost/ | `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 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` | +| `UnsafeSocketDirectoryError` at startup | The socket's directory 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`) | 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 b80d6f32d..265262d91 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md +++ b/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md @@ -145,7 +145,7 @@ 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 startup refuses a socket directory 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. +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 startup refuses a socket directory another user owns, or one 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 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..bd936aa64 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 { @@ -86,6 +85,8 @@ export async function loadSecrets(configDir: string): Promise { return fsp.stat(p); } -/** Link type accepted by {@link symlink}; only meaningful on Windows. */ /** * Get file/directory stats without following a symlink. * @@ -332,6 +331,7 @@ export async function chmod(p: string, mode: number): Promise { await fsp.chmod(p, mode); } +/** Link type accepted by {@link symlink}; only meaningful on Windows. */ export type SymlinkType = 'dir' | 'file' | 'junction'; /** From 8346884339ded5e49e4736098ba43553e22fc3de Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 04:38:08 +0200 Subject: [PATCH 4/6] fix(sdk): refuse socket paths a directory above could replace, and bind owner-only or into a private directory - assertSafeSocketDirectory checks every directory from the socket's up to the root: one another user owns, or one the group or others can write without the sticky bit, could rename the socket's directory and put another socket in its place. - Where the umask cannot be set (a worker thread) or a cluster primary binds the socket, the socket's directory must be owner-only, or startup fails with SocketPermissionError before binding. - frontmcp socket, the pm commands and the generated daemon start take group and other write away from ~/.frontmcp. - The stdio e2e helper runs the entrypoint in node directly, so SIGTERM reaches the server rather than npx's shell; the live-socket unit test ignores the reset its stand-in server reads from the guard's probe; the utils chmod mode check runs on POSIX only. - Catalog: the socket directory rules cover directories above the socket, and the daemon example creates the socket directory first. --- .../e2e/helpers/stdio-json-rpc.ts | 3 +- .../exec/__tests__/generate-cli-entry.spec.ts | 5 ++ .../exec/cli-runtime/generate-cli-entry.ts | 4 +- .../src/commands/pm/__tests__/paths.spec.ts | 13 ++++ libs/cli/src/commands/pm/paths.ts | 5 +- libs/sdk/src/errors/transport.errors.ts | 4 +- .../express.host.adapter.unix-socket.spec.ts | 61 ++++++++++++++++++- .../src/server/security/unix-socket-guard.ts | 33 ++++++++-- .../references/configure-http.md | 44 +++++++------ .../references/build-for-cli.md | 2 +- .../security-and-permissions.md | 8 +++ .../references/production-cli-daemon.md | 2 +- libs/utils/src/fs/fs.spec.ts | 11 +++- 13 files changed, 161 insertions(+), 34 deletions(-) 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 4ef5dfbec..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,8 +30,9 @@ export class StdioJsonRpcProcess { }); } + /** 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('npx', ['tsx', entrypointPath], { + const child = spawn(process.execPath, ['--import', 'tsx', entrypointPath], { cwd, env: { ...process.env, ...env } as Record, stdio: ['pipe', 'pipe', 'pipe'], 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 45bd45550..f1b621459 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 @@ -985,6 +985,11 @@ describe('generateCliEntry', () => { 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('removes a leftover socket only once no daemon is running', () => { const source = generateCliEntry(makeOptions()); const runningCheck = source.indexOf("console.log('Daemon already running"); 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 7646799c6..309ca4972 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,8 +1823,10 @@ daemonCmd var socketDir = pathMod.join(FRONTMCP_HOME, 'sockets'); fs.mkdirSync(pidDir, { recursive: true }); fs.mkdirSync(logDir, { recursive: true }); - // The server refuses a socket directory others can write to + // 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); fs.chmodSync(socketDir, 0o700); var usePort = !!opts.port; diff --git a/libs/cli/src/commands/pm/__tests__/paths.spec.ts b/libs/cli/src/commands/pm/__tests__/paths.spec.ts index 4b3545048..7e77197aa 100644 --- a/libs/cli/src/commands/pm/__tests__/paths.spec.ts +++ b/libs/cli/src/commands/pm/__tests__/paths.spec.ts @@ -96,6 +96,19 @@ describe('pm sockets directory', () => { expect(fs.statSync(paths.PM_DIRS.sockets).mode & 0o777).toBe(0o700); }); + 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('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 }); diff --git a/libs/cli/src/commands/pm/paths.ts b/libs/cli/src/commands/pm/paths.ts index e7a158b9e..9da8953b2 100644 --- a/libs/cli/src/commands/pm/paths.ts +++ b/libs/cli/src/commands/pm/paths.ts @@ -8,6 +8,7 @@ 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, @@ -53,8 +54,10 @@ export function ensurePmDirs(): void { ensureSocketsDir(); } -/** The server refuses a socket directory others can write to, so this one is owner-only (0700). */ +/** 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); fs.chmodSync(PM_DIRS.sockets, 0o700); } diff --git a/libs/sdk/src/errors/transport.errors.ts b/libs/sdk/src/errors/transport.errors.ts index 71a864d2c..9397b6f3e 100644 --- a/libs/sdk/src/errors/transport.errors.ts +++ b/libs/sdk/src/errors/transport.errors.ts @@ -127,7 +127,7 @@ function octalMode(mode: number): string { } /** - * Thrown at startup when another user could replace a Unix socket in its directory: the directory + * 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 { @@ -137,7 +137,7 @@ export class UnsafeSocketDirectoryError extends InternalMcpError { ? '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 in ${directory} (mode ${octalMode(mode)}): ${reason}. ` + + `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', ); 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 index 783324b0b..a0d80ad2b 100644 --- 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 @@ -4,6 +4,7 @@ * 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'; @@ -174,6 +175,32 @@ describe('ExpressHostAdapter on a Unix socket', () => { 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('replaces a stale socket a crashed server left behind', async () => { leaveStaleSocket(socketPath); expect(fs.lstatSync(socketPath).isSocket()).toBe(true); @@ -184,7 +211,10 @@ describe('ExpressHostAdapter on a Unix socket', () => { }); posixIt('refuses to take over a live socket, and the server on it keeps serving', async () => { - const first = net.createServer((socket) => socket.end('first')); + 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); @@ -248,6 +278,35 @@ describe('ExpressHostAdapter on a Unix socket', () => { 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(() => { diff --git a/libs/sdk/src/server/security/unix-socket-guard.ts b/libs/sdk/src/server/security/unix-socket-guard.ts index 3579e216f..46ad1a006 100644 --- a/libs/sdk/src/server/security/unix-socket-guard.ts +++ b/libs/sdk/src/server/security/unix-socket-guard.ts @@ -1,4 +1,5 @@ // 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'; @@ -16,6 +17,7 @@ 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; @@ -28,12 +30,33 @@ function errorCode(error: unknown): string | undefined { return (error as NodeJS.ErrnoException | undefined)?.code; } -/** Refuse a directory another user owns (the sticky bit does not bind its owner) or can write without the sticky bit. */ +/** 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 directory = path.dirname(path.resolve(socketPath)); - 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); + const socketDirectory = path.dirname(path.resolve(socketPath)); + for (let directory = socketDirectory; ; 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) break; + } + 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)}`, + ); + } +} + +/** 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'> { diff --git a/libs/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index 90b1a5d02..3aa8d595b 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -486,12 +486,16 @@ 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`. -- The directory 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). 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`. Prefer it (or another directory + `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, 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 @@ -558,20 +562,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 | Check directory permissions; a stale `.sock` file from a crashed run is replaced automatically | -| `UnsafeSocketDirectoryError` at startup | The socket's directory 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 | +| 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 265262d91..14d783a2c 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md +++ b/libs/skills/catalog/frontmcp-deployment/references/build-for-cli.md @@ -145,7 +145,7 @@ 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 startup refuses a socket directory another user owns, or one 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. +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, another user 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 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 bd936aa64..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 @@ -83,6 +83,14 @@ export async function loadSecrets(configDir: string): Promise { }); describe('chmod', () => { - it('sets the permission bits', async () => { + 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'); From a09e8c29ca3faf23e6f070f9a7ebf87cc62894c6 Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 05:14:13 +0200 Subject: [PATCH 5/6] fix(sdk): check the directories a socket's symlinks lead to, and refuse a symlinked sockets directory - assertSafeSocketDirectory walks the real path of the socket's directory too, so a symlink cannot route the socket through a directory another user can replace. - ensureSocketsDir and the generated daemon start refuse a symlinked ~/.frontmcp/sockets instead of setting the mode of the directory it points to. - The utils lstat symlink test runs where creating a symlink needs no elevated rights. - Catalog: the root-owned exception and symlinks in the socket rules, and the loopback Origin check in the default CORS description. --- .../exec/__tests__/generate-cli-entry.spec.ts | 7 ++++++ .../exec/cli-runtime/generate-cli-entry.ts | 4 ++++ .../src/commands/pm/__tests__/paths.spec.ts | 12 ++++++++++ libs/cli/src/commands/pm/paths.ts | 5 ++++ .../express.host.adapter.unix-socket.spec.ts | 14 +++++++++++ .../src/server/security/unix-socket-guard.ts | 21 ++++++++++------ .../references/configure-http.md | 24 ++++++++++--------- .../references/build-for-cli.md | 2 +- libs/utils/src/fs/fs.spec.ts | 4 +++- 9 files changed, 73 insertions(+), 20 deletions(-) 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 f1b621459..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 @@ -990,6 +990,13 @@ describe('generateCliEntry', () => { 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"); 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 309ca4972..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 @@ -1827,6 +1827,10 @@ daemonCmd 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; diff --git a/libs/cli/src/commands/pm/__tests__/paths.spec.ts b/libs/cli/src/commands/pm/__tests__/paths.spec.ts index 7e77197aa..43cdffa00 100644 --- a/libs/cli/src/commands/pm/__tests__/paths.spec.ts +++ b/libs/cli/src/commands/pm/__tests__/paths.spec.ts @@ -109,6 +109,18 @@ describe('pm sockets directory', () => { }, ); + 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 }); diff --git a/libs/cli/src/commands/pm/paths.ts b/libs/cli/src/commands/pm/paths.ts index 9da8953b2..a6c40b271 100644 --- a/libs/cli/src/commands/pm/paths.ts +++ b/libs/cli/src/commands/pm/paths.ts @@ -59,5 +59,10 @@ 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/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 index a0d80ad2b..776d57289 100644 --- 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 @@ -201,6 +201,20 @@ describe('ExpressHostAdapter on a Unix socket', () => { 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); diff --git a/libs/sdk/src/server/security/unix-socket-guard.ts b/libs/sdk/src/server/security/unix-socket-guard.ts index 46ad1a006..a00ca0ddc 100644 --- a/libs/sdk/src/server/security/unix-socket-guard.ts +++ b/libs/sdk/src/server/security/unix-socket-guard.ts @@ -4,7 +4,7 @@ import type * as http from 'node:http'; import * as net from 'node:net'; import * as path from 'node:path'; -import { chmod, lstat, stat, unlink } from '@frontmcp/utils'; +import { chmod, lstat, realpath, stat, unlink } from '@frontmcp/utils'; import { SocketInUseError, @@ -33,12 +33,9 @@ function errorCode(error: unknown): string | undefined { /** 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)); - for (let directory = socketDirectory; ; 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) break; - } + 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) { @@ -50,6 +47,16 @@ export async function assertSafeSocketDirectory(socketPath: 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; diff --git a/libs/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index 3aa8d595b..79d5f2312 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -196,9 +196,10 @@ browser sends one on every cross-site `POST`. Rejecting the absent case breaks e ### 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 +When `cors` is not specified, the server sends **no CORS headers**. 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 @@ -489,15 +490,16 @@ Windows named pipes are not checked): `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, 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 +- 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. + 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. 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 14d783a2c..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,7 +145,7 @@ 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, another user 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. +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 diff --git a/libs/utils/src/fs/fs.spec.ts b/libs/utils/src/fs/fs.spec.ts index 213f2f1c1..b18428829 100644 --- a/libs/utils/src/fs/fs.spec.ts +++ b/libs/utils/src/fs/fs.spec.ts @@ -387,7 +387,9 @@ describe('FS Utils', () => { }); describe('lstat', () => { - it('describes a symlink itself, not its target', async () => { + 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'); From a6c0f92322198123b5e5e69215ceeacabf12eba6 Mon Sep 17 00:00:00 2001 From: David Antoon Date: Mon, 2 Nov 2026 05:38:06 +0200 Subject: [PATCH 6/6] fix(cli): harden the sockets directory only when a socket server starts ensurePmDirs no longer chmods ~/.frontmcp/sockets, so read-only pm commands such as frontmcp status cannot fail on its mode. frontmcp socket and frontmcp start --socket on the default path still make it owner-only first. Catalog: the default CORS description names the server card's public CORS. --- libs/cli/src/commands/pm/__tests__/paths.spec.ts | 12 +++++++++++- libs/cli/src/commands/pm/__tests__/spawn.spec.ts | 13 +++++++++++++ libs/cli/src/commands/pm/paths.ts | 1 - libs/cli/src/commands/pm/spawn.ts | 3 ++- .../frontmcp-config/references/configure-http.md | 8 +++++--- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/libs/cli/src/commands/pm/__tests__/paths.spec.ts b/libs/cli/src/commands/pm/__tests__/paths.spec.ts index 43cdffa00..fe1cc6d9a 100644 --- a/libs/cli/src/commands/pm/__tests__/paths.spec.ts +++ b/libs/cli/src/commands/pm/__tests__/paths.spec.ts @@ -91,11 +91,21 @@ describe('pm sockets directory', () => { posixIt('creates ~/.frontmcp/sockets owner-only (0700)', () => { const paths = pathsUnder(home); - paths.ensurePmDirs(); + 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', () => { 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 a6c40b271..94b84df0c 100644 --- a/libs/cli/src/commands/pm/paths.ts +++ b/libs/cli/src/commands/pm/paths.ts @@ -51,7 +51,6 @@ export function ensurePmDirs(): void { fs.mkdirSync(dir, { recursive: true }); } } - ensureSocketsDir(); } /** The server refuses a socket others could replace, so ~/.frontmcp is not writable by others and its sockets are owner-only. */ 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/skills/catalog/frontmcp-config/references/configure-http.md b/libs/skills/catalog/frontmcp-config/references/configure-http.md index 79d5f2312..dc2f5d101 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-http.md @@ -196,11 +196,13 @@ browser sends one on every cross-site `POST`. Rejecting the absent case breaks e ### No CORS Headers (Default) -When `cors` is not specified, the server sends **no CORS headers**. On a loopback bind, the derived +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. +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`.