Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 92 additions & 101 deletions clients/mcpdo/__tests__/daemon-coverage.test.ts

Large diffs are not rendered by default.

48 changes: 30 additions & 18 deletions clients/mcpdo/__tests__/daemon-handshake.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
createDaemonHello,
} from "../src/daemon/handshake.js";
import type { ConnectParams } from "../src/daemon/protocol.js";
import { unixDaemonOnly } from "./helpers/platform.js";
import { daemonEndpoint } from "./helpers/daemon-endpoint.js";
import { publishFakeDaemonToken } from "./helpers/fake-daemon.js";

/** The victim's token, published in its daemon dir. */
Expand Down Expand Up @@ -95,9 +95,9 @@ function helloOf(line: string): { id: string; nonce: string } {

const unverified = { envelope: { code: "daemon_unverified" } };

// Skipped on win32: needs a Unix socket (helpers/platform.ts, #2748). The
// handshake itself is platform-independent; Windows is the platform it is for.
describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
// The handshake is platform-independent, and Windows is the platform it is
// for: a named pipe lives in a machine-wide namespace (see handshake.ts).
describe("daemon hello handshake", () => {
let dir: string | undefined;
const servers: net.Server[] = [];
let daemon: DaemonServer | undefined;
Expand All @@ -118,7 +118,12 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {

function freshSock(): string {
dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-hello-"));
return path.join(dir, "mcpdod.sock");
return daemonEndpoint(dir);
}

function freshDir(): string {
freshSock();
return dir!;
}

async function listenImpostor(sock: string, answer: Answer) {
Expand Down Expand Up @@ -201,12 +206,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
"callDaemon('connect') refuses an impostor answering with %s, having sent only the hello",
async (_label, answer) => {
const sock = freshSock();
publishFakeDaemonToken(sock, VICTIM_TOKEN);
publishFakeDaemonToken(dir!, VICTIM_TOKEN);
const { received } = await listenImpostor(sock, answer);
let elicited = false;
await expect(
callDaemon("connect", connectParams, {
socketPath: sock,
dir,
timeoutMs: 2000,
onElicitation: () => {
elicited = true;
Expand All @@ -223,14 +229,15 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
"streamDaemon refuses an impostor answering with %s, having sent only the hello",
async (_label, answer) => {
const sock = freshSock();
publishFakeDaemonToken(sock, VICTIM_TOKEN);
publishFakeDaemonToken(dir!, VICTIM_TOKEN);
const { received } = await listenImpostor(sock, answer);
const data: unknown[] = [];
await expect(
streamDaemon(
{ method: "logging/tail" },
{
socketPath: sock,
dir,
timeoutMs: 2000,
onData: (d) => {
data.push(d);
Expand All @@ -253,13 +260,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
delete process.env.MCP_INSPECTOR_DAEMON_TOKEN;
try {
await expect(
callDaemon("connect", connectParams, { socketPath: sock }),
callDaemon("connect", connectParams, { socketPath: sock, dir }),
).rejects.toMatchObject({
...unverified,
message: expect.stringContaining("No connection daemon token found"),
});
await expect(
streamDaemon({}, { socketPath: sock, onData: () => {} }),
streamDaemon({}, { socketPath: sock, dir, onData: () => {} }),
).rejects.toMatchObject(unverified);
// ensureDaemon finds the endpoint occupied and must not ping it.
await expect(ensureDaemon({ dir: dir! })).rejects.toMatchObject(
Expand All @@ -276,13 +283,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
it("without a token, nothing listening is still reported as unreachable", async () => {
const sock = freshSock();
await expect(
callDaemon("ping", {}, { socketPath: sock, timeoutMs: 2000 }),
callDaemon("ping", {}, { socketPath: sock, dir, timeoutMs: 2000 }),
).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" } });
});

it("ensureDaemon refuses an impostor on the endpoint even with a token on disk", async () => {
const sock = freshSock();
publishFakeDaemonToken(sock, VICTIM_TOKEN);
publishFakeDaemonToken(dir!, VICTIM_TOKEN);
const { received } = await listenImpostor(sock, (line, socket) =>
reply(socket, {
id: helloOf(line).id,
Expand All @@ -295,27 +302,32 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
});

it("the genuine daemon proves its token and serves calls and streams", async () => {
const sock = freshSock();
daemon = new DaemonServer({ dir: path.dirname(sock), idleMs: 0 });
const d = freshDir();
daemon = new DaemonServer({ dir: d, idleMs: 0 });
await daemon.start();
const pong = await callDaemon<{ pong: boolean }>(
"ping",
{},
{ socketPath: daemon.socketPath, timeoutMs: 2000 },
{ socketPath: daemon.socketPath, dir: d, timeoutMs: 2000 },
);
expect(pong.pong).toBe(true);
// A stream over an unknown connection is refused by the daemon itself —
// which it can only do once the handshake let the request through.
await expect(
streamDaemon(
{ name: "missing", method: "logging/tail" },
{ socketPath: daemon.socketPath, timeoutMs: 2000, onData: () => {} },
{
socketPath: daemon.socketPath,
dir: d,
timeoutMs: 2000,
onData: () => {},
},
),
).rejects.not.toMatchObject(unverified);
});

it("the daemon answers hello with the HMAC of the nonce, without the token, even while stopping", async () => {
const d = path.dirname(freshSock());
const d = freshDir();
daemon = new DaemonServer({ dir: d, idleMs: 0 });
await daemon.start();
const token = readDaemonTokenFile(d)!;
Expand Down Expand Up @@ -345,7 +357,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {

it("the daemon rejects a malformed hello nonce", async () => {
const srv = new DaemonServer({
dir: path.dirname(freshSock()),
dir: freshDir(),
idleMs: 0,
requiredToken: "t",
});
Expand All @@ -367,7 +379,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => {
delete process.env.MCP_INSPECTOR_DAEMON_TOKEN;
try {
const srv = new DaemonServer({
dir: path.dirname(freshSock()),
dir: freshDir(),
idleMs: 0,
});
const answer = await srv.handle({
Expand Down
150 changes: 102 additions & 48 deletions clients/mcpdo/__tests__/daemon-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,21 @@ import {
getDaemonSocketPath,
} from "../src/daemon/paths.js";
import { writeFormattedResult } from "@inspector/core/cli/handlers/format-output.js";
import { unixDaemonOnly } from "./helpers/platform.js";
import { isWindows } from "./helpers/platform.js";

/** Run `fn` with `process.platform` reporting `platform`. */
function withPlatform<T>(platform: NodeJS.Platform, fn: () => T): T {
const real = Object.getOwnPropertyDescriptor(
process,
"platform",
) as PropertyDescriptor;
Object.defineProperty(process, "platform", { value: platform });
try {
return fn();
} finally {
Object.defineProperty(process, "platform", real);
}
}

describe("daemon paths", () => {
const backup: Record<string, string | undefined> = {};
Expand All @@ -23,6 +37,8 @@ describe("daemon paths", () => {
"MCP_STORAGE_DIR",
"HOME",
"TMPDIR",
"TEMP",
"TMP",
]) {
if (key in backup) {
if (backup[key] === undefined) delete process.env[key];
Expand All @@ -38,23 +54,28 @@ describe("daemon paths", () => {
else process.env[key] = value;
}

// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748).
it.skipIf(unixDaemonOnly)(
"prefers MCP_INSPECTOR_DAEMON_DIR over MCP_STORAGE_DIR",
() => {
const a = path.join(os.tmpdir(), "daemon-a");
const b = path.join(os.tmpdir(), "daemon-b");
setEnv("MCP_STORAGE_DIR", b);
setEnv("MCP_INSPECTOR_DAEMON_DIR", a);
expect(getDaemonDir()).toBe(path.resolve(a));
it("prefers MCP_INSPECTOR_DAEMON_DIR over MCP_STORAGE_DIR", () => {
const a = path.join(os.tmpdir(), "daemon-a");
const b = path.join(os.tmpdir(), "daemon-b");
setEnv("MCP_STORAGE_DIR", b);
setEnv("MCP_INSPECTOR_DAEMON_DIR", a);
expect(getDaemonDir()).toBe(path.resolve(a));
if (isWindows) {
// The endpoint is a pipe keyed by the directory, not a file inside it.
expect(getDaemonSocketPath()).toMatch(
/^\\\\\.\\pipe\\mcp-conn-[0-9a-f]{16}$/,
);
expect(getDaemonSocketPath()).toBe(getDaemonSocketPath(path.resolve(a)));
expect(getDaemonSocketPath()).not.toBe(
getDaemonSocketPath(path.resolve(b)),
);
} else {
expect(getDaemonSocketPath()).toBe(
path.join(path.resolve(a), "mcpdod.sock"),
);
expect(getDaemonLockPath()).toBe(
path.join(path.resolve(a), "mcpdod.lock"),
);
},
);
}
expect(getDaemonLockPath()).toBe(path.join(path.resolve(a), "mcpdod.lock"));
});

it("falls back to MCP_STORAGE_DIR then ~/.mcp-inspector", () => {
const storage = path.join(os.tmpdir(), "daemon-storage");
Expand All @@ -72,8 +93,9 @@ describe("daemon paths", () => {
fs.rmSync(dir, { recursive: true, force: true });
});

// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748).
it.skipIf(unixDaemonOnly)(
// POSIX-only: asserts 0700 mode bits, and `ensureDaemonDir`'s ownership and
// mode checks are a no-op on Windows (no getuid, no UNIX mode semantics).
it.skipIf(isWindows)(
"tightens a pre-existing loose daemon directory to 0700 and rejects symlinks",
() => {
// mkdirSync never re-modes an existing dir; ~/.mcp-inspector commonly
Expand All @@ -94,26 +116,25 @@ describe("daemon paths", () => {
},
);

// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748).
it.skipIf(unixDaemonOnly)(
"createPrivateDaemonDir nests under a short 0700 tmpdir layout",
() => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-t-"));
setEnv("TMPDIR", tmp + path.sep);
const dir = createPrivateDaemonDir();
// $TMPDIR/mcp-conn-<uid>/<8-hex>; short enough that mcpdod.sock stays inside
// the platform sun_path limit even for macOS /var/folders tmpdirs.
expect(dir.startsWith(tmp)).toBe(true);
expect(path.basename(dir)).toMatch(/^[0-9a-f]{8}$/);
expect(path.basename(path.dirname(dir))).toMatch(/^mcp-conn-/);
expect(fs.statSync(dir).isDirectory()).toBe(true);
if (process.platform !== "win32") {
expect(fs.statSync(dir).mode & 0o777).toBe(0o700);
expect(fs.statSync(path.dirname(dir)).mode & 0o777).toBe(0o700);
}
fs.rmSync(tmp, { recursive: true, force: true });
},
);
it("createPrivateDaemonDir nests under a short 0700 tmpdir layout", () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-t-"));
// os.tmpdir() reads TMPDIR on POSIX and TEMP/TMP on Windows.
setEnv("TMPDIR", tmp + path.sep);
setEnv("TEMP", tmp);
setEnv("TMP", tmp);
const dir = createPrivateDaemonDir();
// $TMPDIR/mcp-conn-<uid>/<8-hex>; short enough that mcpdod.sock stays inside
// the platform sun_path limit even for macOS /var/folders tmpdirs.
expect(dir.startsWith(tmp)).toBe(true);
expect(path.basename(dir)).toMatch(/^[0-9a-f]{8}$/);
expect(path.basename(path.dirname(dir))).toMatch(/^mcp-conn-/);
expect(fs.statSync(dir).isDirectory()).toBe(true);
if (process.platform !== "win32") {
expect(fs.statSync(dir).mode & 0o777).toBe(0o700);
expect(fs.statSync(path.dirname(dir)).mode & 0o777).toBe(0o700);
}
fs.rmSync(tmp, { recursive: true, force: true });
});

it("createPrivateDaemonDir refuses a symlinked mcp-conn root", () => {
if (process.platform === "win32") return;
Expand Down Expand Up @@ -142,20 +163,53 @@ describe("daemon paths", () => {
fs.rmSync(tmp, { recursive: true, force: true });
});

// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748).
it.skipIf(unixDaemonOnly)(
"assertSocketPathWithinLimit rejects paths over the sun_path limit",
() => {
expect(() =>
assertSocketPathWithinLimit("/tmp/short/mcpdod.sock"),
).not.toThrow();
const long = "/" + "x".repeat(150) + "/mcpdod.sock";
expect(() => assertSocketPathWithinLimit(long)).toThrow(
/too long for this platform/,
);
// Each platform arm below is otherwise reached only on its own OS, and the
// per-file coverage gate runs on Linux, macOS and Windows alike (#2922), so
// the platform is faked rather than the case skipped.
it.each([
["linux", 107],
["darwin", 103],
] as const)(
"assertSocketPathWithinLimit enforces the %s sun_path limit (%i bytes)",
(platform, limit) => {
withPlatform(platform, () => {
const at = "/" + "x".repeat(limit - 1);
expect(Buffer.byteLength(at)).toBe(limit);
expect(() => assertSocketPathWithinLimit(at)).not.toThrow();
expect(() => assertSocketPathWithinLimit(at + "x")).toThrow(
/too long for this platform/,
);
});
},
);

it("derives a socket file inside the daemon dir off Windows", () => {
withPlatform("linux", () => {
expect(getDaemonSocketPath("/some/daemon/dir")).toBe(
path.join("/some/daemon/dir", "mcpdod.sock"),
);
});
});

it("assertTrustedPrivateRoot rejects a non-directory and accepts a root it owns", () => {
// The checks run wherever `process.getuid` exists; Windows has none, so
// stand in the owner's uid there to reach them on every host.
const realGetuid = process.getuid;
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-trust-"));
try {
const root = path.join(tmp, "root");
fs.mkdirSync(root, { mode: 0o755 });
process.getuid = realGetuid ?? (() => fs.lstatSync(root).uid);
const file = path.join(tmp, "not-a-dir");
fs.writeFileSync(file, "");
expect(() => assertTrustedPrivateRoot(file)).toThrow(/not a directory/);
expect(() => assertTrustedPrivateRoot(root)).not.toThrow();
} finally {
process.getuid = realGetuid;
fs.rmSync(tmp, { recursive: true, force: true });
}
});

it("uses a deterministic named-pipe path on Windows with no sun_path limit", () => {
const realPlatform = Object.getOwnPropertyDescriptor(
process,
Expand Down
Loading
Loading