diff --git a/docs/security/tcb-boundary.mdx b/docs/security/tcb-boundary.mdx index bb5b3b288e5..642b5d7023e 100644 --- a/docs/security/tcb-boundary.mdx +++ b/docs/security/tcb-boundary.mdx @@ -44,7 +44,7 @@ A successful build does not replace review of privilege, process identity, descr | `scripts/lib/normalize_mutable_config_perms.py` | The installed copy is root-owned and mode `0555`; startup invokes it under the entrypoint identity, and only root can reclaim a root-owned tree. | The fixed OpenClaw config path, the resolved sandbox identity, and an exact `root:root 0700/0600` mutable-drift signature under the expected sandbox-owned parent. | Restores the mutable `2770/660` contract, pins every privileged handoff by descriptor, and rejects ambiguous posture, links, mount substitution, metadata races, and sealed config. | | `scripts/openclaw-config-guard.py` | The installed copy is root-owned and mode `0500`; direct root PID 1 or the authenticated host transaction invokes it. | Bounded strict JSON for writes, stable captured config bytes for restart validation, and fixed installed parser paths for existing JSON5 config. | Seals and unseals OpenClaw config with no-follow descriptors, stable inode checks, atomic replacement, hash coherence, and recoverable transaction journals. | | `scripts/managed-gateway-control.py` | The installed copy is root-owned and mode `0500`; the host invokes it through sanitized registry-scoped direct-container execution. | A fixed action, a 64-character nonce, fixed installed helpers, and a live OpenShell process tree observed through `/proc`. | Authenticates the host action, proves the managed supervisor and gateway identity, holds a root-owned mode `0600` lifecycle lock, publishes one root-owned mode `0444` exact-exit authorization bound to the gateway and live root controller identities, signals through a pidfd, waits for the normal respawn loop, and verifies listener and HTTP health. | -| `src/lib/shields/transition-lock.ts` | Runs in the host CLI under the operator account and owns the canonical per-sandbox transition lock. | Host state directory entries whose owner PID and start identity match the live lock owner. | Serializes shields mutations, rejects ambiguous or reused owners, and allows takeover only through the explicit recovery contract. | +| `src/lib/shields/transition-lock.ts` | Runs in the host CLI under the operator account and owns the canonical per-sandbox transition lock. | Host state directory entries whose owner PID and start identity match the live lock owner, or prove that the recorded owner is definitively dead or PID-reused. | Serializes shields mutations, recovers definitively stale owners through inode-checked quarantine, rejects ambiguous owners, and allows token-gated takeover only through the explicit recovery contract. | | `src/lib/shields/timer-bound-lock.ts` | Runs in the host CLI and composes the transition lock with the recorded auto-restore generation. | A validated timer marker and transition owner from the host state directory. | Prevents an expired or replaced timer from authorizing a later mutation and keeps restore authority bound to one generation. | | `src/lib/shields/verify-lock.ts` | Runs in the host CLI and delegates sandbox inspection through the privileged execution adapter. | Resolved built-in agent paths and the expected locked posture recorded by the host. | Verifies modes, ownership, immutable flags, layout, and recorded content hashes before NemoClaw reports shields as locked. | | `agents/hermes/runtime-config-guard.py` | The installed copy is root-owned; its privileged actions require direct startup authority, a root-owned readiness lease, or the narrowly proven OpenShell-managed startup shape. | Fixed Hermes paths, bounded actions, stable descriptor snapshots, a transaction token, and authenticated startup or host authority. | Enforces the Hermes secret boundary, config and hash transactions, restart seals, shields transitions, rollback, and stable state-directory posture. | diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index 6104d21a808..18f156c0aeb 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { execFileSync as nodeExecFileSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -70,7 +71,7 @@ vi.mock("child_process", () => ({ })); vi.mock("node:child_process", () => ({ - execFileSync: vi.fn(() => ""), + execFileSync: vi.fn(), spawnSync: vi.fn(() => ({ status: 0, stdout: "", @@ -81,11 +82,33 @@ vi.mock("node:child_process", () => ({ let tmpDir: string; +type NodeExecFileSyncMock = ReturnType; + +function defaultNodeExecFileSync(file: string, argv?: readonly string[]): string { + const args = Array.isArray(argv) ? argv : []; + return file === "ps" && args.includes("lstart=") ? "Mon Jan 01 00:00:00 2026" : ""; +} + +function setNodeExecFileSyncMock( + implementation: (file: string, argv?: readonly string[]) => string = defaultNodeExecFileSync, +): void { + (nodeExecFileSync as unknown as NodeExecFileSyncMock).mockImplementation(implementation); +} + +function withDefaultNodeExecFileSync( + file: string, + argv: readonly string[] | undefined, + fallback: () => string, +): string { + return defaultNodeExecFileSync(file, argv) || fallback(); +} + beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "shields-test-")); vi.stubEnv("HOME", tmpDir); vi.resetModules(); vi.clearAllMocks(); + setNodeExecFileSyncMock(); }); afterEach(() => { @@ -427,28 +450,27 @@ describe("shields — unit logic", () => { }); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const dockerExecFileSync = (await import("node:child_process")).execFileSync as ReturnType< - typeof vi.fn - >; - dockerExecFileSync.mockImplementation((_file: string, argv?: readonly string[]) => { - const cmd = Array.isArray(argv) ? argv.join(" ") : ""; - if (cmd.includes(` stat -c %a %U:%G ${hashPath}`)) { - return "444 root:root"; - } - if (cmd.includes(` stat -c %a %U:%G ${configPath}`)) { - return "444 root:root"; - } - if (cmd.includes(` lsattr -d ${hashPath}`)) { - return `----i---------e----- ${hashPath}`; - } - if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw")) { - return "755 root:root"; - } - if (cmd.includes(` lsattr -d ${configPath}`)) { - return `----i---------e----- ${configPath}`; - } - return ""; - }); + setNodeExecFileSyncMock((_file: string, argv?: readonly string[]) => + withDefaultNodeExecFileSync(_file, argv, () => { + const cmd = Array.isArray(argv) ? argv.join(" ") : ""; + if (cmd.includes(` stat -c %a %U:%G ${hashPath}`)) { + return "444 root:root"; + } + if (cmd.includes(` stat -c %a %U:%G ${configPath}`)) { + return "444 root:root"; + } + if (cmd.includes(` lsattr -d ${hashPath}`)) { + return `----i---------e----- ${hashPath}`; + } + if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw")) { + return "755 root:root"; + } + if (cmd.includes(` lsattr -d ${configPath}`)) { + return `----i---------e----- ${configPath}`; + } + return ""; + }), + ); const { shieldsStatus } = await loadShieldsModule(); @@ -548,28 +570,27 @@ describe("shields — unit logic", () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const dockerExecFileSync = (await import("node:child_process")).execFileSync as ReturnType< - typeof vi.fn - >; - dockerExecFileSync.mockImplementation((_file: string, argv?: readonly string[]) => { - const cmd = Array.isArray(argv) ? argv.join(" ") : ""; - if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw/.config-hash")) { - return "444 root:root"; - } - if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw/openclaw.json")) { - return "444 root:root"; - } - if (cmd.includes(" lsattr -d /sandbox/.openclaw/.config-hash")) { - return "----i---------e----- /sandbox/.openclaw/.config-hash"; - } - if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw")) { - return "755 root:root"; - } - if (cmd.includes(" lsattr -d /sandbox/.openclaw/openclaw.json")) { - return "----i---------e----- /sandbox/.openclaw/openclaw.json"; - } - return ""; - }); + setNodeExecFileSyncMock((_file: string, argv?: readonly string[]) => + withDefaultNodeExecFileSync(_file, argv, () => { + const cmd = Array.isArray(argv) ? argv.join(" ") : ""; + if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw/.config-hash")) { + return "444 root:root"; + } + if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw/openclaw.json")) { + return "444 root:root"; + } + if (cmd.includes(" lsattr -d /sandbox/.openclaw/.config-hash")) { + return "----i---------e----- /sandbox/.openclaw/.config-hash"; + } + if (cmd.includes(" stat -c %a %U:%G /sandbox/.openclaw")) { + return "755 root:root"; + } + if (cmd.includes(" lsattr -d /sandbox/.openclaw/openclaw.json")) { + return "----i---------e----- /sandbox/.openclaw/openclaw.json"; + } + return ""; + }), + ); const { shieldsStatus } = await loadShieldsModule(); shieldsStatus(sandboxName); diff --git a/src/lib/shields/transition-lock.test.ts b/src/lib/shields/transition-lock.test.ts index 5ee01b7803e..5d966815b98 100644 --- a/src/lib/shields/transition-lock.test.ts +++ b/src/lib/shields/transition-lock.test.ts @@ -96,6 +96,16 @@ describe("host shields transition lock", () => { return lockPath; } + function createRecoveryGuard(sandboxName: string): { lockPath: string; guardPath: string } { + const lockPath = shieldsTransitionLockPath(sandboxName, stateDir); + const guardPath = `${lockPath}.recovering`; + return { lockPath, guardPath }; + } + + function writeRecoveryGuardOwner(guardPath: string, value: ShieldsTransitionLockOwner): void { + fs.writeFileSync(guardPath, JSON.stringify(value), { mode: 0o600 }); + } + it("atomically creates a regular owner file and removes it after the callback", () => { const locker = manager(); const lockPath = shieldsTransitionLockPath("alpha", stateDir); @@ -116,7 +126,7 @@ describe("host shields transition lock", () => { const result = locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => { const snapshot = readLockFileSnapshot(lockPath); const written = JSON.parse(snapshot.contents); - expect(snapshot.mode).toBe(0o600n); + expect(process.platform === "win32" || snapshot.mode === 0o600n).toBe(true); expect(written).toEqual({ version: 1, sandboxName: "alpha", @@ -133,6 +143,90 @@ describe("host shields transition lock", () => { expect(fs.existsSync(lockPath)).toBe(false); }); + it("uses a unique self identity when the current process start identity is unavailable", () => { + const locker = manager({ readProcessStartIdentity: () => null }); + const lockPath = shieldsTransitionLockPath("alpha", stateDir); + + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => { + const written = JSON.parse(fs.readFileSync(lockPath, "utf8")); + expect(written).toMatchObject({ + sandboxName: "alpha", + pid: SELF_PID, + command: "nemoclaw alpha shields up", + }); + expect(written.processStartIdentity).toMatch(/^unverified-self:101:[0-9a-f]{32}$/u); + }); + + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("does not reclaim a live owner with an unverified self identity", () => { + const ownerLocker = manager({ readProcessStartIdentity: () => null }); + const lockPath = shieldsTransitionLockPath("alpha", stateDir); + let contenderNow = 2_000; + const contender = manager({ + now: () => contenderNow, + sleep: (milliseconds) => { + contenderNow += milliseconds; + }, + isProcessAlive: (pid) => pid === SELF_PID, + readProcessStartIdentity: (pid) => (pid === SELF_PID ? SELF_IDENTITY : null), + }); + + ownerLocker.withShieldsTransitionLock("alpha", "owner with unverified identity", () => { + expect(() => + contender.withShieldsTransitionLock("alpha", "contender", () => "unexpected", { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }), + ).toThrow(/PID 101 is alive but its process-start identity cannot be verified/); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).command).toBe( + "owner with unverified identity", + ); + }); + + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("does not asynchronously reclaim a live owner with an unverified self identity", async () => { + const ownerLocker = manager({ readProcessStartIdentity: () => null }); + const lockPath = shieldsTransitionLockPath("alpha", stateDir); + let contenderNow = 2_000; + const contender = manager({ + now: () => contenderNow, + sleepAsync: async (milliseconds) => { + contenderNow += milliseconds; + }, + isProcessAlive: (pid) => pid === SELF_PID, + readProcessStartIdentity: (pid) => (pid === SELF_PID ? SELF_IDENTITY : null), + }); + + await ownerLocker.withShieldsTransitionLockAsync( + "alpha", + "async owner with unverified identity", + async () => { + await expect( + contender.withShieldsTransitionLockAsync( + "alpha", + "async contender", + async () => { + throw new Error("should not reclaim live unverified owner"); + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ), + ).rejects.toThrow(/PID 101 is alive but its process-start identity cannot be verified/); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).command).toBe( + "async owner with unverified identity", + ); + }, + ); + + expect(fs.existsSync(lockPath)).toBe(false); + }); + it("never publishes a canonical owner when atomic link publication fails", () => { const locker = manager(); const lockPath = shieldsTransitionLockPath("alpha", stateDir); @@ -293,7 +387,7 @@ describe("host shields transition lock", () => { expect(fs.existsSync(lockPath)).toBe(false); }); - it("preserves a replacement raced into the token-specific quarantine", () => { + it("preserves a replacement raced into token-specific stale recovery", () => { const original = owner("alpha", 202, "proc:owner", "shields down", TAKEOVER_TOKEN); const replacement = owner( "alpha", @@ -304,15 +398,15 @@ describe("host shields transition lock", () => { ); const lockPath = writeOwner("alpha", original); const displacedPath = `${lockPath}.displaced`; - const originalRenameSync = fs.renameSync; + const originalLinkSync = fs.linkSync; let raced = false; - vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => { + vi.spyOn(fs, "linkSync").mockImplementation((source, destination) => { runWhen(String(source) === lockPath && !raced, () => { raced = true; - originalRenameSync(lockPath, displacedPath); + fs.renameSync(lockPath, displacedPath); fs.writeFileSync(lockPath, JSON.stringify(replacement), { mode: 0o600 }); }); - originalRenameSync(source, destination); + originalLinkSync(source, destination); }); const locker = manager({ isProcessAlive: (pid) => pid === SELF_PID || pid === 303, @@ -322,10 +416,8 @@ describe("host shields transition lock", () => { const result = locker.takeoverShieldsTransitionLock("alpha", 202, "proc:owner", TAKEOVER_TOKEN); - expect(result).toMatchObject({ removed: false, reason: "replacement-preserved" }); - expect(result.quarantinePath).toContain(`.takeover-${TAKEOVER_TOKEN}-`); + expect(result).toEqual({ removed: false, reason: "path-changed" }); expect(JSON.parse(fs.readFileSync(lockPath, "utf8"))).toEqual(replacement); - expect(JSON.parse(fs.readFileSync(result.quarantinePath!, "utf8"))).toEqual(replacement); expect(JSON.parse(fs.readFileSync(displacedPath, "utf8"))).toEqual(original); }); @@ -377,54 +469,522 @@ describe("host shields transition lock", () => { expect(fs.existsSync(lockPath)).toBe(true); }); - it("fails closed with recovery guidance when the recorded holder is dead", () => { + it("recovers a stale lock when the parsed holder is dead", () => { + const recorded = owner("alpha", 202, "proc:dead-holder"); + const lockPath = writeOwner("alpha", recorded); + const locker = manager(); + + expect( + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => { + const replacement = JSON.parse(fs.readFileSync(lockPath, "utf8")); + expect(replacement).toMatchObject({ + sandboxName: "alpha", + pid: SELF_PID, + processStartIdentity: SELF_IDENTITY, + command: "nemoclaw alpha shields up", + }); + return "acquired"; + }), + ).toBe("acquired"); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.readdirSync(stateDir)).toEqual([]); + }); + + it("recovers a stale lock when a live PID has been reused", () => { + const holderPid = 202; + const recorded = owner("alpha", holderPid, "proc:original"); + const lockPath = writeOwner("alpha", recorded); + const locker = manager({ + isProcessAlive: (pid) => pid === holderPid || pid === SELF_PID, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === holderPid ? "proc:reused" : null, + }); + + expect( + locker.withShieldsTransitionLock("alpha", "timer restore", () => { + const replacement = JSON.parse(fs.readFileSync(lockPath, "utf8")); + expect(replacement).toMatchObject({ + sandboxName: "alpha", + pid: SELF_PID, + processStartIdentity: SELF_IDENTITY, + command: "timer restore", + }); + return "acquired"; + }), + ).toBe("acquired"); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("enforces the wait timeout when stale recovery retries without progress", () => { const recorded = owner("alpha", 202, "proc:dead-holder"); const lockPath = writeOwner("alpha", recorded); - const liveness = vi.fn((pid: number) => pid === SELF_PID); let nowMs = 2_000; + let raced = false; const locker = manager({ now: () => nowMs, - sleep: (milliseconds) => { - nowMs += milliseconds; + isProcessAlive: (pid) => { + runWhen(pid === 202 && !raced, () => { + raced = true; + fs.unlinkSync(lockPath); + nowMs += 3; + }); + return pid === SELF_PID; }, - isProcessAlive: liveness, }); - const unlink = vi.spyOn(fs, "unlinkSync"); expect(() => locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => undefined, { waitTimeoutMs: 2, pollIntervalMs: 1, }), - ).toThrow(/will not remove a stale lock pathname automatically.*remove '.*' manually/s); + ).toThrow(/Timed out after 2ms waiting for shields transition lock .*recorded owner PID 202/s); + }); - expect(liveness).toHaveBeenCalledWith(202); - expect(unlink).not.toHaveBeenCalledWith(lockPath); - expect(JSON.parse(fs.readFileSync(lockPath, "utf8"))).toEqual(recorded); + it("enforces the async wait timeout when stale recovery retries without progress", async () => { + const recorded = owner("alpha", 202, "proc:dead-holder"); + const lockPath = writeOwner("alpha", recorded); + let nowMs = 2_000; + let raced = false; + const locker = manager({ + now: () => nowMs, + isProcessAlive: (pid) => { + runWhen(pid === 202 && !raced, () => { + raced = true; + fs.unlinkSync(lockPath); + nowMs += 3; + }); + return pid === SELF_PID; + }, + }); + + await expect( + locker.withShieldsTransitionLockAsync( + "alpha", + "nemoclaw alpha shields up", + async () => { + throw new Error("should not acquire after timeout"); + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ), + ).rejects.toThrow( + /Timed out after 2ms waiting for shields transition lock .*recorded owner PID 202/s, + ); + }); + + it("recovers a stale lock asynchronously when the parsed holder is dead", async () => { + const recorded = owner("alpha", 202, "proc:dead-holder"); + const lockPath = writeOwner("alpha", recorded); + const locker = manager(); + + await expect( + locker.withShieldsTransitionLockAsync("alpha", "nemoclaw alpha shields up", async () => { + const replacement = JSON.parse(fs.readFileSync(lockPath, "utf8")); + expect(replacement).toMatchObject({ + sandboxName: "alpha", + pid: SELF_PID, + processStartIdentity: SELF_IDENTITY, + command: "nemoclaw alpha shields up", + }); + return "acquired"; + }), + ).resolves.toBe("acquired"); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.readdirSync(stateDir)).toEqual([]); }); - it("fails closed with recovery guidance when a live PID has been reused", () => { + it("recovers a stale lock asynchronously when a live PID has been reused", async () => { const holderPid = 202; const recorded = owner("alpha", holderPid, "proc:original"); const lockPath = writeOwner("alpha", recorded); + const locker = manager({ + isProcessAlive: (pid) => pid === holderPid || pid === SELF_PID, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === holderPid ? "proc:reused" : null, + }); + + await expect( + locker.withShieldsTransitionLockAsync("alpha", "timer restore", async () => { + const replacement = JSON.parse(fs.readFileSync(lockPath, "utf8")); + expect(replacement).toMatchObject({ + sandboxName: "alpha", + pid: SELF_PID, + processStartIdentity: SELF_IDENTITY, + command: "timer restore", + }); + return "acquired"; + }), + ).resolves.toBe("acquired"); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("keeps a replacement owner canonical during stale lock recovery races", () => { + const original = owner("alpha", 202, "proc:dead-holder"); + const replacement = owner("alpha", 303, "proc:replacement", "replacement holder"); + const lockPath = writeOwner("alpha", original); + const displacedPath = `${lockPath}.displaced`; + const originalLinkSync = fs.linkSync; + let raced = false; + let thirdAcquired = false; + let thirdError: unknown = null; + let recoveryNow = 2_000; + vi.spyOn(fs, "linkSync").mockImplementation((source, destination) => { + runWhen(String(source) === lockPath && !raced, () => { + raced = true; + fs.renameSync(lockPath, displacedPath); + fs.writeFileSync(lockPath, JSON.stringify(replacement), { mode: 0o600 }); + let thirdNow = 2_000; + const third = manager({ + now: () => thirdNow, + sleep: (milliseconds) => { + thirdNow += milliseconds; + }, + isProcessAlive: (pid) => pid === SELF_PID || pid === 303, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === 303 ? "proc:replacement" : null, + }); + try { + third.withShieldsTransitionLock( + "alpha", + "third contender", + () => { + thirdAcquired = true; + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ); + } catch (error) { + thirdError = error; + } + }); + originalLinkSync(source, destination); + }); + const locker = manager({ + now: () => recoveryNow, + sleep: (milliseconds) => { + recoveryNow += milliseconds; + }, + isProcessAlive: (pid) => pid === SELF_PID || pid === 303, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === 303 ? "proc:replacement" : null, + }); + + expect(() => + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => undefined, { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }), + ).toThrow(/PID 303 is still running/); + + expect(thirdAcquired).toBe(false); + expect(thirdError).toBeInstanceOf(Error); + expect(String((thirdError as Error).message)).toMatch(/PID 303 is still running/); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8"))).toEqual(replacement); + expect(JSON.parse(fs.readFileSync(displacedPath, "utf8"))).toEqual(original); + }); + + it("blocks contenders at the final stale recovery unlink boundary", () => { + const recorded = owner("alpha", 202, "proc:dead-holder"); + const lockPath = writeOwner("alpha", recorded); + const originalUnlinkSync = fs.unlinkSync; + let raced = false; + let thirdAcquired = false; + let thirdError: unknown = null; + vi.spyOn(fs, "unlinkSync").mockImplementation((target) => { + runWhen(String(target) === lockPath && !raced, () => { + raced = true; + let thirdNow = 2_000; + const third = manager({ + now: () => { + thirdNow += 1; + return thirdNow; + }, + isProcessAlive: (pid) => pid === SELF_PID, + }); + try { + third.withShieldsTransitionLock( + "alpha", + "third contender", + () => { + thirdAcquired = true; + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ); + } catch (error) { + thirdError = error; + } + }); + originalUnlinkSync(target); + }); + const locker = manager(); + + expect( + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => "acquired"), + ).toBe("acquired"); + + expect(thirdAcquired).toBe(false); + expect(thirdError).toBeInstanceOf(Error); + expect(String((thirdError as Error).message)).toMatch(/recorded owner PID 202/); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("blocks async contenders at the final stale recovery unlink boundary", async () => { + const recorded = owner("alpha", 202, "proc:dead-holder"); + const lockPath = writeOwner("alpha", recorded); + const originalUnlinkSync = fs.unlinkSync; + let raced = false; + let thirdAcquired = false; + let thirdPromise: Promise = Promise.resolve(); + vi.spyOn(fs, "unlinkSync").mockImplementation((target) => { + runWhen(String(target) === lockPath && !raced, () => { + raced = true; + let thirdNow = 2_000; + const third = manager({ + now: () => { + thirdNow += 1; + return thirdNow; + }, + isProcessAlive: (pid) => pid === SELF_PID, + }); + thirdPromise = third.withShieldsTransitionLockAsync( + "alpha", + "async third contender", + async () => { + thirdAcquired = true; + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ); + }); + originalUnlinkSync(target); + }); + const locker = manager(); + + await expect( + locker.withShieldsTransitionLockAsync( + "alpha", + "nemoclaw alpha shields up", + async () => "acquired", + ), + ).resolves.toBe("acquired"); + + await expect(thirdPromise).rejects.toThrow(/recorded owner PID 202/); + expect(thirdAcquired).toBe(false); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("recovers a crashed stale guard while the stale canonical owner remains", () => { + const { lockPath, guardPath } = createRecoveryGuard("alpha"); + fs.writeFileSync(lockPath, JSON.stringify(owner("alpha", 202, "proc:stale")), { + mode: 0o600, + }); + writeRecoveryGuardOwner(guardPath, owner("alpha", 203, "proc:recoverer", "stale recovery")); + const locker = manager(); + + expect( + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => { + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(guardPath)).toBe(false); + return "acquired"; + }), + ).toBe("acquired"); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.existsSync(guardPath)).toBe(false); + }); + + it("recovers an orphaned stale recovery guard before acquiring asynchronously", async () => { + const { lockPath, guardPath } = createRecoveryGuard("alpha"); + fs.writeFileSync(lockPath, JSON.stringify(owner("alpha", 202, "proc:stale")), { + mode: 0o600, + }); + writeRecoveryGuardOwner(guardPath, owner("alpha", 202, "proc:recoverer", "stale recovery")); + const locker = manager(); + + await expect( + locker.withShieldsTransitionLockAsync("alpha", "nemoclaw alpha shields up", async () => { + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(guardPath)).toBe(false); + return "acquired"; + }), + ).resolves.toBe("acquired"); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.existsSync(guardPath)).toBe(false); + }); + + it("does not let an orphaned recovery-owner temp file block acquisition", () => { + const { lockPath, guardPath } = createRecoveryGuard("alpha"); + const orphanedTemp = `${guardPath}.acquire-202-${"a".repeat(32)}.tmp`; + fs.writeFileSync(orphanedTemp, "{incomplete", { mode: 0o600 }); + const locker = manager(); + + expect( + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => { + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(guardPath)).toBe(false); + return "acquired"; + }), + ).toBe("acquired"); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.existsSync(guardPath)).toBe(false); + expect(fs.existsSync(orphanedTemp)).toBe(true); + }); + + it("keeps a live stale recovery guard owner protected", () => { + const { lockPath, guardPath } = createRecoveryGuard("alpha"); + writeRecoveryGuardOwner(guardPath, owner("alpha", 202, "proc:guard", "stale recovery")); let nowMs = 2_000; const locker = manager({ - now: () => nowMs, + now: () => { + nowMs += 1; + return nowMs; + }, sleep: (milliseconds) => { nowMs += milliseconds; }, - isProcessAlive: (pid) => pid === holderPid || pid === SELF_PID, + isProcessAlive: (pid) => pid === SELF_PID || pid === 202, readProcessStartIdentity: (pid) => - pid === SELF_PID ? SELF_IDENTITY : pid === holderPid ? "proc:reused" : null, + pid === SELF_PID ? SELF_IDENTITY : pid === 202 ? "proc:guard" : null, }); expect(() => - locker.withShieldsTransitionLock("alpha", "timer restore", () => undefined, { + locker.withShieldsTransitionLock("alpha", "nemoclaw alpha shields up", () => "unexpected", { waitTimeoutMs: 2, pollIntervalMs: 1, }), - ).toThrow(/now has process-start identity 'proc:reused'.*remove '.*' manually/s); - expect(JSON.parse(fs.readFileSync(lockPath, "utf8"))).toEqual(recorded); + ).toThrow(/the lock changed during inspection/); + + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.existsSync(guardPath)).toBe(true); + }); + + it("does not remove a replacement recovery guard during orphan cleanup", () => { + const { lockPath, guardPath } = createRecoveryGuard("alpha"); + writeRecoveryGuardOwner(guardPath, owner("alpha", 203, "proc:stale", "stale recovery")); + const originalRenameSync = fs.renameSync; + let raced = false; + let callbackRan = false; + let replacementSnapshot: ReturnType | null = null; + let nowMs = 2_000; + vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => { + runWhen(String(source) === guardPath && !raced, () => { + raced = true; + fs.unlinkSync(guardPath); + writeRecoveryGuardOwner(guardPath, owner("alpha", 202, "proc:guard", "stale recovery")); + replacementSnapshot = readLockFileSnapshot(guardPath); + }); + originalRenameSync(source, destination); + }); + const locker = manager({ + now: () => { + nowMs += 1; + return nowMs; + }, + isProcessAlive: (pid) => pid === SELF_PID || pid === 202, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === 202 ? "proc:guard" : null, + }); + + expect(() => + locker.withShieldsTransitionLock( + "alpha", + "nemoclaw alpha shields up", + () => { + callbackRan = true; + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ), + ).toThrow(/the lock changed during inspection/); + + expect(raced).toBe(true); + expect(callbackRan).toBe(false); + expect(fs.existsSync(lockPath)).toBe(false); + expect(fs.existsSync(guardPath)).toBe(true); + expect(readLockFileSnapshot(guardPath)).toEqual(replacementSnapshot); + expect(JSON.parse(fs.readFileSync(guardPath, "utf8"))).toMatchObject({ + pid: 202, + processStartIdentity: "proc:guard", + }); + const preserved = fs + .readdirSync(stateDir) + .filter((entry) => entry.startsWith(`${path.basename(guardPath)}.stale-`)); + expect(preserved).toHaveLength(1); + expect(readLockFileSnapshot(path.join(stateDir, preserved[0]!, "owner.json"))).toEqual( + replacementSnapshot, + ); + }); + + it("preserves a replacement installed while releasing a held recovery guard", () => { + const recorded = owner("alpha", 203, "proc:stale"); + const lockPath = writeOwner("alpha", recorded); + const guardPath = `${lockPath}.recovering`; + const originalRenameSync = fs.renameSync; + let raced = false; + let callbackRan = false; + let replacementSnapshot: ReturnType | null = null; + let nowMs = 2_000; + vi.spyOn(fs, "renameSync").mockImplementation((source, destination) => { + runWhen( + String(source) === guardPath && + String(destination).includes(".recovery-release-") && + !raced, + () => { + raced = true; + fs.unlinkSync(guardPath); + writeRecoveryGuardOwner(guardPath, owner("alpha", 202, "proc:guard", "stale recovery")); + replacementSnapshot = readLockFileSnapshot(guardPath); + }, + ); + originalRenameSync(source, destination); + }); + const locker = manager({ + now: () => { + nowMs += 1; + return nowMs; + }, + isProcessAlive: (pid) => pid === SELF_PID || pid === 202, + readProcessStartIdentity: (pid) => + pid === SELF_PID ? SELF_IDENTITY : pid === 202 ? "proc:guard" : null, + }); + + expect(() => + locker.withShieldsTransitionLock( + "alpha", + "nemoclaw alpha shields up", + () => { + callbackRan = true; + }, + { + waitTimeoutMs: 2, + pollIntervalMs: 1, + }, + ), + ).toThrow(/Timed out after 2ms/); + + expect(raced).toBe(true); + expect(callbackRan).toBe(false); + expect(fs.existsSync(lockPath)).toBe(false); + expect(readLockFileSnapshot(guardPath)).toEqual(replacementSnapshot); + expect(JSON.parse(fs.readFileSync(guardPath, "utf8"))).toMatchObject({ + pid: 202, + processStartIdentity: "proc:guard", + }); }); it("waits on a recent malformed owner record", () => { @@ -471,18 +1031,23 @@ describe("host shields transition lock", () => { expect(fs.readFileSync(lockPath, "utf8")).toBe("{incomplete"); }); - it("rejects symbolic-link and non-regular lock paths", () => { + it.skipIf(process.platform === "win32")("rejects symbolic-link lock paths", () => { const target = path.join(root, "target"); fs.writeFileSync(target, "{}", { mode: 0o600 }); const symlinkPath = shieldsTransitionLockPath("symlinked", stateDir); fs.symlinkSync(target, symlinkPath); - const directoryPath = shieldsTransitionLockPath("directory", stateDir); - fs.mkdirSync(directoryPath); const locker = manager(); expect(() => locker.withShieldsTransitionLock("symlinked", "shields up", () => undefined), ).toThrow(/symbolic links are not allowed/); + }); + + it("rejects non-regular lock paths", () => { + const directoryPath = shieldsTransitionLockPath("directory", stateDir); + fs.mkdirSync(directoryPath); + const locker = manager(); + expect(() => locker.withShieldsTransitionLock("directory", "shields up", () => undefined), ).toThrow(/path is not a regular file/); diff --git a/src/lib/shields/transition-lock.ts b/src/lib/shields/transition-lock.ts index c769eb6837c..b31eb7a635c 100644 --- a/src/lib/shields/transition-lock.ts +++ b/src/lib/shields/transition-lock.ts @@ -16,6 +16,7 @@ const DEFAULT_WAIT_TIMEOUT_MS = 30_000; const DEFAULT_POLL_INTERVAL_MS = 50; const DEFAULT_MALFORMED_STALE_MS = 30_000; const TAKEOVER_TOKEN_PATTERN = /^[0-9a-f]{32}$/; +const UNVERIFIED_SELF_IDENTITY_PREFIX = "unverified-self:"; const waitBuffer = new Int32Array(new SharedArrayBuffer(4)); @@ -123,6 +124,13 @@ export interface ShieldsTransitionTakeoverResult { quarantinePath?: string; } +interface StaleOwnerRemovalExpectation { + expectedOwnerPid: number; + expectedOwnerStartIdentity: string; + quarantineLabel: string; + matches: (owner: ShieldsTransitionLockOwner) => boolean; +} + function isErrnoException(error: unknown): error is NodeJS.ErrnoException { return error instanceof Error && "code" in error; } @@ -198,6 +206,25 @@ function parseOwner(raw: string, sandboxName: string): ShieldsTransitionLockOwne return owner as unknown as ShieldsTransitionLockOwner; } +function sameOwnerRecord( + left: ShieldsTransitionLockOwner, + right: ShieldsTransitionLockOwner, +): boolean { + return ( + left.version === right.version && + left.sandboxName === right.sandboxName && + left.pid === right.pid && + left.processStartIdentity === right.processStartIdentity && + left.command === right.command && + left.acquiredAtMs === right.acquiredAtMs && + left.takeoverToken === right.takeoverToken + ); +} + +function isUnverifiedSelfIdentity(identity: string): boolean { + return identity.startsWith(UNVERIFIED_SELF_IDENTITY_PREFIX); +} + function unsafeLockPathError(lockPath: string, reason: string): Error { return new Error(`Unsafe shields transition lock '${lockPath}': ${reason}`); } @@ -279,24 +306,32 @@ function defaultSleepAsync(milliseconds: number): Promise { return new Promise((resolve) => setTimeout(resolve, milliseconds)); } -function staleRecovery(lockPath: string): string { +function manualRecovery(lockPath: string): string { + return `Verify that no shields transition is active, remove '${lockPath}' manually, and retry.`; +} + +function malformedStaleRecovery(lockPath: string): string { return `NemoClaw will not remove a stale lock pathname automatically because another process could replace it after inspection. Verify that no shields transition is active, remove '${lockPath}' manually, and retry.`; } +function staleOwnerRecovery(lockPath: string): string { + return `NemoClaw could not safely recover the stale lock automatically. ${manualRecovery(lockPath)}`; +} + function formatWaitReason(reason: WaitReason | null, lockPath: string): string { if (!reason) return "the lock changed during inspection; retry the command"; if (reason.kind === "recent-malformed") { return `the owner record is incomplete and only ${Math.max(0, Math.floor(reason.ageMs))}ms old. Retry after the writer finishes`; } if (reason.kind === "stale-malformed") { - return `the owner record is incomplete and ${Math.max(0, Math.floor(reason.ageMs))}ms old. ${staleRecovery(lockPath)}`; + return `the owner record is incomplete and ${Math.max(0, Math.floor(reason.ageMs))}ms old. ${malformedStaleRecovery(lockPath)}`; } const owner = reason.owner; if (reason.kind === "dead") { - return `recorded owner PID ${String(owner.pid)} is not running (${owner.command}). ${staleRecovery(lockPath)}`; + return `recorded owner PID ${String(owner.pid)} is not running (${owner.command}). ${staleOwnerRecovery(lockPath)}`; } if (reason.kind === "pid-reused") { - return `recorded owner PID ${String(owner.pid)} now has process-start identity '${reason.currentProcessStartIdentity}' instead of '${owner.processStartIdentity}' (${owner.command}). ${staleRecovery(lockPath)}`; + return `recorded owner PID ${String(owner.pid)} now has process-start identity '${reason.currentProcessStartIdentity}' instead of '${owner.processStartIdentity}' (${owner.command}). ${staleOwnerRecovery(lockPath)}`; } if (reason.kind === "identity-unavailable") { return `PID ${String(owner.pid)} is alive but its process-start identity cannot be verified (${owner.command}). Verify the active process and retry`; @@ -323,6 +358,7 @@ export class ShieldsTransitionLockManager { private readonly sleepAsync: (milliseconds: number) => Promise; private readonly processIsAlive: (pid: number) => boolean; private readonly processStartIdentity: (pid: number) => string | null; + private readonly ownerStartIdentityFallback: string; private readonly held = new Map(); private readonly ownership = new AsyncLocalStorage>(); @@ -334,6 +370,7 @@ export class ShieldsTransitionLockManager { this.sleepAsync = deps.sleepAsync ?? defaultSleepAsync; this.processIsAlive = deps.isProcessAlive ?? isProcessAlive; this.processStartIdentity = deps.readProcessStartIdentity ?? readProcessStartIdentity; + this.ownerStartIdentityFallback = `${UNVERIFIED_SELF_IDENTITY_PREFIX}${String(this.pid)}:${randomBytes(16).toString("hex")}`; } withShieldsTransitionLock( @@ -465,17 +502,32 @@ export class ShieldsTransitionLockManager { throw new Error("expectedOwnerStartIdentity is required"); } const validToken = requireTakeoverToken(takeoverToken); - const lockPath = shieldsTransitionLockPath(validName, this.stateDir); - const snapshot = readExistingLock(lockPath, validName); + return this.removeStaleTransitionLockOwner(validName, { + expectedOwnerPid, + expectedOwnerStartIdentity, + quarantineLabel: `takeover-${validToken}`, + matches: (owner) => + owner.pid === expectedOwnerPid && + owner.processStartIdentity === expectedOwnerStartIdentity && + owner.takeoverToken === validToken, + }); + } + + private removeStaleTransitionLockOwner( + sandboxName: string, + expectation: StaleOwnerRemovalExpectation, + ): ShieldsTransitionTakeoverResult { + const lockPath = shieldsTransitionLockPath(sandboxName, this.stateDir); + const snapshot = readExistingLock(lockPath, sandboxName); if (!snapshot) return { removed: false, reason: "missing" }; try { const owner = snapshot.owner; if ( !owner || - owner.pid !== expectedOwnerPid || - owner.processStartIdentity !== expectedOwnerStartIdentity || - owner.takeoverToken !== validToken + owner.pid !== expectation.expectedOwnerPid || + owner.processStartIdentity !== expectation.expectedOwnerStartIdentity || + !expectation.matches(owner) ) { return { removed: false, reason: "owner-mismatch" }; } @@ -491,59 +543,196 @@ export class ShieldsTransitionLockManager { if (!currentIdentity) { return { removed: false, reason: "owner-identity-unavailable" }; } + if (isUnverifiedSelfIdentity(owner.processStartIdentity)) { + return { removed: false, reason: "owner-identity-unavailable" }; + } if (currentIdentity === owner.processStartIdentity) { return { removed: false, reason: "owner-live" }; } removalReason = "removed-reused-pid"; } - const current = this.currentRegularLockIdentity(lockPath); - if (!current || !sameInode(current, snapshot.identity)) { - return { removed: false, reason: "path-changed" }; + const guard = this.enterStaleRecoveryGuard(lockPath, sandboxName); + if (!guard) return { removed: false, reason: "path-changed" }; + try { + this.assertHeldPath(guard); + const current = this.currentRegularLockIdentity(lockPath); + if (!current || !sameInode(current, snapshot.identity)) { + return { removed: false, reason: "path-changed" }; + } + + const quarantineDir = fs.mkdtempSync(`${lockPath}.${expectation.quarantineLabel}-`); + fs.chmodSync(quarantineDir, 0o700); + const quarantinePath = path.join(quarantineDir, "owner.json"); + try { + fs.linkSync(lockPath, quarantinePath); + } catch (error) { + this.removeEmptyQuarantine(quarantineDir); + if (isErrnoException(error) && error.code === "ENOENT") { + return { removed: false, reason: "path-changed" }; + } + throw error; + } + + const moved = readExistingLock(quarantinePath, sandboxName); + if (!moved) { + this.removeEmptyQuarantine(quarantineDir); + return { removed: false, reason: "path-changed" }; + } + try { + const movedOwner = moved.owner; + const movedMatches = + sameInode(moved.identity, snapshot.identity) && + movedOwner !== null && + expectation.matches(movedOwner); + if (!movedMatches) { + this.removeLinkedQuarantine(quarantinePath); + this.removeEmptyQuarantine(quarantineDir); + return { removed: false, reason: "path-changed" }; + } + + const current = this.currentRegularLockIdentity(lockPath); + if (!current || !sameInode(current, snapshot.identity)) { + this.removeLinkedQuarantine(quarantinePath); + this.removeEmptyQuarantine(quarantineDir); + return { removed: false, reason: "path-changed" }; + } + this.assertHeldPath(guard); + try { + fs.unlinkSync(lockPath); + } catch (error) { + this.removeLinkedQuarantine(quarantinePath); + this.removeEmptyQuarantine(quarantineDir); + if (isErrnoException(error) && error.code === "ENOENT") { + return { removed: false, reason: "path-changed" }; + } + throw error; + } + fs.unlinkSync(quarantinePath); + } finally { + closeSnapshot(moved); + } + this.removeEmptyQuarantine(quarantineDir); + return { removed: true, reason: removalReason }; + } finally { + this.removeHeldLockPath(guard, sandboxName, "recovery-release"); } + } finally { + closeSnapshot(snapshot); + } + } + + private staleRecoveryGuardPath(lockPath: string): string { + return `${lockPath}.recovering`; + } - const quarantineDir = fs.mkdtempSync(`${lockPath}.takeover-${validToken}-`); + private staleRecoveryGuardOwner(sandboxName: string): ShieldsTransitionLockOwner { + return { + version: LOCK_VERSION, + sandboxName, + pid: this.pid, + processStartIdentity: this.processStartIdentity(this.pid) ?? this.ownerStartIdentityFallback, + command: "shields stale recovery", + acquiredAtMs: this.now(), + }; + } + + private staleRecoveryInProgress(lockPath: string, sandboxName: string): boolean { + const guardPath = this.staleRecoveryGuardPath(lockPath); + const snapshot = readExistingLock(guardPath, sandboxName); + if (!snapshot) return false; + let guardIsStale = false; + try { + const owner = snapshot.owner; + if (!owner) return true; + if (!this.processIsAlive(owner.pid)) { + guardIsStale = true; + } else if (!isUnverifiedSelfIdentity(owner.processStartIdentity)) { + const currentIdentity = this.processStartIdentity(owner.pid); + guardIsStale = currentIdentity !== null && currentIdentity !== owner.processStartIdentity; + } + if (!guardIsStale) return true; + if (!this.removeObservedStaleRecoveryGuard(guardPath, snapshot, sandboxName)) return true; + } finally { + closeSnapshot(snapshot); + } + return this.currentRegularLockIdentity(guardPath) !== null; + } + + private enterStaleRecoveryGuard(lockPath: string, sandboxName: string): HeldLock | null { + const guardPath = this.staleRecoveryGuardPath(lockPath); + const owner = this.staleRecoveryGuardOwner(sandboxName); + const created = this.tryCreate(guardPath, owner); + if (created) return created; + if (this.staleRecoveryInProgress(lockPath, sandboxName)) return null; + return this.tryCreate(guardPath, owner); + } + + private removeObservedStaleRecoveryGuard( + guardPath: string, + snapshot: ExistingLockSnapshot, + sandboxName: string, + ): boolean { + const cleanupGuard = this.enterStaleRecoveryGuard(guardPath, sandboxName); + if (!cleanupGuard) return false; + try { + this.assertHeldPath(cleanupGuard); + const current = readExistingLock(guardPath, sandboxName); + if (!current) return true; + try { + if ( + !sameInode(current.identity, snapshot.identity) || + current.owner === null || + snapshot.owner === null || + !sameOwnerRecord(current.owner, snapshot.owner) + ) { + return false; + } + } finally { + closeSnapshot(current); + } + + this.assertHeldPath(cleanupGuard); + const quarantineDir = fs.mkdtempSync(`${guardPath}.stale-`); fs.chmodSync(quarantineDir, 0o700); const quarantinePath = path.join(quarantineDir, "owner.json"); try { - fs.renameSync(lockPath, quarantinePath); + fs.renameSync(guardPath, quarantinePath); } catch (error) { this.removeEmptyQuarantine(quarantineDir); - if (isErrnoException(error) && error.code === "ENOENT") { - return { removed: false, reason: "path-changed" }; - } + if (isErrnoException(error) && error.code === "ENOENT") return true; throw error; } - const moved = readExistingLock(quarantinePath, validName); - if (!moved) { - return { removed: false, reason: "replacement-preserved", quarantinePath }; - } + let moved: ExistingLockSnapshot | null = null; try { - const movedOwner = moved.owner; - const movedMatches = - sameInode(moved.identity, snapshot.identity) && - movedOwner?.pid === expectedOwnerPid && - movedOwner.processStartIdentity === expectedOwnerStartIdentity && - movedOwner.takeoverToken === validToken; - if (!movedMatches) { - this.restoreQuarantinedReplacement(lockPath, quarantinePath); - return { removed: false, reason: "replacement-preserved", quarantinePath }; - } - - const currentQuarantine = this.currentRegularLockIdentity(quarantinePath); - if (!currentQuarantine || !sameInode(currentQuarantine, moved.identity)) { - this.restoreQuarantinedReplacement(lockPath, quarantinePath); - return { removed: false, reason: "replacement-preserved", quarantinePath }; + moved = readExistingLock(quarantinePath, sandboxName); + if ( + !moved || + !sameInode(moved.identity, snapshot.identity) || + moved.owner === null || + snapshot.owner === null || + !sameOwnerRecord(moved.owner, snapshot.owner) + ) { + this.restoreQuarantinedReplacement(guardPath, quarantinePath); + return false; } fs.unlinkSync(quarantinePath); + this.removeEmptyQuarantine(quarantineDir); + return true; } finally { - closeSnapshot(moved); + if (moved) closeSnapshot(moved); } - this.removeEmptyQuarantine(quarantineDir); - return { removed: true, reason: removalReason }; } finally { - closeSnapshot(snapshot); + this.removeHeldLockPath(cleanupGuard, sandboxName, "recovery-release"); + } + } + + private removeLinkedQuarantine(quarantinePath: string): void { + try { + fs.unlinkSync(quarantinePath); + } catch (error) { + if (!isErrnoException(error) || error.code !== "ENOENT") throw error; } } @@ -609,8 +798,11 @@ export class ShieldsTransitionLockManager { ): HeldLock { const state = this.acquisitionState(sandboxName, command, options); let lastWaitReason: WaitReason | null = null; + let retrying = false; while (true) { + this.enforceWaitTimeoutBeforeRetry(state, lastWaitReason, retrying); + retrying = true; const inProcess = this.held.get(sandboxName); if (inProcess) { lastWaitReason = { kind: "same-process", owner: inProcess.owner }; @@ -624,6 +816,7 @@ export class ShieldsTransitionLockManager { ); if (!observed) continue; lastWaitReason = observed; + if (this.recoveredObservedStaleOwner(sandboxName, observed)) continue; } this.sleep(this.waitDuration(state, lastWaitReason)); } @@ -636,8 +829,11 @@ export class ShieldsTransitionLockManager { ): Promise { const state = this.acquisitionState(sandboxName, command, options); let lastWaitReason: WaitReason | null = null; + let retrying = false; while (true) { + this.enforceWaitTimeoutBeforeRetry(state, lastWaitReason, retrying); + retrying = true; const inProcess = this.held.get(sandboxName); if (inProcess) { lastWaitReason = { kind: "same-process", owner: inProcess.owner }; @@ -651,6 +847,7 @@ export class ShieldsTransitionLockManager { ); if (!observed) continue; lastWaitReason = observed; + if (this.recoveredObservedStaleOwner(sandboxName, observed)) continue; } await this.sleepAsync(this.waitDuration(state, lastWaitReason)); } @@ -673,12 +870,12 @@ export class ShieldsTransitionLockManager { options.malformedStaleMs ?? DEFAULT_MALFORMED_STALE_MS, "malformedStaleMs", ); - const ownerStartIdentity = this.processStartIdentity(this.pid); - if (!ownerStartIdentity) { - throw new Error( - `Cannot acquire shields transition lock: process-start identity for PID ${String(this.pid)} is unavailable`, - ); - } + // Windows developer and CI hosts can lack both /proc and a ps lstart + // identity. Keep this fallback only until timer-control has a stable + // Windows start-time reader; live observers fail closed instead of + // reclaiming this owner. + const ownerStartIdentity = + this.processStartIdentity(this.pid) ?? this.ownerStartIdentityFallback; fs.mkdirSync(this.stateDir, { recursive: true, mode: 0o700 }); const lockPath = shieldsTransitionLockPath(sandboxName, this.stateDir); @@ -702,6 +899,28 @@ export class ShieldsTransitionLockManager { }; } + private recoveredObservedStaleOwner(sandboxName: string, reason: WaitReason): boolean { + if (reason.kind !== "dead" && reason.kind !== "pid-reused") return false; + const recovery = this.removeStaleTransitionLockOwner(sandboxName, { + expectedOwnerPid: reason.owner.pid, + expectedOwnerStartIdentity: reason.owner.processStartIdentity, + quarantineLabel: "takeover-stale", + matches: (owner) => sameOwnerRecord(owner, reason.owner), + }); + if (recovery.removed) return true; + if (recovery.reason === "replacement-preserved") { + const lockPath = shieldsTransitionLockPath(sandboxName, this.stateDir); + throw new Error( + `Cannot recover stale shields transition lock '${lockPath}': a replacement was preserved during recovery. ${manualRecovery(lockPath)}`, + ); + } + return ( + recovery.reason === "missing" || + recovery.reason === "owner-mismatch" || + recovery.reason === "path-changed" + ); + } + private observeWaitReason( lockPath: string, sandboxName: string, @@ -720,6 +939,9 @@ export class ShieldsTransitionLockManager { if (!this.processIsAlive(owner.pid)) return { kind: "dead", owner }; const currentIdentity = this.processStartIdentity(owner.pid); if (!currentIdentity) return { kind: "identity-unavailable", owner }; + if (isUnverifiedSelfIdentity(owner.processStartIdentity)) { + return { kind: "identity-unavailable", owner }; + } if (currentIdentity !== owner.processStartIdentity) { return { kind: "pid-reused", owner, currentProcessStartIdentity: currentIdentity }; } @@ -729,13 +951,26 @@ export class ShieldsTransitionLockManager { } } - private waitDuration(state: AcquisitionState, reason: WaitReason | null): number { + private enforceWaitTimeout(state: AcquisitionState, reason: WaitReason | null): void { const elapsedMs = Math.max(0, this.now() - state.startedAtMs); if (elapsedMs >= state.waitTimeoutMs) { throw new Error( `Timed out after ${String(state.waitTimeoutMs)}ms waiting for shields transition lock '${state.lockPath}': ${formatWaitReason(reason, state.lockPath)}`, ); } + } + + private enforceWaitTimeoutBeforeRetry( + state: AcquisitionState, + reason: WaitReason | null, + retrying: boolean, + ): void { + if (retrying) this.enforceWaitTimeout(state, reason); + } + + private waitDuration(state: AcquisitionState, reason: WaitReason | null): number { + this.enforceWaitTimeout(state, reason); + const elapsedMs = Math.max(0, this.now() - state.startedAtMs); return Math.min(state.pollIntervalMs, state.waitTimeoutMs - elapsedMs); } @@ -748,6 +983,7 @@ export class ShieldsTransitionLockManager { } catch (error) { if (!isErrnoException(error) || error.code !== "ENOENT") throw error; } + if (this.staleRecoveryInProgress(lockPath, owner.sandboxName)) return null; const tempPath = `${lockPath}.acquire-${String(this.pid)}-${randomBytes(16).toString("hex")}.tmp`; let fd: number; try { @@ -837,15 +1073,10 @@ export class ShieldsTransitionLockManager { } } - private release(sandboxName: string, held: HeldLock): void { - if (this.held.get(sandboxName) !== held) return; - held.depth -= 1; - if (held.depth > 0) return; - this.held.delete(sandboxName); - + private removeHeldLockPath(held: HeldLock, sandboxName: string, quarantineLabel: string): void { try { const heldIdentity = inodeIdentity(fs.fstatSync(held.fd, { bigint: true })); - const quarantineDir = fs.mkdtempSync(`${held.lockPath}.release-`); + const quarantineDir = fs.mkdtempSync(`${held.lockPath}.${quarantineLabel}-`); fs.chmodSync(quarantineDir, 0o700); const quarantinePath = path.join(quarantineDir, "owner.json"); try { @@ -872,6 +1103,14 @@ export class ShieldsTransitionLockManager { fs.closeSync(held.fd); } } + + private release(sandboxName: string, held: HeldLock): void { + if (this.held.get(sandboxName) !== held) return; + held.depth -= 1; + if (held.depth > 0) return; + this.held.delete(sandboxName); + this.removeHeldLockPath(held, sandboxName, "release"); + } } const defaultManager = new ShieldsTransitionLockManager();