From c49cd66d90476b67c1721d1937ca931beb8d5587 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 07:54:45 +0900 Subject: [PATCH] fix(test): scope the Bun test-run lock to the user, not the whole machine The default lock lived at `tmpdir()/opencodex-bun-test.lock`. On a multi-user host that directory is shared and world-writable, so the serialization primitive was reachable by every local account rather than only by the one running the tests. Any local user could create that directory first and simply keep it. Other users' runs would then wait the full bounded window before failing closed, and a planted owner file that the victim cannot remove keeps the wait permanent. Nothing about serializing Bun test runs requires cross-user scope. Move the default under the current user's home. The wait, reclaim, and run-ID rendezvous behaviour are unchanged; only the default path's scope narrows. Callers passing an explicit `lockPath` are unaffected. --- scripts/test-run-lock.ts | 20 ++++++++++++++++---- scripts/test.ts | 4 ++-- tests/preload.ts | 2 +- tests/test-runner.test.ts | 12 +++++++++++- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/scripts/test-run-lock.ts b/scripts/test-run-lock.ts index d1c65487d5..90e0f09165 100644 --- a/scripts/test-run-lock.ts +++ b/scripts/test-run-lock.ts @@ -8,12 +8,11 @@ import { statSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; +import { homedir } from "node:os"; import { join } 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; @@ -48,6 +47,19 @@ export interface BareTestRunIdentity { runId: string; } +/** + * Resolve the default lock path inside the current OS user's home. + * + * The lock previously lived in `tmpdir()`, which on a multi-user host is shared and + * world-writable. Any local account could pre-create the lock directory and hold it, + * stalling every other user's test run for the full 45-minute wait, or plant an + * unremovable owner file. Serializing test runs is only ever needed per user, so the + * home directory expresses the real scope and removes the cross-user surface. + */ +export function resolveDefaultTestRunLockPath(home = homedir()): string { + return join(home, ".opencodex-bun-test.lock"); +} + /** * Give one bare Bun invocation a stable identity without conflating sibling commands. * @@ -151,7 +163,7 @@ function ownsLock(lockPath: string, owner: TestRunLockOwner): boolean { } /** - * Acquire the machine-wide OpenCodex Bun-test lock. + * Acquire the user-scoped 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 +177,7 @@ export async function acquireTestRunLock(options: AcquireTestRunLockOptions): Pr return { acquired: false, owner: null, release() {} }; } - const lockPath = options.lockPath ?? DEFAULT_LOCK_PATH; + const lockPath = options.lockPath ?? resolveDefaultTestRunLockPath(); 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); 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..94a2586d60 100644 --- a/tests/test-runner.test.ts +++ b/tests/test-runner.test.ts @@ -14,6 +14,7 @@ import { import { acquireTestRunLock, resolveBareTestRunIdentity, + resolveDefaultTestRunLockPath, TEST_RUN_NO_QUEUE_ENV, } from "../scripts/test-run-lock"; import { @@ -361,7 +362,16 @@ describe("bun test argv", () => { }); }); -describe("bun test machine lock", () => { +describe("bun test user lock", () => { + test("the default lock path is user-scoped, not a shared temp directory", () => { + expect(resolveDefaultTestRunLockPath("/home/alice")).toBe(join("/home/alice", ".opencodex-bun-test.lock")); + // Two accounts must not rendezvous on one path: a shared temp lock lets any local + // user hold every other user's test run for the full wait window. + expect(resolveDefaultTestRunLockPath("/home/alice")) + .not.toBe(resolveDefaultTestRunLockPath("/home/mallory")); + expect(resolveDefaultTestRunLockPath("/home/alice").startsWith(tmpdir())).toBe(false); + }); + test("independent bare runners do not inherit a shared long-lived parent identity", () => { expect(resolveBareTestRunIdentity({ pid: 101, ppid: 50 })).toEqual({ ownerPid: 101,