From 8dadba6b164c216ab636b32db44264d6ee7db8dc Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 11:59:28 -0400 Subject: [PATCH 1/6] fix: keep a published Unix socket's identity through startup failure A review of #109 found two startup ownership gaps. The native HTTPS owner discarded the identity listenPublishedUnixSocket returned and only recorded one after lstat, chmod and lstat of the path. A failure after publication then skipped listener and endpoint cleanup, and a replacement in that window could be chmodded or recorded as the endpoint. The owner now keeps the published identity at once, compares its first observation against it, and never changes the endpoint by path. Failure cleanup always closes the listener, which can no longer unlink anything but the retired staging name, and then removes the endpoint only while it is this listener's inode; a replacement is kept. The helper awaited listen() outside its cleanup, so a listen that bound the staging name and then failed could leak that socket and listener. Listening is now inside the cleanup, and a failed start removes only a staging socket this user created. The helper also sets the endpoint mode (0600) on the staging inode before publication, so the MCP backend, the HTTPS owner and the owner challenge no longer chmod the public path. Controls: bind-then-fail, mode-preparation failure, an endpoint at the AF_UNIX path limit (and one byte over: refused cleanly on Bun 1.3.9, bound in full on 1.4.2, never truncated), and an owner fault or replacement between publication and first observation, on Bun 1.3.9 and 1.4.2. --- src/backends/native-https-owner-server.ts | 35 ++++--- src/backends/native-project-https.ts | 3 +- src/lib/unix-socket-publish.ts | 106 ++++++++++++++++------ src/mcp/socket-backend.ts | 4 +- tests/native-https-owner.test.ts | 59 +++++++++++- tests/unix-socket-publish.test.ts | 80 +++++++++++++++- 6 files changed, 235 insertions(+), 52 deletions(-) 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..46bb454b 100644 --- a/src/backends/native-project-https.ts +++ b/src/backends/native-project-https.ts @@ -714,8 +714,7 @@ async function startOwnerChallenge( 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. - identity = await listenPublishedUnixSocket(server, path); - await setMode(path, 0o600); + identity = await listenPublishedUnixSocket(server, path, { setMode }); const prepared = await lstat(path); if ( !prepared.isSocket() || diff --git a/src/lib/unix-socket-publish.ts b/src/lib/unix-socket-publish.ts index a95d4d4f..fbe31a73 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 { chmod, link, lstat, unlink } from "node:fs/promises"; import type { Server } from "node:net"; import { basename, dirname, join } from "node:path"; @@ -43,6 +43,30 @@ async function removeIfSame( } } +/** + * Removes the staging name after a failed start: the socket this attempt bound (by identity once + * known) or, before that, only a socket of this user, since the fresh private name held nothing + * before listen. + */ +async function removeStaging( + staging: string, + bound: UnixSocketIdentity | undefined +): Promise { + const current = await lstat(staging).catch((error: unknown) => { + if (missing(error)) { + return null; + } + throw error; + }); + if ( + current?.isSocket() && + current.uid === process.getuid?.() && + (!bound || (current.dev === bound.dev && current.ino === bound.ino)) + ) { + await unlink(staging); + } +} + /** * Listens on a Unix socket published at `path`, so that the runtime never replaces or removes a * file at `path`. @@ -51,19 +75,28 @@ 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 + * hold, under an owner-only umask, and set to `mode` before publication. 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. + * The directory must be private to this user. On any failure, including a listen that bound the + * staging name and then failed, the server is closed, a staging socket this attempt created is + * removed, and `path` is removed only while it still holds this server's socket. Callers keep the + * returned identity at once and compare every later observation of `path` against it. */ export async function listenPublishedUnixSocket( server: Server, - path: string + path: string, + options: { + /** Endpoint permissions, applied before publication. */ + readonly mode?: number; + /** Changes a mode by path; replaceable to inject a preparation failure. */ + readonly setMode?: typeof chmod; + } = {} ): Promise { + const mode = options.mode ?? 0o600; + const setMode = options.setMode ?? chmod; const name = basename(path); const staging = join( dirname(path), @@ -79,25 +112,43 @@ export async function listenPublishedUnixSocket( ) { 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 bound: UnixSocketIdentity | undefined; let published: UnixSocketIdentity | undefined; 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 that socket or a listener behind. + 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); + } + }); + const created = await lstat(staging); + if (!created.isSocket() || created.uid !== process.getuid?.()) { + throw new Error( + "Unix socket staging endpoint changed before publication" + ); + } + bound = { dev: created.dev, ino: created.ino }; + // Set the mode before publication, so the endpoint is never changed by path + // afterwards (a replacement there is never chmodded or adopted). + await setMode(staging, mode); + const prepared = await lstat(staging); + if ( + !prepared.isSocket() || + prepared.dev !== bound.dev || + prepared.ino !== bound.ino || + (prepared.mode & 0o777) !== mode + ) { throw new Error( "Unix socket staging endpoint changed before publication" ); @@ -115,7 +166,7 @@ export async function listenPublishedUnixSocket( } throw error; } - published = { dev: bound.dev, ino: bound.ino }; + published = bound; const current = await lstat(path); if ( !current.isSocket() || @@ -127,12 +178,9 @@ export async function listenPublishedUnixSocket( await unlink(staging); return published; } catch (error) { + // Closing a server that never listened reports an error; either way it is closed. await new Promise((resolve) => server.close(() => resolve())); - await unlink(staging).catch((cleanup: unknown) => { - if (!missing(cleanup)) { - throw cleanup; - } - }); + await removeStaging(staging, bound); if (published) { await removeIfSame(path, published); } 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/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/unix-socket-publish.test.ts b/tests/unix-socket-publish.test.ts index db44202a..a5b23e33 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,79 @@ 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([]); +}); + +test("a mode preparation failure publishes nothing", async () => { + const directory = await privateDirectory(); + const path = join(directory, "owner.sock"); + const server = echoServer(); + await expect( + listenPublishedUnixSocket(server, path, { + setMode: async () => { + throw new Error("injected mode failure"); + }, + }) + ).rejects.toThrow("injected mode failure"); + expect(server.listening).toBe(false); + expect(await readdir(directory)).toEqual([]); +}); + +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([]); + } +}); From f91addc148fadd8f79c4687eed7f5cb7ea4687f8 Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 12:19:23 -0400 Subject: [PATCH 2/6] test: observe the MCP socket's private mode where it is now set The private-bind fixture hooked chmod on the public mcp.sock to prove the socket is private before its mode is set and that the creation mask is restored. Since the endpoint's mode is now set on its staging socket before publication, the hook never fired and the check failed. It now observes that staging chmod with the same privacy and mask checks, requires mode 0600, and fails if the published endpoint is ever chmodded by path. --- tests/fixtures/mcp/private-bind.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/fixtures/mcp/private-bind.ts b/tests/fixtures/mcp/private-bind.ts index 6ede6fab..fadb7c3f 100644 --- a/tests/fixtures/mcp/private-bind.ts +++ b/tests/fixtures/mcp/private-bind.ts @@ -1,19 +1,25 @@ import { mock } from "bun:test"; 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 directory = await original.realpath(root); +const socketPath = join(directory, "mcp.sock"); process.umask(0o022); let observed = false; mock.module("node:fs/promises", () => ({ ...original, chmod: async (path: string, mode: number) => { + // The endpoint mode is set on its staging socket before publication; the + // published endpoint must never be changed by path. if (path === socketPath) { + throw new Error("Published endpoint was chmodded by path"); + } + if (dirname(path) === directory && basename(path).startsWith(".")) { const socket = await original.lstat(path); if (!socket.isSocket() || (socket.mode & 0o077) !== 0) { throw new Error("Socket was public before chmod"); @@ -21,6 +27,9 @@ mock.module("node:fs/promises", () => ({ if (process.umask() !== 0o022) { throw new Error("Creation mask leaked across await"); } + if (mode !== 0o600) { + throw new Error("Unexpected endpoint mode"); + } observed = true; } return original.chmod(path, mode); From 70c78f77fb737475238608dae12e2d97cd860a6b Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 12:52:40 -0400 Subject: [PATCH 3/6] test: serialize the frontend acceptance stand-in's shared state Stand-in invocations ran concurrently (a foreground `up` while the driver polls `ps`) and each did an unlocked read-modify-write of state.json. A `ps` that loaded the state before the second `up` saved its foreground token then saved its stale copy over it, so `up` saw its token gone and exited 0 before readiness; lost call records failed the acceptance test as well. That failed 26 of 40 local runs, and Runtime state models on CI. Each invocation now holds an exclusive flock across its read-modify-write and releases it only before its long waits (the foreground loop and the ps-hang fault). 40 of 40 local runs pass. --- tests/python/test_native_frontend_acceptance.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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"], From 5d99981d1a318ae08c874800b948142e84ffb6b6 Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 13:31:08 -0400 Subject: [PATCH 4/6] fix: never remove or chmod an unproven staging entry when publishing a socket A second review of the Unix-socket publication found three staging-path windows where an entry this attempt could not prove was its own could be changed or removed: - After an ambiguous partial bind (no recorded identity), cleanup unlinked any same-uid socket at the staging name. - The mode was set by path after an awaited lstat, so a replacement in that gap could be chmodded before the postcheck refused it. - On success the staging name was unlinked unconditionally after the awaited link and endpoint lstat. The socket is now created with exactly its mode (the umask during the synchronous bind), so nothing is ever chmodded, and its identity is recorded in the same tick. Publication links only while the staging name is still that socket, and retirement removes it only while it is (check and removal back to back). An entry that is not this socket, or cannot be proven to be, is never removed: on failure it is moved aside while the server closes, so the runtime's close-time unlink by name cannot reach it, and then put back with the same inode. The owner challenge's chmod dependency becomes an afterOwnerSocketPublish test seam, and the private-bind fixture now requires the socket to be created private with mode 0600 and no socket name to be chmodded. --- src/backends/native-project-https.ts | 23 ++- src/lib/unix-socket-publish.ts | 209 +++++++++++++++------------ tests/fixtures/mcp/private-bind.ts | 53 ++++--- tests/native-project-https.test.ts | 2 +- tests/unix-socket-publish.test.ts | 98 ++++++++++++- 5 files changed, 264 insertions(+), 121 deletions(-) diff --git a/src/backends/native-project-https.ts b/src/backends/native-project-https.ts index 46bb454b..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,9 +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. - identity = await listenPublishedUnixSocket(server, path, { setMode }); + // 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 afterPublish(path); const prepared = await lstat(path); if ( !prepared.isSocket() || @@ -1252,7 +1256,7 @@ export async function startNativeProjectHttps(opts: { spawn: spawnNativeHttpsChild, permissionPort, adminReady, - chmodOwnerSocket: chmod, + afterOwnerSocketPublish: async () => undefined, ...opts.dependencies, }; const limit = opts.certificateNameLimit ?? 256; @@ -1371,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 fbe31a73..138d5e66 100644 --- a/src/lib/unix-socket-publish.ts +++ b/src/lib/unix-socket-publish.ts @@ -1,5 +1,11 @@ import { randomBytes } from "node:crypto"; -import { chmod, link, lstat, unlink } from "node:fs/promises"; +import { + linkSync, + lstatSync, + renameSync, + type Stats, + unlinkSync, +} from "node:fs"; import type { Server } from "node:net"; import { basename, dirname, join } from "node:path"; @@ -19,51 +25,86 @@ 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; } -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); } } /** - * Removes the staging name after a failed start: the socket this attempt bound (by identity once - * known) or, before that, only a socket of this user, since the fresh private name held nothing - * before listen. + * 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 fresh 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). */ -async function removeStaging( +async function closeKeepingStaging( + server: Server, staging: string, bound: UnixSocketIdentity | undefined ): Promise { - const current = await lstat(staging).catch((error: unknown) => { - if (missing(error)) { - return null; + let held: string | undefined; + if (entry(staging)) { + const candidate = `${staging}.${randomBytes(4).toString("hex")}`; + if (!entry(candidate)) { + renameSync(staging, candidate); + held = candidate; + } + } + // 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; } - throw error; - }); - if ( - current?.isSocket() && - current.uid === process.getuid?.() && - (!bound || (current.dev === bound.dev && current.ino === bound.ino)) - ) { - await unlink(staging); } } @@ -75,28 +116,31 @@ async function removeStaging( * 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, and set to `mode` before publication. 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. * - * The directory must be private to this user. On any failure, including a listen that bound the - * staging name and then failed, the server is closed, a staging socket this attempt created is - * removed, and `path` is removed only while it still holds this server's socket. Callers keep the - * returned identity at once and compare every later observation of `path` against it. + * The directory must be private to this user. On any failure the server is closed without its + * close-time unlink reaching an unproven staging entry, 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 that cannot be proven to be (an ambiguous partial bind), is kept. After a + * successful return the staging name is retired; an entry placed there later is outside this + * function, and a runtime close-time unlink by name can still reach it. */ export async function listenPublishedUnixSocket( server: Server, path: string, options: { - /** Endpoint permissions, applied before publication. */ + /** Endpoint permissions, set at creation. */ readonly mode?: number; - /** Changes a mode by path; replaceable to inject a preparation failure. */ - readonly setMode?: typeof chmod; + readonly hooks?: UnixSocketPublishHooks; } = {} ): Promise { const mode = options.mode ?? 0o600; - const setMode = options.setMode ?? chmod; + const hooks = options.hooks ?? {}; const name = basename(path); const staging = join( dirname(path), @@ -104,85 +148,70 @@ 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"); } let bound: UnixSocketIdentity | undefined; - let published: UnixSocketIdentity | undefined; + let published = false; try { // Inside the cleanup: a runtime can create the staging socket and still fail - // the listen, which must not leave that socket or a listener behind. + // the listen, which must not leave a listener behind. 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. + // Bun binds synchronously inside listen(): create the socket with exactly + // `mode` and restore the mask before any await. const previousMask = process.umask(); - process.umask(previousMask | 0o077); + 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); } }); - const created = await lstat(staging); - if (!created.isSocket() || created.uid !== process.getuid?.()) { + if (!bound) { throw new Error( - "Unix socket staging endpoint changed before publication" + "Unix socket staging endpoint is not this server's socket" ); } - bound = { dev: created.dev, ino: created.ino }; - // Set the mode before publication, so the endpoint is never changed by path - // afterwards (a replacement there is never chmodded or adopted). - await setMode(staging, mode); - const prepared = await lstat(staging); - if ( - !prepared.isSocket() || - prepared.dev !== bound.dev || - prepared.ino !== bound.ino || - (prepared.mode & 0o777) !== mode - ) { + 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 = bound; - 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) { - // Closing a server that never listened reports an error; either way it is closed. - await new Promise((resolve) => server.close(() => resolve())); - await removeStaging(staging, bound); - if (published) { - await removeIfSame(path, published); + await closeKeepingStaging(server, staging, bound); + if (published && bound) { + removeIfSame(path, bound); } throw error; } diff --git a/tests/fixtures/mcp/private-bind.ts b/tests/fixtures/mcp/private-bind.ts index fadb7c3f..f507a090 100644 --- a/tests/fixtures/mcp/private-bind.ts +++ b/tests/fixtures/mcp/private-bind.ts @@ -1,4 +1,5 @@ import { mock } from "bun:test"; +import * as fsSync from "node:fs"; import * as fs from "node:fs/promises"; import { basename, dirname, join } from "node:path"; @@ -7,32 +8,47 @@ if (!(root && sourceRoot)) { throw new Error("Missing fixture paths"); } const original = { ...fs }; +const originalSync = { ...fsSync }; const directory = await original.realpath(root); -const socketPath = join(directory, "mcp.sock"); 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) => { - // The endpoint mode is set on its staging socket before publication; the - // published endpoint must never be changed by path. - if (path === socketPath) { - throw new Error("Published endpoint was chmodded by path"); - } - if (dirname(path) === directory && basename(path).startsWith(".")) { - 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"); - } - if (mode !== 0o600) { - throw new Error("Unexpected endpoint mode"); + 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( @@ -43,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-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/unix-socket-publish.test.ts b/tests/unix-socket-publish.test.ts index a5b23e33..6c15b2c4 100644 --- a/tests/unix-socket-publish.test.ts +++ b/tests/unix-socket-publish.test.ts @@ -223,19 +223,107 @@ test("a listen that binds the staging name and then fails leaves no socket or li expect(await readdir(directory)).toEqual([]); }); -test("a mode preparation failure publishes nothing", async () => { +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, { - setMode: async () => { - throw new Error("injected mode failure"); + hooks: { + afterBind: async (name) => { + staging = name; + await unlink(name); + original = await foreignFile(name); + }, }, }) - ).rejects.toThrow("injected mode failure"); + ).rejects.toThrow("changed before publication"); expect(server.listening).toBe(false); - expect(await readdir(directory)).toEqual([]); + // 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 endpoint at the AF_UNIX path limit publishes and serves, and one byte over is refused cleanly or served at its full name", async () => { From 83571a61e5fe33a36b294b377e40630364601a44 Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 13:50:55 -0400 Subject: [PATCH 5/6] fix: move an unproven staging entry aside without overwriting a holding entry The failure close moved an unproven staging entry to one random holding name: if that name was occupied the close went ahead unprotected, and a holding entry created between the check and the rename was overwritten. The entry is now moved with link(2), which never replaces a holding entry, over a bounded list of fresh holding names, and the staging name is dropped only while it is still the linked entry. An entry that cannot be moved aside (every holding name occupied, or not hard-linkable) leaves the close to proceed, now documented as a residual. The helper's documentation now states its scope: accidental and concurrent entries in a private directory, not an adversarial process of the same user, with the remaining path-based and post-return residuals. --- src/lib/unix-socket-publish.ts | 95 +++++++++++++++++++++++-------- tests/unix-socket-publish.test.ts | 35 ++++++++++++ 2 files changed, 106 insertions(+), 24 deletions(-) diff --git a/src/lib/unix-socket-publish.ts b/src/lib/unix-socket-publish.ts index 138d5e66..7cbefb2b 100644 --- a/src/lib/unix-socket-publish.ts +++ b/src/lib/unix-socket-publish.ts @@ -1,11 +1,5 @@ import { randomBytes } from "node:crypto"; -import { - linkSync, - lstatSync, - renameSync, - type Stats, - unlinkSync, -} from "node:fs"; +import { linkSync, lstatSync, type Stats, unlinkSync } from "node:fs"; import type { Server } from "node:net"; import { basename, dirname, join } from "node:path"; @@ -31,6 +25,18 @@ export interface UnixSocketPublishHooks { 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")}` + ); } function errorCode(error: unknown): unknown { @@ -69,26 +75,53 @@ function removeIfSame(path: string, identity: UnixSocketIdentity): void { } } +/** + * 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 fresh holding name while the server closes. Afterwards this attempt's socket + * 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 + bound: UnixSocketIdentity | undefined, + holdingNames: (staging: string) => readonly string[] ): Promise { - let held: string | undefined; - if (entry(staging)) { - const candidate = `${staging}.${randomBytes(4).toString("hex")}`; - if (!entry(candidate)) { - renameSync(staging, candidate); - held = candidate; - } - } + 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) { @@ -123,12 +156,21 @@ async function closeKeepingStaging( * identity-checked cleanup, using the returned identity; callers keep it at once and compare every * later observation of `path` against it. * - * The directory must be private to this user. On any failure the server is closed without its - * close-time unlink reaching an unproven staging entry, 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 that cannot be proven to be (an ambiguous partial bind), is kept. After a - * successful return the staging name is retired; an entry placed there later is outside this - * function, and a runtime close-time unlink by name can still reach 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. + * + * 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, @@ -209,7 +251,12 @@ export async function listenPublishedUnixSocket( removeIfSame(staging, bound); return bound; } catch (error) { - await closeKeepingStaging(server, staging, bound); + await closeKeepingStaging( + server, + staging, + bound, + hooks.holdingNames ?? freshHoldingNames + ); if (published && bound) { removeIfSame(path, bound); } diff --git a/tests/unix-socket-publish.test.ts b/tests/unix-socket-publish.test.ts index 6c15b2c4..dc644f08 100644 --- a/tests/unix-socket-publish.test.ts +++ b/tests/unix-socket-publish.test.ts @@ -326,6 +326,41 @@ test("a replacement after publication is kept when the staging name is retired", ); }); +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 From a459762a92fd98d2b30d5378febe97e9ee91ea06 Mon Sep 17 00:00:00 2001 From: hack-cli-tests Date: Wed, 30 Sep 2026 09:11:43 -0400 Subject: [PATCH 6/6] build: pin Bun 1.4.2 across mise, CI, release and runtime images Bun 1.3.9 intermittently segfaulted inside its own socket poll dispatch while running tests/native-https-owner.test.ts on the macOS test job (2 of 60 recent test jobs, HACK-1211). Bun 1.4 carries upstream fixes to that usockets dispatch path. Every repository pin moves together: mise.toml, .tool-versions, packageManager, the CI and release workflows, the node and slim runtime images, and the native candidate build's version check. The lockfile is unchanged; a frozen install succeeds. --- .github/workflows/ci.yml | 10 +++++----- .github/workflows/release-node-runtime-image.yml | 2 +- .github/workflows/release-prepare.yml | 2 +- .github/workflows/release.yml | 6 +++--- .tool-versions | 2 +- docker/node-runtime/Dockerfile | 2 +- docker/slim-runtime/Dockerfile | 4 ++-- docs/guides/native-candidate.md | 2 +- mise.toml | 2 +- package.json | 2 +- scripts/build-native-candidate.sh | 4 ++-- 11 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5bc74da..d26c64f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - uses: actions/setup-java@v4 with: distribution: temurin @@ -114,7 +114,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Setup Blacksmith Builder uses: useblacksmith/setup-docker-builder@v1 - name: Install dependencies @@ -151,7 +151,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Install tmux run: sudo apt-get update && sudo apt-get install --yes tmux - name: Install dependencies @@ -182,7 +182,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Install dependencies run: bun install - name: Typecheck (Turbo) @@ -206,7 +206,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Require native regression tools run: | command -v python3 diff --git a/.github/workflows/release-node-runtime-image.yml b/.github/workflows/release-node-runtime-image.yml index ecc0250e..21f92665 100644 --- a/.github/workflows/release-node-runtime-image.yml +++ b/.github/workflows/release-node-runtime-image.yml @@ -27,7 +27,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Setup QEMU uses: docker/setup-qemu-action@v3 diff --git a/.github/workflows/release-prepare.yml b/.github/workflows/release-prepare.yml index e7690cf4..ba5001c0 100644 --- a/.github/workflows/release-prepare.yml +++ b/.github/workflows/release-prepare.yml @@ -33,7 +33,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Install dependencies run: bun install diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 47943bfa..ca0b09ae 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,7 +26,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Resolve version id: version run: | @@ -78,7 +78,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Install dependencies run: bun install - name: Build release artifacts @@ -171,7 +171,7 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@v1 with: - bun-version: "1.3.9" + bun-version: "1.4.2" - name: Render formula env: GH_TOKEN: ${{ github.token }} diff --git a/.tool-versions b/.tool-versions index 7eb3673c..e7521662 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ -bun 1.3.9 +bun 1.4.2 nodejs 24.11.0 zig 0.15.2 diff --git a/docker/node-runtime/Dockerfile b/docker/node-runtime/Dockerfile index 23b8b1c7..58eca2a7 100644 --- a/docker/node-runtime/Dockerfile +++ b/docker/node-runtime/Dockerfile @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.7 -FROM oven/bun:1.3.9 AS hack-build +FROM oven/bun:1.4.2 AS hack-build WORKDIR /src COPY package.json bun.lock bunfig.toml tsconfig.json index.ts ./ diff --git a/docker/slim-runtime/Dockerfile b/docker/slim-runtime/Dockerfile index 17ef7f3d..2b8b17cc 100644 --- a/docker/slim-runtime/Dockerfile +++ b/docker/slim-runtime/Dockerfile @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.7 -FROM oven/bun:1.3.9 AS hack-build +FROM oven/bun:1.4.2 AS hack-build WORKDIR /src COPY package.json bun.lock bunfig.toml tsconfig.json index.ts ./ @@ -10,7 +10,7 @@ COPY src ./src RUN bun install --frozen-lockfile RUN bun build index.ts --compile --outfile /out/hack -FROM oven/bun:1.3.9 +FROM oven/bun:1.4.2 ARG HACK_VERSION=dev ARG BUILD_DATE=unknown diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 258774a1..eb3c3e89 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -6,7 +6,7 @@ projects, or install DNS or trust automatically. Its graph commands support a bounded subset; this bundle is not application parity or release qualification. -Build with the repository's pinned Bun 1.3.9, Rust 1.97.1 and Zig 0.15.2 toolchain, Python 3, +Build with the repository's pinned Bun 1.4.2, Rust 1.97.1 and Zig 0.15.2 toolchain, Python 3, and the Rust `aarch64-unknown-linux-musl` standard library already installed. The build refuses missing prerequisites; it does not install toolchains: diff --git a/mise.toml b/mise.toml index bcde2cf9..805b4e6c 100644 --- a/mise.toml +++ b/mise.toml @@ -1,5 +1,5 @@ [tools] -bun = "1.3.9" +bun = "1.4.2" node = "24.11.0" zig = "0.15.2" rust = "1.97.1" diff --git a/package.json b/package.json index 7ca6004d..1226a649 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "hack-cli", "version": "4.2.1", - "packageManager": "bun@1.3.9", + "packageManager": "bun@1.4.2", "module": "index.ts", "type": "module", "workspaces": [ diff --git a/scripts/build-native-candidate.sh b/scripts/build-native-candidate.sh index 65dae289..12cd7a2a 100755 --- a/scripts/build-native-candidate.sh +++ b/scripts/build-native-candidate.sh @@ -25,8 +25,8 @@ if [ "$(zig version)" != 0.15.2 ]; then exit 69 fi command -v python3 >/dev/null -if [ "$(bun --version)" != 1.3.9 ]; then - echo "Pinned Bun 1.3.9 is required" >&2 +if [ "$(bun --version)" != 1.4.2 ]; then + echo "Pinned Bun 1.4.2 is required" >&2 exit 69 fi stdlib=$(rustc --print target-libdir --target aarch64-unknown-linux-musl)