diff --git a/src/backends/native-https-owner-server.ts b/src/backends/native-https-owner-server.ts index 54269bf4..8a167530 100644 --- a/src/backends/native-https-owner-server.ts +++ b/src/backends/native-https-owner-server.ts @@ -1,5 +1,4 @@ import { - chmod, lstat, mkdtemp, readdir, @@ -51,6 +50,11 @@ export interface NativeHttpsOwnerServerDependencies { phase: "acquire" | "release" ) => Promise; readonly verifyIdle: (binding: NativeHttpsOwnerBinding) => Promise; + /** + * Test seam: runs after the control socket is published and before its first + * ownership observation. Production passes none. + */ + readonly afterPublish?: (socketPath: string) => Promise; } function verifyCleanGraphObservations( @@ -251,9 +255,10 @@ export async function serveNativeHttpsOwner(opts: { } }; const closeServer = async () => { - // Refuse before closing when any owned path changed. The runtime's close-time - // unlink reaches only the retired staging name (listenPublishedUnixSocket). - await checkPaths(); + // Always close the listener: the runtime's close-time unlink reaches only the + // retired staging name (listenPublishedUnixSocket). retireSocketPath then + // removes the endpoint only while it is this listener's inode; a replacement + // is kept and the failure retained. for (const client of clients) { client.destroy(); } @@ -584,21 +589,21 @@ export async function serveNativeHttpsOwner(opts: { } }; void frontend.exited.then(exited, exited); - // 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); + // Keep the published inode at once: failure cleanup then closes this listener + // and retires only this inode, and every later observation compares against it. + // The helper set its mode before publication, so the path is never chmodded. + socketIdentity = await listenPublishedUnixSocket(server, socketPath); + await opts.dependencies.afterPublish?.(socketPath); + const published = await lstat(socketPath); if ( - !after.isSocket() || - before.dev !== after.dev || - before.ino !== after.ino || - after.uid !== process.getuid?.() + !published.isSocket() || + published.dev !== socketIdentity.dev || + published.ino !== socketIdentity.ino || + published.uid !== process.getuid?.() || + (published.mode & 0o777) !== 0o600 ) { throw nativeHttpsOwnerRefused(); } - socketIdentity = { dev: after.dev, ino: after.ino }; if (frontendFailed) { throw nativeHttpsOwnerRefused(); } diff --git a/src/backends/native-project-https.ts b/src/backends/native-project-https.ts index 0e61e891..9be34807 100644 --- a/src/backends/native-project-https.ts +++ b/src/backends/native-project-https.ts @@ -9,7 +9,6 @@ import { } from "node:crypto"; import { closeSync, constants, mkdtempSync, openSync, rmSync } from "node:fs"; import { - chmod, link, lstat, mkdir, @@ -69,7 +68,11 @@ interface Dependencies { readonly spawn: (input: SpawnInput) => HttpsChild; readonly permissionPort: () => Promise; readonly adminReady: (socket: string) => Promise; - readonly chmodOwnerSocket: typeof chmod; + /** + * Test seam: runs after the owner socket is published and before it is + * verified. Production does nothing. + */ + readonly afterOwnerSocketPublish: (path: string) => Promise; } const SAFE_TLS_CODES = new Set([ "ERR_SSL_TLSV1_ALERT_INTERNAL_ERROR", @@ -684,7 +687,7 @@ function caDerSha256(pem: Buffer): string { } async function startOwnerChallenge( path: string, - setMode: typeof chmod + afterPublish: (path: string) => Promise ): Promise<{ readonly server: Server; readonly publicKey: string; @@ -712,10 +715,10 @@ async function startOwnerChallenge( }); let identity: { dev: number; ino: number } | undefined; try { - // Published by link from an owner-only staging name (a verified socket of - // this user): closing the server never removes a replacement at path. + // Created with mode 0600 and published by link from a staging name: closing + // the server never removes a replacement at path, and path is never chmodded. identity = await listenPublishedUnixSocket(server, path); - await setMode(path, 0o600); + await afterPublish(path); const prepared = await lstat(path); if ( !prepared.isSocket() || @@ -1253,7 +1256,7 @@ export async function startNativeProjectHttps(opts: { spawn: spawnNativeHttpsChild, permissionPort, adminReady, - chmodOwnerSocket: chmod, + afterOwnerSocketPublish: async () => undefined, ...opts.dependencies, }; const limit = opts.certificateNameLimit ?? 256; @@ -1372,7 +1375,10 @@ export async function startNativeProjectHttps(opts: { await removeOwnedDirectory(lock, lockIdentity, false); })()); try { - owner = await startOwnerChallenge(ownerSocket, deps.chmodOwnerSocket); + owner = await startOwnerChallenge( + ownerSocket, + deps.afterOwnerSocketPublish + ); const before = await inspect(); if ( !( diff --git a/src/lib/unix-socket-publish.ts b/src/lib/unix-socket-publish.ts index a95d4d4f..7cbefb2b 100644 --- a/src/lib/unix-socket-publish.ts +++ b/src/lib/unix-socket-publish.ts @@ -1,5 +1,5 @@ import { randomBytes } from "node:crypto"; -import { link, lstat, unlink } from "node:fs/promises"; +import { linkSync, lstatSync, type Stats, unlinkSync } from "node:fs"; import type { Server } from "node:net"; import { basename, dirname, join } from "node:path"; @@ -19,27 +19,125 @@ export class UnixSocketEndpointExists extends Error { } } -function missing(error: unknown): boolean { - return ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" +/** Test seams between publication steps; production passes none. */ +export interface UnixSocketPublishHooks { + /** After this server bound the staging name and its identity was recorded. */ + readonly afterBind?: (staging: string) => Promise; + /** After the endpoint was linked and verified, before the staging name is retired. */ + readonly afterLink?: (staging: string) => Promise; + /** Holding names tried, in order, when an unproven staging entry is moved aside. */ + readonly holdingNames?: (staging: string) => readonly string[]; +} + +/** How many fresh holding names are tried before a close proceeds without one. */ +const HOLDING_ATTEMPTS = 8; + +function freshHoldingNames(staging: string): string[] { + return Array.from( + { length: HOLDING_ATTEMPTS }, + () => `${staging}.${randomBytes(4).toString("hex")}` ); } -async function removeIfSame( - path: string, - identity: UnixSocketIdentity -): Promise { - const current = await lstat(path).catch((error: unknown) => { - if (missing(error)) { +function errorCode(error: unknown): unknown { + return typeof error === "object" && error !== null && "code" in error + ? error.code + : undefined; +} + +function entry(path: string): Stats | null { + try { + return lstatSync(path); + } catch (error) { + if (errorCode(error) === "ENOENT") { return null; } throw error; - }); - if (current && current.dev === identity.dev && current.ino === identity.ino) { - await unlink(path); + } +} + +function same( + stat: Stats | null, + identity: UnixSocketIdentity | undefined +): boolean { + return ( + stat !== null && + identity !== undefined && + stat.dev === identity.dev && + stat.ino === identity.ino + ); +} + +/** Removes `path` only while it is `identity`; the check and the removal run back to back. */ +function removeIfSame(path: string, identity: UnixSocketIdentity): void { + if (same(entry(path), identity)) { + unlinkSync(path); + } +} + +/** + * Moves the staging entry to the first free holding name: link(2) never replaces an existing + * holding entry, and the staging name is dropped only while it is still the linked entry. + * Returns the holding name, or undefined when no holding name was free or the entry cannot be + * hard-linked (the entry stays put). + */ +function moveAside( + staging: string, + names: readonly string[] +): string | undefined { + for (const candidate of names) { + try { + linkSync(staging, candidate); + } catch (error) { + if (errorCode(error) === "EEXIST") { + continue; + } + // Not linkable (a directory, for example): it cannot be moved aside. + return undefined; + } + const held = entry(candidate); + if (held && same(entry(staging), { dev: held.dev, ino: held.ino })) { + unlinkSync(staging); + } + return candidate; + } + return undefined; +} + +/** + * Closes `server` without letting its close-time unlink by name (Bun 1.3.14 and later, as in Node) + * reach an entry at the staging name that this attempt cannot prove is its own. Whatever the name + * holds is moved to a free holding name while the server closes. Afterwards this attempt's socket + * is removed; anything else is put back by link, which never replaces a newer entry, so its inode, + * bytes and mode are unchanged (if the name was taken meanwhile, it stays at the holding name). + * If none of the bounded holding names is free, or the entry cannot be hard-linked, the close + * proceeds and the runtime can remove the entry. + */ +async function closeKeepingStaging( + server: Server, + staging: string, + bound: UnixSocketIdentity | undefined, + holdingNames: (staging: string) => readonly string[] +): Promise { + const held = entry(staging) + ? moveAside(staging, holdingNames(staging)) + : undefined; + // Closing a server that never listened reports an error; either way it is closed. + await new Promise((resolve) => server.close(() => resolve())); + if (!held) { + return; + } + if (bound && same(entry(held), bound)) { + unlinkSync(held); + return; + } + try { + linkSync(held, staging); + unlinkSync(held); + } catch (error) { + if (errorCode(error) !== "EEXIST") { + throw error; + } } } @@ -51,19 +149,40 @@ async function removeIfSame( * 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. + * hold. The socket is created with exactly `mode` (the umask during the synchronous bind), so no + * name is ever chmodded, and its identity is recorded in the same tick. It is published with + * link(2), which refuses an existing `path` (EEXIST) rather than replacing it, and the staging name + * is then retired only while it is still this socket. `path` is removed only by the caller's + * identity-checked cleanup, using the returned identity; callers keep it at once and compare every + * later observation of `path` against it. + * + * On any failure the server is closed, only this attempt's socket is removed, and `path` is + * removed only while it still holds this socket. An entry at the staging name that is not this + * socket, or cannot be proven to be (an ambiguous partial bind), is moved to a free holding name + * for the close and put back. * - * 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. + * Scope: the directory must be private to this user. These guarantees cover accidental and + * concurrent entries, not an adversarial process of the same user, which can race any path-based + * check. Residuals: + * - Each check and its operation run back to back, but by path: an entry changed between them is + * outside the guarantee. + * - If the staging entry cannot be moved aside (every bounded holding name occupied, or an entry + * that cannot be hard-linked, such as a directory), the failure close proceeds and the runtime's + * close-time unlink can remove it. + * - After a successful return the staging name is retired. The runtime's normal close later + * unlinks that name, and can remove an entry placed there afterwards. */ export async function listenPublishedUnixSocket( server: Server, - path: string + path: string, + options: { + /** Endpoint permissions, set at creation. */ + readonly mode?: number; + readonly hooks?: UnixSocketPublishHooks; + } = {} ): Promise { + const mode = options.mode ?? 0o600; + const hooks = options.hooks ?? {}; const name = basename(path); const staging = join( dirname(path), @@ -71,70 +190,75 @@ export async function listenPublishedUnixSocket( .toString("hex") .slice(0, Math.max(4, name.length - 1))}` ); - if ( - await lstat(staging).then( - () => true, - (error: unknown) => (missing(error) ? false : Promise.reject(error)) - ) - ) { + if (entry(staging)) { throw new Error("Unix socket staging name is already in use"); } - await new Promise((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; + let bound: UnixSocketIdentity | undefined; + let published = false; try { - const bound = await lstat(staging); - if (!bound.isSocket() || bound.uid !== process.getuid?.()) { + // Inside the cleanup: a runtime can create the staging socket and still fail + // the listen, which must not leave a listener behind. + await new Promise((resolve, reject) => { + server.once("error", reject); + // Bun binds synchronously inside listen(): create the socket with exactly + // `mode` and restore the mask before any await. + const previousMask = process.umask(); + process.umask(0o777 & ~mode); + try { + server.listen(staging, () => { + server.off("error", reject); + resolve(); + }); + // Record what this server bound before anything else runs here. + const created = entry(staging); + if ( + created?.isSocket() && + created.uid === process.getuid?.() && + (created.mode & 0o777) === mode + ) { + bound = { dev: created.dev, ino: created.ino }; + } + } finally { + process.umask(previousMask); + } + }); + if (!bound) { + throw new Error( + "Unix socket staging endpoint is not this server's socket" + ); + } + await hooks.afterBind?.(staging); + // Publish only this server's socket; the check and the link run back to back. + if (!same(entry(staging), bound)) { throw new Error( "Unix socket staging endpoint changed before publication" ); } try { - await link(staging, path); + linkSync(staging, path); } catch (error) { - if ( - typeof error === "object" && - error !== null && - "code" in error && - error.code === "EEXIST" - ) { + if (errorCode(error) === "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 - ) { + published = true; + if (!same(entry(path), bound)) { throw new Error("Unix socket endpoint changed during publication"); } - await unlink(staging); - return published; + await hooks.afterLink?.(staging); + // Retire the staging name only while it is this socket; anything else is kept. + removeIfSame(staging, bound); + return bound; } catch (error) { - await new Promise((resolve) => server.close(() => resolve())); - await unlink(staging).catch((cleanup: unknown) => { - if (!missing(cleanup)) { - throw cleanup; - } - }); - if (published) { - await removeIfSame(path, published); + await closeKeepingStaging( + server, + staging, + bound, + hooks.holdingNames ?? freshHoldingNames + ); + if (published && bound) { + removeIfSame(path, bound); } throw error; } diff --git a/src/mcp/socket-backend.ts b/src/mcp/socket-backend.ts index b617350a..41a141ff 100644 --- a/src/mcp/socket-backend.ts +++ b/src/mcp/socket-backend.ts @@ -1,4 +1,4 @@ -import { chmod, lstat, mkdir, realpath, rmdir, unlink } from "node:fs/promises"; +import { lstat, mkdir, realpath, rmdir, unlink } from "node:fs/promises"; import { createServer, type Socket } from "node:net"; import { isAbsolute, join } from "node:path"; import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; @@ -189,8 +189,6 @@ async function prepareBackend( throw error; }); startup.check(); - await chmod(socketPath, 0o600); - startup.check(); releaseReceipt = await recordMcpSocketReceipt({ directory, claim: claimStat, diff --git a/tests/fixtures/mcp/private-bind.ts b/tests/fixtures/mcp/private-bind.ts index 6ede6fab..f507a090 100644 --- a/tests/fixtures/mcp/private-bind.ts +++ b/tests/fixtures/mcp/private-bind.ts @@ -1,29 +1,54 @@ import { mock } from "bun:test"; +import * as fsSync from "node:fs"; import * as fs from "node:fs/promises"; -import { join } from "node:path"; +import { basename, dirname, join } from "node:path"; const [root, sourceRoot] = process.argv.slice(2); if (!(root && sourceRoot)) { throw new Error("Missing fixture paths"); } const original = { ...fs }; -const socketPath = join(await original.realpath(root), "mcp.sock"); +const originalSync = { ...fsSync }; +const directory = await original.realpath(root); process.umask(0o022); let observed = false; +const refuseChmod = (path: unknown) => { + // The socket is created private with its final mode; no socket name is ever + // chmodded, so a replacement there can never be changed by path. + if (typeof path === "string" && dirname(path) === directory) { + throw new Error("A socket name was chmodded by path"); + } +}; mock.module("node:fs/promises", () => ({ ...original, chmod: async (path: string, mode: number) => { - if (path === socketPath) { - const socket = await original.lstat(path); - if (!socket.isSocket() || (socket.mode & 0o077) !== 0) { - throw new Error("Socket was public before chmod"); - } - if (process.umask() !== 0o022) { - throw new Error("Creation mask leaked across await"); + refuseChmod(path); + return original.chmod(path, mode); + }, +})); +mock.module("node:fs", () => ({ + ...originalSync, + chmodSync: (path: string, mode: number) => { + refuseChmod(path); + return originalSync.chmodSync(path, mode); + }, + lstatSync: (...args: Parameters) => { + const stat = originalSync.lstatSync(...args); + const [path] = args; + // The first observation of the staging socket, in the tick that created it. + if ( + !observed && + stat?.isSocket() && + typeof path === "string" && + dirname(path) === directory && + basename(path).startsWith(".") + ) { + if ((Number(stat.mode) & 0o777) !== 0o600) { + throw new Error("Socket was not created private with mode 0600"); } observed = true; } - return original.chmod(path, mode); + return stat; }, })); const { startMcpSocketBackend } = await import( @@ -34,6 +59,9 @@ const backend = await startMcpSocketBackend({ backendId: "private-bind", idleTimeoutMs: 50, }); +if (process.umask() !== 0o022) { + throw new Error("Creation mask leaked past startup"); +} await backend.closed; if (!observed || process.umask() !== 0o022) { throw new Error("Private bind was not observed or mask was not restored"); diff --git a/tests/native-https-owner.test.ts b/tests/native-https-owner.test.ts index 00262d0c..339a944f 100644 --- a/tests/native-https-owner.test.ts +++ b/tests/native-https-owner.test.ts @@ -11,7 +11,7 @@ import { } from "node:fs/promises"; import type { Socket } from "node:net"; import { tmpdir } from "node:os"; -import { join, resolve } from "node:path"; +import { dirname, join, resolve } from "node:path"; import { connectNativeHttpsOwner, ensureNativeHttpsOwner, @@ -72,6 +72,8 @@ async function fixture( options: { readonly immediateExit?: boolean; readonly holdRetirement?: boolean; + /** Fault injected between control-socket publication and its first observation. */ + readonly afterPublish?: "fail" | "replace"; } = {} ) { const home = await mkdtemp(join(await realpath(tmpdir()), "hk-owner-test-")); @@ -83,8 +85,9 @@ async function fixture( frontend, `#!${process.execPath} import { serveNativeHttpsOwner } from ${JSON.stringify(serverModule)}; -import { appendFile, readFile, writeFile } from "node:fs/promises"; +import { appendFile, readFile, unlink, writeFile } from "node:fs/promises"; const home = ${JSON.stringify(home)}; +const fault = ${JSON.stringify(options.afterPublish ?? null)}; await writeFile(home + "/helper-pid", String(process.pid)); const never = ${options.immediateExit ? 'Promise.resolve({component:"fixture",code:1})' : "new Promise(() => {})"}; try { @@ -99,6 +102,11 @@ try { verify: async (_binding, lease, phase) => { if (phase === "release" && await readFile(home + "/clean-" + lease.run, "utf8") !== "clean") { throw new Error("unproven"); } }, + afterPublish: async (path) => { + await writeFile(home + "/published-path", path); + if (fault === "fail") { throw new Error("injected failure after publication"); } + if (fault === "replace") { await unlink(path); await writeFile(path, "foreign replacement", { mode: 0o644 }); } + }, verifyIdle: async () => { ${ options.holdRetirement @@ -470,6 +478,53 @@ test("child exit during startup never publishes a reusable running endpoint", as await expect(connectNativeHttpsOwner(configuration, false)).rejects.toThrow(); }, 20_000); +test("a failure or replacement right after publication closes the listener and retires only its own endpoint", async () => { + for (const fault of ["fail", "replace"] as const) { + const f = await fixture({ afterPublish: fault }); + await ensureNativeHttpsOwner({ binding: f.binding }); + const published = await until(() => + readFile(join(f.home, "published-path"), "utf8") + ); + const pid = Number( + await until(() => readFile(join(f.home, "helper-pid"), "utf8")) + ); + // The owner exits: a listener left open would keep it running. + await until(async () => { + try { + process.kill(pid, 0); + } catch { + return true; + } + throw new Error("owner still running"); + }); + await expect( + lstat(join(nativeHttpsOwnerRoot(f.home), "endpoint.json")) + ).rejects.toThrow(); + if (fault === "fail") { + // Its own endpoint and private socket directory are gone. + await expect(lstat(published)).rejects.toThrow(); + await expect(lstat(dirname(published))).rejects.toThrow(); + } else { + // The replacement is kept exactly: never removed, adopted or chmodded. + const stat = await lstat(published); + expect(stat.isFile()).toBe(true); + expect(stat.mode & 0o777).toBe(0o644); + expect(await readFile(published, "utf8")).toBe("foreign replacement"); + await rm(dirname(published), { recursive: true, force: true }); + } + } + // A later owner starts and serves normally. + const f = await fixture(); + const started = await f.start(); + const identity = await f.acquire( + started.socket, + started.configuration.ownerGeneration, + "b" + ); + await f.clean("b"); + await f.release(started.socket, identity); +}, 40_000); + test("dead helper and mismatched generation or binary binding are never adopted", async () => { const f = await fixture(); const started = await f.start(); diff --git a/tests/native-project-https.test.ts b/tests/native-project-https.test.ts index dd42e4ad..24211324 100644 --- a/tests/native-project-https.test.ts +++ b/tests/native-project-https.test.ts @@ -405,7 +405,7 @@ test("owner socket preparation failure retires only its bound socket and lock", ...f.opts, dependencies: { ...f.opts.dependencies, - chmodOwnerSocket: async () => { + afterOwnerSocketPublish: async () => { throw new Error("simulated socket preparation failure"); }, }, diff --git a/tests/python/test_native_frontend_acceptance.py b/tests/python/test_native_frontend_acceptance.py index 5b1bf371..09329f82 100644 --- a/tests/python/test_native_frontend_acceptance.py +++ b/tests/python/test_native_frontend_acceptance.py @@ -22,11 +22,16 @@ # One stand-in serves as both hack-native and hack-v5 (by file name) over a shared state file. # `up` refuses unless a prepared base is required, then stays in the foreground until `down`. FAKE = r'''#!PYTHON -import json, os, pathlib, secrets, sys, time +import fcntl, json, os, pathlib, secrets, sys, time here = pathlib.Path(__file__).parent name = pathlib.Path(__file__).name path = here / "state.json" fault = os.environ.get("FAKE_FAULT", "") +# Invocations run concurrently (a foreground `up` while the driver polls `ps`). Each holds this +# exclusive lock across its read-modify-write of the shared state, so no invocation saves a stale +# copy over another's update; it is released only before a long wait. +lock = open(here / "state.lock", "a") +fcntl.flock(lock, fcntl.LOCK_EX) def load(): @@ -86,6 +91,7 @@ def reply(body, code=0): up_env={k: v for k, v in os.environ.items() if k.startswith("HACK_NATIVE_")}) state.setdefault("run", "r" + secrets.token_hex(8)) save(state) + fcntl.flock(lock, fcntl.LOCK_UN) deadline = time.time() + 30 while time.time() < deadline: time.sleep(0.02) @@ -94,6 +100,7 @@ def reply(body, code=0): sys.exit(3) if argv[0] == "ps": if fault == "ps-hang": + fcntl.flock(lock, fcntl.LOCK_UN) time.sleep(10) if state.get("foreground"): reply({"status": "observed", "phase": "ready-observed", "backend": "native", "run": state["run"], diff --git a/tests/unix-socket-publish.test.ts b/tests/unix-socket-publish.test.ts index db44202a..dc644f08 100644 --- a/tests/unix-socket-publish.test.ts +++ b/tests/unix-socket-publish.test.ts @@ -1,7 +1,9 @@ import { afterEach, expect, test } from "bun:test"; +import { EventEmitter } from "node:events"; import { chmod, lstat, + mkdir, mkdtemp, readdir, readFile, @@ -96,7 +98,7 @@ test("a published endpoint serves clients, survives close and is removed only by expect(await readdir(directory)).toEqual(["mcp.sock"]); const stat = await lstat(path); expect(stat.isSocket()).toBe(true); - expect(stat.mode & 0o777 & 0o077).toBe(0); + expect(stat.mode & 0o777).toBe(0o600); expect({ dev: stat.dev, ino: stat.ino }).toEqual(identity); expect(await reply(path)).toBe("ok"); await close(server); @@ -192,3 +194,202 @@ test("a startup that cannot bind creates nothing", async () => { expect(server.listening).toBe(false); expect(await readdir(directory)).toEqual([]); }); + +test("a listen that binds the staging name and then fails leaves no socket or listener", async () => { + const directory = await privateDirectory(); + const path = join(directory, "control.sock"); + const inner = createServer(); + servers.push(inner); + let closed = false; + // A runtime that creates the staging socket and then reports a listen error. + const partial = Object.assign(new EventEmitter(), { + listen(staging: string) { + inner.listen(staging, () => { + partial.emit("error", new Error("injected failure after bind")); + }); + return partial; + }, + close(callback?: () => void) { + closed = true; + inner.close(() => callback?.()); + return partial; + }, + }); + await expect( + listenPublishedUnixSocket(partial as unknown as Server, path) + ).rejects.toThrow("injected failure after bind"); + expect(closed).toBe(true); + expect(inner.listening).toBe(false); + expect(await readdir(directory)).toEqual([]); +}); + +async function foreignFile(path: string) { + await writeFile(path, "foreign bytes", { mode: 0o644 }); + const stat = await lstat(path); + return { ino: stat.ino, mode: stat.mode & 0o777 }; +} + +async function expectForeignFileKept( + path: string, + original: { ino: number; mode: number } +) { + const stat = await lstat(path); + expect({ ino: stat.ino, mode: stat.mode & 0o777 }).toEqual(original); + expect(await readFile(path, "utf8")).toBe("foreign bytes"); +} + +test("an ambiguous partial bind keeps a foreign same-uid socket at the staging name", async () => { + const directory = await privateDirectory(); + const path = join(directory, "control.sock"); + const foreign = createServer((socket) => socket.end("foreign")); + servers.push(foreign); + let closed = false; + let original: { ino: number; mode: number } | undefined; + // A listen that never proves it bound anything, while a foreign socket takes the name. + const partial = Object.assign(new EventEmitter(), { + listen(staging: string) { + setTimeout(() => { + foreign.listen(staging, async () => { + const stat = await lstat(staging); + original = { ino: stat.ino, mode: stat.mode & 0o777 }; + partial.emit("error", new Error("injected ambiguous bind failure")); + }); + }, 10); + return partial; + }, + close(callback?: () => void) { + closed = true; + callback?.(); + return partial; + }, + }); + await expect( + listenPublishedUnixSocket(partial as unknown as Server, path) + ).rejects.toThrow("injected ambiguous bind failure"); + expect(closed).toBe(true); + const [staging] = (await readdir(directory)).filter((name) => + name.startsWith(".") + ); + expect(staging).toBeDefined(); + const stat = await lstat(join(directory, staging ?? "")); + expect({ ino: stat.ino, mode: stat.mode & 0o777 }).toEqual( + original ?? { ino: 0, mode: 0 } + ); + expect(await reply(join(directory, staging ?? ""))).toBe("foreign"); + expect(await readdir(directory)).toEqual([staging ?? ""]); +}); + +test("a replacement after bind is refused and kept: never chmodded, adopted or removed", async () => { + const directory = await privateDirectory(); + const path = join(directory, "owner.sock"); + const server = echoServer(); + let staging = ""; + let original: { ino: number; mode: number } | undefined; + await expect( + listenPublishedUnixSocket(server, path, { + hooks: { + afterBind: async (name) => { + staging = name; + await unlink(name); + original = await foreignFile(name); + }, + }, + }) + ).rejects.toThrow("changed before publication"); + expect(server.listening).toBe(false); + // The helper's own server close could not remove it either. + await expectForeignFileKept(staging, original ?? { ino: 0, mode: 0 }); + expect(await readdir(directory)).toEqual([basename(staging)]); +}); + +test("a replacement after publication is kept when the staging name is retired", async () => { + const directory = await privateDirectory(); + const path = join(directory, "mcp.sock"); + const server = echoServer(); + let staging = ""; + let original: { ino: number; mode: number } | undefined; + const identity = await listenPublishedUnixSocket(server, path, { + hooks: { + afterLink: async (name) => { + staging = name; + await unlink(name); + original = await foreignFile(name); + }, + }, + }); + const stat = await lstat(path); + expect({ dev: stat.dev, ino: stat.ino }).toEqual(identity); + expect(await reply(path)).toBe("ok"); + await expectForeignFileKept(staging, original ?? { ino: 0, mode: 0 }); + expect((await readdir(directory)).sort()).toEqual( + [basename(staging), "mcp.sock"].sort() + ); +}); + +test("an occupied holding name is never overwritten; the next free one keeps the staging entry", async () => { + const directory = await privateDirectory(); + const path = join(directory, "owner.sock"); + const server = echoServer(); + let staging = ""; + let original: { ino: number; mode: number } | undefined; + let occupied: { ino: number; mode: number } | undefined; + await expect( + listenPublishedUnixSocket(server, path, { + hooks: { + afterBind: async (name) => { + staging = name; + await unlink(name); + original = await foreignFile(name); + await writeFile(`${name}.held-a`, "holding bytes", { mode: 0o640 }); + const stat = await lstat(`${name}.held-a`); + occupied = { ino: stat.ino, mode: stat.mode & 0o777 }; + }, + holdingNames: (name) => [`${name}.held-a`, `${name}.held-b`], + }, + }) + ).rejects.toThrow("changed before publication"); + expect(server.listening).toBe(false); + // The staging entry went to the free name for the close and came back unchanged. + await expectForeignFileKept(staging, original ?? { ino: 0, mode: 0 }); + const held = await lstat(`${staging}.held-a`); + expect({ ino: held.ino, mode: held.mode & 0o777 }).toEqual( + occupied ?? { ino: 0, mode: 0 } + ); + expect(await readFile(`${staging}.held-a`, "utf8")).toBe("holding bytes"); + expect((await readdir(directory)).sort()).toEqual( + [basename(staging), `${basename(staging)}.held-a`].sort() + ); +}); + +test("an endpoint at the AF_UNIX path limit publishes and serves, and one byte over is refused cleanly or served at its full name", async () => { + // sun_path holds 104 bytes on macOS and 108 on Linux, including the terminating NUL. + // Bun 1.3.9 refuses a longer path; Bun 1.4 binds it in full. Neither may truncate it + // or leave a partial staging entry. + const limit = process.platform === "darwin" ? 103 : 107; + const name = "mcp.sock"; + for (const extra of [0, 1]) { + const directory = await privateDirectory(); + const fill = limit - Buffer.byteLength(directory) - 2 - name.length + extra; + expect(fill).toBeGreaterThan(0); + const parent = join(directory, "d".repeat(fill)); + await mkdir(parent, { mode: 0o700 }); + const path = join(parent, name); + expect(Buffer.byteLength(path)).toBe(limit + extra); + const server = echoServer(); + const identity = await listenPublishedUnixSocket(server, path).catch( + () => null + ); + if (identity) { + const stat = await lstat(path); + expect({ dev: stat.dev, ino: stat.ino }).toEqual(identity); + expect(await reply(path)).toBe("ok"); + expect(await readdir(parent)).toEqual([name]); + await close(server); + await removeOwned(path, identity); + } else { + expect(extra).toBe(1); + expect(server.listening).toBe(false); + } + expect(await readdir(parent)).toEqual([]); + } +});