From 81cbdec9ee7a77f937c28be7babf0fe1095a7a5b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 08:52:24 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20tests:=20test=20the=20lock-o?= =?UTF-8?q?wner=20check=20itself,=20keep=20make's=20dry=20runs=20in=20a=20?= =?UTF-8?q?temp=20runtime=20dir,=20and=20refuse=20a=20default=20path=20tha?= =?UTF-8?q?t=20does=20not=20fit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red: assertLockPathUsable is not exported, and the chooser creates the folder before it checks the length of the default path. --- scripts/devServerAuth.test.ts | 45 +++++++++++++++---- src/node/utils/privateUnixSocket.test.ts | 16 +++++-- .../utils/privateUnixSocketFolder.test.ts | 14 ++++++ 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/scripts/devServerAuth.test.ts b/scripts/devServerAuth.test.ts index d6a7f707fb3..a9b0a982f54 100644 --- a/scripts/devServerAuth.test.ts +++ b/scripts/devServerAuth.test.ts @@ -1,10 +1,16 @@ -import { afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import * as fsSync from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; const REPO_ROOT = path.resolve(import.meta.dir, ".."); +// make claims the default socket folder at parse time, in `$XDG_RUNTIME_DIR` or /tmp. A dry run must +// not leave one on the test host, so every make call here gets a runtime dir of its own. +const RUNTIME_BASE = fsSync.mkdtempSync(path.join(os.tmpdir(), "xum-make-base-")); +afterAll(() => fsSync.rmSync(RUNTIME_BASE, { recursive: true, force: true })); + interface DryRun { /** XUM_BACKEND_SOCKET as make exports it to the recipe's processes (env, not secret). */ socket: string | undefined; @@ -23,7 +29,12 @@ async function dryRun( ): Promise { const proc = Bun.spawn(["make", "-pn", ...makeFlags, goal], { cwd: REPO_ROOT, - env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "", ...env }, + env: { + PATH: process.env.PATH ?? "", + HOME: process.env.HOME ?? "", + XDG_RUNTIME_DIR: RUNTIME_BASE, + ...env, + }, stdout: "pipe", stderr: "pipe", }); @@ -250,7 +261,7 @@ describe.skipIf(process.platform === "win32")("make dev-server backend socket", test("the backend listens on the exported path, and the path fits sun_path", async () => { const socket = exportedSocket(socketRun); - expect(socket.startsWith("/")).toBe(true); + expect(socket.startsWith(`${RUNTIME_BASE}${path.sep}`)).toBe(true); expect(socket).toMatch(/b3111-127\.0\.0\.1\.sock$/); expect(Buffer.byteLength(socket)).toBeLessThanOrEqual(100); expect(await backendSocketArgument(socketRun)).toBe(socket); @@ -344,26 +355,44 @@ describe.skipIf(process.platform === "win32")("make dev-server backend socket", expect(await backendSocketArgument(run)).toBe(socket); }); - test("make stops with a message when the parent cannot be trusted", async () => { - const shared = path.join(runtimeDir, "shared"); - await fs.mkdir(shared); - await fs.chmod(shared, 0o777); + /** `make -pn dev-server` that is expected to stop: its exit code and stderr. */ + async function makeStops(xdgRuntimeDir: string) { const proc = Bun.spawn(["make", "-pn", "dev-server"], { cwd: REPO_ROOT, env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "", - XDG_RUNTIME_DIR: shared, + XDG_RUNTIME_DIR: xdgRuntimeDir, }, stdout: "pipe", stderr: "pipe", }); const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]); + return { exitCode, stderr }; + } + + test("make stops with a message when the parent cannot be trusted", async () => { + const shared = path.join(runtimeDir, "shared"); + await fs.mkdir(shared); + await fs.chmod(shared, 0o777); + const { exitCode, stderr } = await makeStops(shared); expect(exitCode).not.toBe(0); expect(stderr).toContain("XUM_BACKEND_SOCKET"); expect(stderr).toContain("XDG_RUNTIME_DIR"); expect(await fs.readdir(shared)).toEqual([]); }); + + // The backend refuses a path that does not fit sun_path, so a folder made for it would be an + // empty folder for nothing: make stops first, and creates nothing. + test("make stops with a message when the default path is too long, and creates nothing", async () => { + const deep = path.join(runtimeDir, "d".repeat(100)); + await fs.mkdir(deep, { mode: 0o700 }); + const { exitCode, stderr } = await makeStops(deep); + expect(exitCode).not.toBe(0); + expect(stderr).toMatch(/too long/); + expect(stderr).toContain("XUM_BACKEND_SOCKET"); + expect(await fs.readdir(deep)).toEqual([]); + }); }); test("Windows ignores an ambient XUM_BACKEND_SOCKET", () => { diff --git a/src/node/utils/privateUnixSocket.test.ts b/src/node/utils/privateUnixSocket.test.ts index b4dae5afbd4..ceaf69fe5b5 100644 --- a/src/node/utils/privateUnixSocket.test.ts +++ b/src/node/utils/privateUnixSocket.test.ts @@ -5,7 +5,7 @@ import * as net from "node:net"; import * as os from "node:os"; import * as path from "node:path"; import { acquireProcessFileLock } from "@/node/utils/concurrency/fileLock"; -import { withPrivateUnixSocketPath } from "./privateUnixSocket"; +import { assertLockPathUsable, withPrivateUnixSocketPath } from "./privateUnixSocket"; import { MAX_UNIX_SOCKET_PATH_BYTES } from "./privateUnixSocketFolder"; const posix = process.platform !== "win32"; @@ -337,12 +337,22 @@ describe.skipIf(!posix)("withPrivateUnixSocketPath", () => { expect((await fs.lstat(`${socketPath}.lock`)).isSymbolicLink()).toBe(true); }); + // Called on its own: through withPrivateUnixSocketPath a foreign uid fails the folder check first, + // and only root could make a file of another user inside our private folder. The file here is + // ours, and the expected owner is not. test("refuses a lock file that another user owns", async () => { await fs.mkdir(dir, { mode: 0o700 }); - await fs.writeFile(`${socketPath}.lock`, "someone else", { mode: 0o600 }); - await expect(prepare(socketPath, process.getuid!() + 1)).rejects.toThrow( + const lockPath = `${socketPath}.lock`; + await fs.writeFile(lockPath, "someone else", { mode: 0o600 }); + await expect(assertLockPathUsable(lockPath, process.getuid!() + 1)).rejects.toThrow( /owned by another user/ ); + await assertLockPathUsable(lockPath, process.getuid!()); + }); + + test("accepts a missing lock file", async () => { + await fs.mkdir(dir, { mode: 0o700 }); + await assertLockPathUsable(`${socketPath}.lock`, process.getuid!()); }); test("refuses a lock path that is a folder", async () => { diff --git a/src/node/utils/privateUnixSocketFolder.test.ts b/src/node/utils/privateUnixSocketFolder.test.ts index 346daec5805..b33a9e8fec4 100644 --- a/src/node/utils/privateUnixSocketFolder.test.ts +++ b/src/node/utils/privateUnixSocketFolder.test.ts @@ -47,6 +47,20 @@ describe.skipIf(!posix)("chooseDevSocketPath", () => { expect(stat.mode & 0o777).toBe(0o700); }); + test("a default path that cannot fit is refused before the folder is created", async () => { + const error = await chooseDevSocketPath( + preferred, + "s".repeat(MAX_UNIX_SOCKET_PATH_BYTES), + uid + ).then( + () => null, + (e: unknown) => e + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/too long/); + expect(await fs.readdir(base)).toEqual([]); + }); + test("a private folder of ours is the answer", async () => { await fs.mkdir(preferred, { mode: 0o700 }); const choice = await chooseDevSocketPath(preferred, "b1.sock", uid); From 1dff9b6e298b1e01b84da90ac8e648bf1eff622c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 08:55:39 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A4=96=20fix:=20export=20the=20lock?= =?UTF-8?q?=20path=20check=20for=20its=20test,=20and=20refuse=20a=20defaul?= =?UTF-8?q?t=20socket=20path=20that=20does=20not=20fit=20before=20make=20c?= =?UTF-8?q?reates=20the=20folder?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/node/utils/privateUnixSocket.ts | 2 +- src/node/utils/privateUnixSocketFolder.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node/utils/privateUnixSocket.ts b/src/node/utils/privateUnixSocket.ts index b46e344bba6..5ed19204768 100644 --- a/src/node/utils/privateUnixSocket.ts +++ b/src/node/utils/privateUnixSocket.ts @@ -56,7 +56,7 @@ async function removeStaleSocket(socketPath: string, ownerUid: number): Promise< } /** The lock file must be a regular file of ours: a symlink would send the lock somewhere else. */ -async function assertLockPathUsable(lockPath: string, ownerUid: number): Promise { +export async function assertLockPathUsable(lockPath: string, ownerUid: number): Promise { let entry; try { entry = await fs.lstat(lockPath); diff --git a/src/node/utils/privateUnixSocketFolder.ts b/src/node/utils/privateUnixSocketFolder.ts index 7c5c16d93fc..1c085b40127 100644 --- a/src/node/utils/privateUnixSocketFolder.ts +++ b/src/node/utils/privateUnixSocketFolder.ts @@ -130,6 +130,7 @@ const MKDTEMP_SUFFIX = ".XXXXXX"; * caller to tell the user. The Vite hop never falls back to TCP: the squat window that the socket * closes would open again. * + * Throws, before it creates anything, when the default path does not fit a socket path. * Throws when a parent of the folder is unsafe: a fresh folder next to it would be just as open to * another account, so only the user can fix it (XUM_BACKEND_SOCKET or XDG_RUNTIME_DIR). It also * throws, before it creates the fresh folder, when the longer fallback path would not fit. @@ -140,6 +141,9 @@ export async function chooseDevSocketPath( ownerUid: number ): Promise { const preferred = path.join(folder, fileName); + // Before anything is created: the backend refuses a path that does not fit, and the folder made + // for it would stay empty. + assertSocketPathFits(preferred); await assertParentsTrusted(folder, ownerUid); try { await fs.mkdir(folder, { mode: 0o700 }); From 88f5e8de813f7c2b1b7567e7da7e4dc8bc7171bd Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 4 Nov 2026 09:10:38 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A4=96=20tests:=20build=20the=20make?= =?UTF-8?q?=20tests'=20runtime=20dirs=20under=20/tmp=20so=20the=20socket?= =?UTF-8?q?=20path=20fits=20on=20macOS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review on #7913. os.tmpdir() on macOS is a long /var/folders path, and the default socket path under it exceeds the 103-byte limit that the chooser now checks. --- scripts/devServerAuth.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/devServerAuth.test.ts b/scripts/devServerAuth.test.ts index a9b0a982f54..c10b1fa6431 100644 --- a/scripts/devServerAuth.test.ts +++ b/scripts/devServerAuth.test.ts @@ -8,7 +8,11 @@ const REPO_ROOT = path.resolve(import.meta.dir, ".."); // make claims the default socket folder at parse time, in `$XDG_RUNTIME_DIR` or /tmp. A dry run must // not leave one on the test host, so every make call here gets a runtime dir of its own. -const RUNTIME_BASE = fsSync.mkdtempSync(path.join(os.tmpdir(), "xum-make-base-")); +// +// The base is under /tmp, not os.tmpdir(): macOS's os.tmpdir() is a long /var/folders/... path, and +// `/xum-dev-/b3111-192.168.1.5.sock` must fit the 103 bytes of a macOS socket path. +const SHORT_TMP = process.platform === "win32" ? os.tmpdir() : "/tmp"; +const RUNTIME_BASE = fsSync.mkdtempSync(path.join(SHORT_TMP, "xum-make-base-")); afterAll(() => fsSync.rmSync(RUNTIME_BASE, { recursive: true, force: true })); interface DryRun { @@ -307,7 +311,7 @@ describe.skipIf(process.platform === "win32")("make dev-server backend socket", let runtimeDir: string; beforeEach(async () => { - runtimeDir = await fs.mkdtemp(path.join(os.tmpdir(), "xum-make-test-")); + runtimeDir = await fs.mkdtemp(path.join(SHORT_TMP, "xum-make-test-")); }); afterEach(async () => {