From d0fcbdaf30aac85f993e8c44a93dedff51f0d6d0 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 30 Aug 2026 11:05:55 +0900 Subject: [PATCH] fix(test): scope test lock to user runtime A home-rooted lock can couple separate machines while PID liveness remains host-local, and inaccessible homes fail before discovery with raw filesystem errors.\n\nResolve a validated user runtime from XDG or a private UID temp namespace, include a host discriminator, and surface actionable failures. Cover cross-user, cross-host, Windows, fallback, unsafe-root, and path-containment cases. --- scripts/test-run-lock.ts | 188 ++++++++++++++++++++++++++++++++++++-- scripts/test.ts | 4 +- tests/preload.ts | 2 +- tests/test-runner.test.ts | 134 ++++++++++++++++++++++++++- 4 files changed, 315 insertions(+), 13 deletions(-) diff --git a/scripts/test-run-lock.ts b/scripts/test-run-lock.ts index d1c65487d5..77e5bfa99d 100644 --- a/scripts/test-run-lock.ts +++ b/scripts/test-run-lock.ts @@ -1,5 +1,8 @@ -import { randomUUID } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { + accessSync, + constants, + lstatSync, mkdirSync, readFileSync, readdirSync, @@ -8,15 +11,43 @@ import { statSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { hostname, tmpdir } from "node:os"; +import { isAbsolute, join, posix, win32 } from "node:path"; export const TEST_RUN_ID_ENV = "OCX_TEST_RUN_ID"; export const TEST_RUN_NO_QUEUE_ENV = "OCX_TEST_NO_QUEUE"; -const DEFAULT_LOCK_PATH = join(tmpdir(), "opencodex-bun-test.lock"); const OWNER_FILE = "owner.json"; const MEMBERS_DIR = "members"; const INCOMPLETE_OWNER_GRACE_MS = 10_000; +const POSIX_PRIVATE_MODE = 0o700; + +interface RuntimeDirectoryEntry { + uid: number; + mode: number; + isDirectory(): boolean; + isSymbolicLink(): boolean; +} + +export interface TestRunRuntimeFileSystem { + lstatSync(path: string): RuntimeDirectoryEntry; + mkdirSync(path: string, options: { mode: number }): void; + accessSync(path: string, mode: number): void; +} + +export interface ResolveDefaultTestRunLockPathOptions { + platform?: NodeJS.Platform; + env?: NodeJS.ProcessEnv; + uid?: number; + tempDir?: string; + hostName?: string; + fileSystem?: TestRunRuntimeFileSystem; +} + +const runtimeFileSystem: TestRunRuntimeFileSystem = { + lstatSync, + mkdirSync(path, options) { mkdirSync(path, options); }, + accessSync, +}; export interface TestRunLockOwner { version: 1; @@ -48,6 +79,138 @@ export interface BareTestRunIdentity { runId: string; } +function errorCode(error: unknown): string { + if (!error || typeof error !== "object" || !("code" in error)) return "unknown error"; + return String((error as { code?: unknown }).code ?? "unknown error"); +} + +function inspectRuntimeDirectory(options: { + path: string; + fileSystem: TestRunRuntimeFileSystem; + expectedUid?: number; + requirePrivateMode?: boolean; +}): string | null { + let entry: RuntimeDirectoryEntry; + try { + entry = options.fileSystem.lstatSync(options.path); + } catch (error) { + return `cannot be inspected (${errorCode(error)})`; + } + if (entry.isSymbolicLink() || !entry.isDirectory()) return "is not a real directory"; + if (options.expectedUid !== undefined && entry.uid !== options.expectedUid) { + return "is not owned by the current uid"; + } + if (options.requirePrivateMode && (entry.mode & 0o777) !== POSIX_PRIVATE_MODE) { + return "does not have mode 0700"; + } + try { + options.fileSystem.accessSync(options.path, constants.W_OK | constants.X_OK); + } catch (error) { + return `is not writable/searchable (${errorCode(error)})`; + } + return null; +} + +function machineDiscriminator(hostName: string): string { + const normalized = hostName.trim().toLowerCase(); + if (!normalized) throw new Error("the OS hostname is empty"); + return createHash("sha256").update(normalized).digest("hex").slice(0, 16); +} + +/** + * Resolve a user-scoped, machine-local default lock path without relying on HOME. + * + * POSIX XDG runtime directories are accepted only after an ownership and access + * check. The fallback is a private UID namespace under the OS temp directory. + * The hostname digest remains part of the lock name in either case: even if an + * administrator redirects either root to shared storage, host-local PID liveness + * checks can never reclaim or join another machine's lock. + */ +export function resolveDefaultTestRunLockPath( + options: ResolveDefaultTestRunLockPathOptions = {}, +): string { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const tempDir = options.tempDir ?? tmpdir(); + const fileSystem = options.fileSystem ?? runtimeFileSystem; + let discriminator: string; + try { + discriminator = machineDiscriminator(options.hostName ?? hostname()); + } catch (cause) { + throw new Error( + "Cannot resolve a safe user-scoped Bun test lock: the machine identity is unavailable.", + { cause }, + ); + } + + if (platform === "win32") { + if (!win32.isAbsolute(tempDir)) { + throw new Error( + "Cannot resolve a safe user-scoped Bun test lock: the Windows OS temp/profile path is not absolute.", + ); + } + const issue = inspectRuntimeDirectory({ path: tempDir, fileSystem }); + if (issue) { + throw new Error( + `Cannot resolve a safe user-scoped Bun test lock: the Windows OS temp/profile directory ${issue}.`, + ); + } + return win32.join(tempDir, `opencodex-bun-test-${discriminator}.lock`); + } + + const uid = options.uid ?? (typeof process.getuid === "function" ? process.getuid() : undefined); + if (!Number.isInteger(uid) || (uid ?? -1) < 0) { + throw new Error( + "Cannot resolve a safe user-scoped Bun test lock: the current POSIX uid is unavailable.", + ); + } + + const failures: string[] = []; + const xdgRuntimeDir = env.XDG_RUNTIME_DIR?.trim(); + if (xdgRuntimeDir) { + if (!isAbsolute(xdgRuntimeDir)) { + failures.push("XDG_RUNTIME_DIR is not absolute"); + } else { + const issue = inspectRuntimeDirectory({ + path: xdgRuntimeDir, + fileSystem, + expectedUid: uid, + }); + if (!issue) return posix.join(xdgRuntimeDir, `opencodex-bun-test-${discriminator}.lock`); + failures.push(`XDG_RUNTIME_DIR ${issue}`); + } + } + + if (!isAbsolute(tempDir)) { + failures.push("the OS temporary directory is not absolute"); + } else { + const fallback = posix.join(tempDir, `opencodex-test-runtime-${uid}`); + try { + fileSystem.mkdirSync(fallback, { mode: POSIX_PRIVATE_MODE }); + } catch (error) { + if (errorCode(error) !== "EEXIST") { + failures.push(`the temporary UID runtime directory cannot be created (${errorCode(error)})`); + } + } + if (!failures.some(failure => failure.startsWith("the temporary UID runtime directory cannot be created"))) { + const issue = inspectRuntimeDirectory({ + path: fallback, + fileSystem, + expectedUid: uid, + requirePrivateMode: true, + }); + if (!issue) return posix.join(fallback, `opencodex-bun-test-${discriminator}.lock`); + failures.push(`the temporary UID runtime directory ${issue}`); + } + } + + throw new Error( + "Cannot resolve a safe user-scoped Bun test lock. " + + "Ensure XDG_RUNTIME_DIR is an existing writable directory owned by the current uid, " + + `or make the OS temporary directory usable for a mode-0700 UID runtime (${failures.join("; ")}).`, + ); +} + /** * Give one bare Bun invocation a stable identity without conflating sibling commands. * @@ -151,7 +314,7 @@ function ownsLock(lockPath: string, owner: TestRunLockOwner): boolean { } /** - * Acquire the machine-wide OpenCodex Bun-test lock. + * Acquire the user-scoped, machine-local OpenCodex Bun-test lock. * * `mkdir` is the cross-platform atomic primitive. The owner PID makes a lock left by * SIGKILL recoverable, while the run ID lets every worker belonging to one bare @@ -165,7 +328,8 @@ export async function acquireTestRunLock(options: AcquireTestRunLockOptions): Pr return { acquired: false, owner: null, release() {} }; } - const lockPath = options.lockPath ?? DEFAULT_LOCK_PATH; + const usesDefaultLockPath = options.lockPath === undefined; + const lockPath = options.lockPath ?? resolveDefaultTestRunLockPath({ env }); const ownerPid = options.ownerPid ?? process.pid; const pollMs = Math.max(1, options.pollMs ?? 5_000); const maxWaitMs = Math.max(pollMs, options.maxWaitMs ?? 45 * 60 * 1000); @@ -198,7 +362,17 @@ export async function acquireTestRunLock(options: AcquireTestRunLockOptions): Pr }, }; } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + const code = (error as NodeJS.ErrnoException).code; + if (code !== "EEXIST") { + if (usesDefaultLockPath && ["EACCES", "ENOENT", "EPERM", "EROFS"].includes(code ?? "")) { + throw new Error( + "Cannot acquire the user-scoped Bun test lock because its validated runtime directory " + + "became unavailable or unwritable. Check XDG_RUNTIME_DIR and the OS temporary directory.", + { cause: error }, + ); + } + throw error; + } } const current = readOwner(lockPath); diff --git a/scripts/test.ts b/scripts/test.ts index 832a537191..6d10b2c4a7 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -452,10 +452,10 @@ if (import.meta.main) { const lock = await acquireTestRunLock({ runId, onWait: owner => console.warn( - `[test] another Bun test run${owner ? ` (pid ${owner.pid})` : ""} holds the machine lock; waiting. ` + `[test] another Bun test run${owner ? ` (pid ${owner.pid})` : ""} holds the user lock; waiting. ` + "Set OCX_TEST_NO_QUEUE=1 only for intentional overlap.", ), - onAcquiredAfterWait: elapsedMs => console.warn(`[test] acquired the machine lock after ${Math.round(elapsedMs / 1000)}s.`), + onAcquiredAfterWait: elapsedMs => console.warn(`[test] acquired the user lock after ${Math.round(elapsedMs / 1000)}s.`), }); const startedAt = Date.now(); try { diff --git a/tests/preload.ts b/tests/preload.ts index 37b2233df0..dd04c5c33c 100644 --- a/tests/preload.ts +++ b/tests/preload.ts @@ -34,7 +34,7 @@ await acquireTestRunLock({ runId, ownerPid: bareIdentity.ownerPid, onWait: owner => console.warn( - `[test] bare Bun worker ${process.pid} is waiting for test run${owner ? ` pid ${owner.pid}` : ""} to release the machine lock.`, + `[test] bare Bun worker ${process.pid} is waiting for test run${owner ? ` pid ${owner.pid}` : ""} to release the user lock.`, ), }); diff --git a/tests/test-runner.test.ts b/tests/test-runner.test.ts index 2d5423d628..ed8e92d3ae 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { isAbsolute, join } from "node:path"; +import { dirname, isAbsolute, join, posix, win32 } from "node:path"; import { changedSelectionFailure, createIsolatedTestEnvironment, @@ -14,7 +14,9 @@ import { import { acquireTestRunLock, resolveBareTestRunIdentity, + resolveDefaultTestRunLockPath, TEST_RUN_NO_QUEUE_ENV, + type TestRunRuntimeFileSystem, } from "../scripts/test-run-lock"; import { decodeWindowsIdentityPowerShellOutputForTests, @@ -37,6 +39,28 @@ function runGit(cwd: string, ...args: string[]): string { // handed to git are identical either way. const FIXTURE_COMMIT_EMAIL = ["test", "opencodex.invalid"].join("@"); +function pathIsContainedBy(parent: string, candidate: string, platform: "posix" | "win32"): boolean { + const path = platform === "win32" ? win32 : posix; + const relative = path.relative(path.resolve(parent), path.resolve(candidate)); + return relative === "" || (!relative.startsWith(`..${path.sep}`) + && relative !== ".." && !path.isAbsolute(relative)); +} + +function acceptingRuntimeFileSystem(uid: number, writable = true): TestRunRuntimeFileSystem { + return { + lstatSync: () => ({ + uid, + mode: 0o700, + isDirectory: () => true, + isSymbolicLink: () => false, + }), + mkdirSync: () => {}, + accessSync: () => { + if (!writable) throw Object.assign(new Error("denied"), { code: "EACCES" }); + }, + }; +} + function commitFixture(cwd: string, path: string, contents: string, message: string): string { writeFileSync(join(cwd, path), contents); runGit(cwd, "add", path); @@ -361,7 +385,111 @@ describe("bun test argv", () => { }); }); -describe("bun test machine lock", () => { +describe("bun test user lock", () => { + test("distinct POSIX users receive distinct temp-runtime locks", () => { + const common = { env: {}, tempDir: "/tmp", hostName: "builder-1", platform: "linux" as const }; + const alice = resolveDefaultTestRunLockPath({ + ...common, + uid: 1001, + fileSystem: acceptingRuntimeFileSystem(1001), + }); + const bob = resolveDefaultTestRunLockPath({ + ...common, + uid: 1002, + fileSystem: acceptingRuntimeFileSystem(1002), + }); + + expect(alice).not.toBe(bob); + expect(pathIsContainedBy("/tmp/opencodex-test-runtime-1001", alice, "posix")).toBe(true); + expect(pathIsContainedBy("/tmp/opencodex-test-runtime-1002", bob, "posix")).toBe(true); + }); + + test("a shared home cannot couple locks from distinct hosts", () => { + const common = { + env: { HOME: "/network/users/alice" }, + uid: 1001, + tempDir: "/tmp", + platform: "linux" as const, + fileSystem: acceptingRuntimeFileSystem(1001), + }; + const firstHost = resolveDefaultTestRunLockPath({ ...common, hostName: "builder-1" }); + const secondHost = resolveDefaultTestRunLockPath({ ...common, hostName: "builder-2" }); + + expect(firstHost).not.toBe(secondHost); + expect(pathIsContainedBy(common.env.HOME, firstHost, "posix")).toBe(false); + expect(pathIsContainedBy(common.env.HOME, secondHost, "posix")).toBe(false); + }); + + test("Windows uses the OS temp/profile result when USER is absent", () => { + const common = { + platform: "win32" as const, + tempDir: "C:\\Users\\Alice\\AppData\\Local\\Temp", + hostName: "desktop-1", + fileSystem: acceptingRuntimeFileSystem(0), + }; + const withoutUser = resolveDefaultTestRunLockPath({ ...common, env: {} }); + const withUnrelatedUser = resolveDefaultTestRunLockPath({ + ...common, + env: { USER: "someone-else" }, + }); + + expect(withoutUser).toBe(withUnrelatedUser); + expect(pathIsContainedBy(common.tempDir, withoutUser, "win32")).toBe(true); + }); + + test("falls back from an unsafe XDG root to a validated mode-0700 UID directory", () => { + if (process.platform === "win32" || typeof process.getuid !== "function") return; + const root = mkdtempSync(join(tmpdir(), "opencodex-runtime-fallback-")); + const unsafeXdg = join(root, "not-a-directory"); + writeFileSync(unsafeXdg, "unsafe\n"); + try { + const lockPath = resolveDefaultTestRunLockPath({ + env: { XDG_RUNTIME_DIR: unsafeXdg }, + uid: process.getuid(), + tempDir: root, + hostName: "builder-1", + }); + const runtimeRoot = dirname(lockPath); + const entry = statSync(runtimeRoot); + + expect(runtimeRoot).toBe(join(root, `opencodex-test-runtime-${process.getuid()}`)); + expect(entry.isDirectory()).toBe(true); + expect(entry.uid).toBe(process.getuid()); + expect(entry.mode & 0o777).toBe(0o700); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + test("fails immediately with actionable guidance when every runtime root is unwritable", () => { + expect(() => resolveDefaultTestRunLockPath({ + platform: "linux", + env: { XDG_RUNTIME_DIR: "/run/user/1001" }, + uid: 1001, + tempDir: "/tmp", + hostName: "builder-1", + fileSystem: acceptingRuntimeFileSystem(1001, false), + })).toThrow( + "Cannot resolve a safe user-scoped Bun test lock. Ensure XDG_RUNTIME_DIR", + ); + }); + + test("containment checks do not confuse path string prefixes on POSIX or Windows", () => { + const home = "/home/alice"; + const lockPath = resolveDefaultTestRunLockPath({ + platform: "linux", + env: { HOME: home }, + uid: 1001, + tempDir: "/home", + hostName: "builder-1", + fileSystem: acceptingRuntimeFileSystem(1001), + }); + + expect(home.startsWith("/home")).toBe(true); + expect(pathIsContainedBy(home, lockPath, "posix")).toBe(false); + expect(pathIsContainedBy("C:\\Users\\Ann", "C:\\Users\\Anna\\lock", "win32")).toBe(false); + }); + test("independent bare runners do not inherit a shared long-lived parent identity", () => { expect(resolveBareTestRunIdentity({ pid: 101, ppid: 50 })).toEqual({ ownerPid: 101,