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
11 changes: 6 additions & 5 deletions src/backends/native-https-owner-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { createServer, type Socket } from "node:net";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { isRecord } from "../lib/guards.ts";
import { listenPublishedUnixSocket } from "../lib/unix-socket-publish.ts";
import {
decodeNativeHttpsOwnerFrame,
encodeNativeHttpsOwnerFrame,
Expand Down Expand Up @@ -250,7 +251,8 @@ export async function serveNativeHttpsOwner(opts: {
}
};
const closeServer = async () => {
// Node may unlink its bound path on close. Refuse before invoking it when replaced.
// Refuse before closing when any owned path changed. The runtime's close-time
// unlink reaches only the retired staging name (listenPublishedUnixSocket).
await checkPaths();
for (const client of clients) {
client.destroy();
Expand Down Expand Up @@ -582,10 +584,9 @@ export async function serveNativeHttpsOwner(opts: {
}
};
void frontend.exited.then(exited, exited);
await new Promise<void>((resolve, reject) => {
server.once("error", reject);
server.listen(socketPath, resolve);
});
// Published by link from a staging name: closing the server never removes
// the control endpoint, which only retireSocketPath removes by identity.
await listenPublishedUnixSocket(server, socketPath);
const before = await lstat(socketPath);
await chmod(socketPath, 0o600);
const after = await lstat(socketPath);
Expand Down
21 changes: 5 additions & 16 deletions src/backends/native-project-https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { tmpdir } from "node:os";
import { isAbsolute, join } from "node:path";
import { connect as tlsConnect } from "node:tls";
import { isRecord } from "../lib/guards.ts";
import { listenPublishedUnixSocket } from "../lib/unix-socket-publish.ts";
import { checkNativeHttpsPort } from "./native-https-port.ts";
import {
invokeNativeRuntime,
Expand Down Expand Up @@ -709,22 +710,11 @@ async function startOwnerChallenge(
}
});
});
let listening = false;
let identity: { dev: number; ino: number } | undefined;
try {
await new Promise<void>((resolve, reject) => {
server.once("error", reject);
server.listen(path, () => {
server.off("error", reject);
listening = true;
resolve();
});
});
const metadata = await lstat(path);
if (!metadata.isSocket() || metadata.uid !== process.getuid?.()) {
throw refused();
}
identity = { dev: metadata.dev, ino: metadata.ino };
// Published by link from an owner-only staging name (a verified socket of
// this user): closing the server never removes a replacement at path.
identity = await listenPublishedUnixSocket(server, path);
await setMode(path, 0o600);
const prepared = await lstat(path);
if (
Expand All @@ -738,10 +728,9 @@ async function startOwnerChallenge(
}
return { server, publicKey, identity };
} catch {
// Without an identity, publication failed and already closed the server.
if (identity) {
await stopOwnerChallenge({ path, server, identity });
} else if (listening) {
await new Promise<void>((resolve) => server.close(() => resolve()));
}
throw refused();
}
Expand Down
141 changes: 141 additions & 0 deletions src/lib/unix-socket-publish.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { randomBytes } from "node:crypto";
import { link, lstat, unlink } from "node:fs/promises";
import type { Server } from "node:net";
import { basename, dirname, join } from "node:path";

/** Device and inode of a published Unix-socket endpoint. */
export interface UnixSocketIdentity {
readonly dev: number;
readonly ino: number;
}

/** Thrown when a file already exists at the requested endpoint path. */
export class UnixSocketEndpointExists extends Error {
readonly code = "EEXIST";
readonly path: string;
constructor(path: string) {
super("Unix socket endpoint already exists");
this.path = path;
}
}

function missing(error: unknown): boolean {
return (
typeof error === "object" &&
error !== null &&
"code" in error &&
error.code === "ENOENT"
);
}

async function removeIfSame(
path: string,
identity: UnixSocketIdentity
): Promise<void> {
const current = await lstat(path).catch((error: unknown) => {
if (missing(error)) {
return null;
}
throw error;
});
if (current && current.dev === identity.dev && current.ino === identity.ino) {
await unlink(path);
}
}

/**
* Listens on a Unix socket published at `path`, so that the runtime never replaces or removes a
* file at `path`.
*
* Bun replaces an existing file at a Unix server's bound path when it listens, and from 1.3.14 it
* unlinks the bound path by name when the server closes, whatever that path holds by then. Binding
* `path` directly would let a close remove a foreign replacement. Instead the server binds a fresh
* staging name in the same directory, never longer than `path`'s own name so path-length budgets
* hold, under an owner-only umask. It is then published with link(2), which refuses an existing
* `path` (EEXIST) rather than replacing it. The staging name is removed right away, so a
* close-time unlink by the runtime finds nothing. `path` is removed only by the caller's
* identity-checked cleanup, using the returned identity.
*
* The directory must be private to this user. On any failure after listening, the server is
* closed, the staging name is removed, and `path` is removed only while it still holds this
* server's socket.
*/
export async function listenPublishedUnixSocket(
server: Server,
path: string
): Promise<UnixSocketIdentity> {
const name = basename(path);
const staging = join(
dirname(path),
`.${randomBytes(Math.ceil(name.length / 2))
.toString("hex")
.slice(0, Math.max(4, name.length - 1))}`
);
if (
await lstat(staging).then(
() => true,
(error: unknown) => (missing(error) ? false : Promise.reject(error))
)
) {
throw new Error("Unix socket staging name is already in use");
}
await new Promise<void>((resolve, reject) => {
server.once("error", reject);
// Bun binds synchronously inside listen(); keep the socket owner-only from creation and
// restore the mask before any await.
const previousMask = process.umask();
process.umask(previousMask | 0o077);
try {
server.listen(staging, () => {
server.off("error", reject);
resolve();
});
} finally {
process.umask(previousMask);
}
});
let published: UnixSocketIdentity | undefined;
try {
const bound = await lstat(staging);
if (!bound.isSocket() || bound.uid !== process.getuid?.()) {
throw new Error(
"Unix socket staging endpoint changed before publication"
);
}
try {
await link(staging, path);
} catch (error) {
if (
typeof error === "object" &&
error !== null &&
"code" in error &&
error.code === "EEXIST"
) {
throw new UnixSocketEndpointExists(path);
}
throw error;
}
published = { dev: bound.dev, ino: bound.ino };
const current = await lstat(path);
if (
!current.isSocket() ||
current.dev !== bound.dev ||
current.ino !== bound.ino
) {
throw new Error("Unix socket endpoint changed during publication");
}
await unlink(staging);
return published;
} catch (error) {
await new Promise<void>((resolve) => server.close(() => resolve()));
await unlink(staging).catch((cleanup: unknown) => {
if (!missing(cleanup)) {
throw cleanup;
}
});
if (published) {
await removeIfSame(path, published);
}
throw error;
}
}
29 changes: 13 additions & 16 deletions src/mcp/socket-backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { isAbsolute, join } from "node:path";
import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
import { JSONRPCMessageSchema } from "@modelcontextprotocol/sdk/types.js";
import { isRecord } from "../lib/guards.ts";
import {
listenPublishedUnixSocket,
UnixSocketEndpointExists,
} from "../lib/unix-socket-publish.ts";
import { McpCommandAdmission } from "./command-admission.ts";
import { createMcpServer } from "./server.ts";
import { recordMcpSocketReceipt } from "./socket-receipt.ts";
Expand Down Expand Up @@ -173,24 +177,17 @@ async function prepareBackend(
throw new Error("MCP socket endpoint already exists");
}
startup.check();
await new Promise<void>((resolve, reject) => {
listener.once("error", reject);
// Bun binds a local Unix endpoint synchronously in listen(). Restrict its
// creation mode so concurrent adapters cannot observe a public socket
// before chmod. Restore immediately; never retain this mask across await
// or apply it to commands executed for connected clients.
const previousMask = process.umask();
process.umask(previousMask | 0o077);
try {
listener.listen(socketPath, () => {
listener.off("error", reject);
resolve();
});
} finally {
process.umask(previousMask);
// Published by link from an owner-only staging name, so no runtime close can
// remove a replacement at socketPath; only removeOwnedEndpoint removes it.
endpointIdentity = await listenPublishedUnixSocket(
listener,
socketPath
).catch((error: unknown) => {
if (error instanceof UnixSocketEndpointExists) {
throw new Error("MCP socket endpoint already exists");
}
throw error;
});
endpointIdentity = await lstat(socketPath);
startup.check();
await chmod(socketPath, 0o600);
startup.check();
Expand Down
3 changes: 2 additions & 1 deletion tests/cli-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
resolveCommand,
} from "../src/cli/command.ts";
import { CLI_SPEC } from "../src/cli/spec.ts";
import { restoreEnv } from "./helpers/env.ts";

type CapturedRunResult = {
readonly exitCode: number;
Expand All @@ -21,7 +22,7 @@ beforeEach(() => {
});

afterEach(() => {
process.env.HACK_LOGGER = originalLogger;
restoreEnv("HACK_LOGGER", originalLogger);
});

test("resolveCommand finds nested subcommand and remaining positionals", () => {
Expand Down
3 changes: 2 additions & 1 deletion tests/clipboard.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { afterEach, beforeEach, expect, test } from "bun:test";

import { copyToClipboard } from "../src/ui/clipboard.ts";
import { restoreEnv } from "./helpers/env.ts";

const ORIGINAL_PATH = process.env.PATH;

Expand All @@ -9,7 +10,7 @@ beforeEach(() => {
});

afterEach(() => {
process.env.PATH = ORIGINAL_PATH;
restoreEnv("PATH", ORIGINAL_PATH);
});

test("copyToClipboard returns error when no helper is available", async () => {
Expand Down
7 changes: 4 additions & 3 deletions tests/config-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from "node:fs/promises";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { restoreEnv } from "./helpers/env.ts";

let tempDir: string | null = null;
let originalHome: string | undefined;
Expand All @@ -30,9 +31,9 @@ afterEach(async () => {
await rm(tempDir, { recursive: true, force: true });
tempDir = null;
}
process.env.HOME = originalHome;
process.env.HACK_LOGGER = originalLogger;
process.env.HACK_GLOBAL_CONFIG_PATH = originalGlobalConfigPath;
restoreEnv("HOME", originalHome);
restoreEnv("HACK_LOGGER", originalLogger);
restoreEnv("HACK_GLOBAL_CONFIG_PATH", originalGlobalConfigPath);
});

test("config set --global updates extension enabled using bracket path", async () => {
Expand Down
12 changes: 8 additions & 4 deletions tests/config-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ const originalGlobalConfigPath = process.env.HACK_GLOBAL_CONFIG_PATH;
const tempDirs: string[] = [];

function restoreEnv(key: string, value: string | undefined): void {
process.env[key] = value;
if (value === undefined) {
Reflect.deleteProperty(process.env, key);
} else {
process.env[key] = value;
}
}

afterEach(async () => {
Expand Down Expand Up @@ -67,7 +71,7 @@ test("resolveGlobalConfigPath override beats HACK_HOME", () => {
});

test("resolveGlobalHackDir defaults to HOME/.hack when HACK_HOME is unset", () => {
process.env.HACK_HOME = undefined;
Reflect.deleteProperty(process.env, "HACK_HOME");
process.env.HACK_GLOBAL_CONFIG_PATH = "";
process.env.HOME = "/tmp/hack-home";

Expand Down Expand Up @@ -129,7 +133,7 @@ test("resolveDaemonPaths explicit home wins over HACK_HOME", () => {
});

test("resolveDaemonPaths is home-based when HACK_HOME is unset", () => {
process.env.HACK_HOME = undefined;
Reflect.deleteProperty(process.env, "HACK_HOME");
process.env.HOME = "/tmp/hack-home";

const paths = resolveDaemonPaths({});
Expand All @@ -142,7 +146,7 @@ test("projects registry is read from under HACK_HOME when set", async () => {
const hackHome = await makeTempDir("hack-home-");
process.env.HACK_HOME = hackHome;
process.env.HOME = "/tmp/hack-home";
process.env.HACK_GLOBAL_CONFIG_PATH = undefined;
Reflect.deleteProperty(process.env, "HACK_GLOBAL_CONFIG_PATH");

const registryPath = join(hackHome, GLOBAL_PROJECTS_REGISTRY_FILENAME);
const record = {
Expand Down
5 changes: 3 additions & 2 deletions tests/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
updateGlobalConfig,
updateProjectConfigBatch,
} from "../src/lib/config.ts";
import { restoreEnv } from "./helpers/env.ts";

describe("global config utilities", () => {
let tempDir: string;
Expand All @@ -23,9 +24,9 @@ describe("global config utilities", () => {

afterEach(async () => {
if (originalConfigPath !== undefined) {
process.env.HACK_GLOBAL_CONFIG_PATH = originalConfigPath;
restoreEnv("HACK_GLOBAL_CONFIG_PATH", originalConfigPath);
} else {
process.env.HACK_GLOBAL_CONFIG_PATH = undefined;
Reflect.deleteProperty(process.env, "HACK_GLOBAL_CONFIG_PATH");
}
await rm(tempDir, { recursive: true, force: true });
});
Expand Down
2 changes: 1 addition & 1 deletion tests/control-plane-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ afterEach(async () => {
tempGlobalConfig = null;
}
if (originalGlobalConfigPath === undefined) {
process.env.HACK_GLOBAL_CONFIG_PATH = undefined;
Reflect.deleteProperty(process.env, "HACK_GLOBAL_CONFIG_PATH");
} else {
process.env.HACK_GLOBAL_CONFIG_PATH = originalGlobalConfigPath;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/control-plane-extensions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ beforeEach(() => {

afterEach(() => {
if (originalGlobalConfigPath === undefined) {
process.env.HACK_GLOBAL_CONFIG_PATH = undefined;
Reflect.deleteProperty(process.env, "HACK_GLOBAL_CONFIG_PATH");
} else {
process.env.HACK_GLOBAL_CONFIG_PATH = originalGlobalConfigPath;
}
Expand Down
Loading
Loading