diff --git a/nemoclaw/src/blueprint/state.test.ts b/nemoclaw/src/blueprint/state.test.ts index af5b77b32b7..c0f549453f5 100644 --- a/nemoclaw/src/blueprint/state.test.ts +++ b/nemoclaw/src/blueprint/state.test.ts @@ -1,12 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, it, expect, beforeEach, vi } from "vitest"; import type fs from "node:fs"; import { homedir } from "node:os"; -import { loadState, saveState, clearState, type NemoClawState } from "./state.js"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { clearState, loadState, type NemoClawState, saveState } from "./state.js"; const store = new Map(); +const writes: Array<{ path: string; options: unknown }> = []; +const renames: Array<{ from: string; to: string }> = []; vi.mock("node:fs", async (importOriginal) => { const original = await importOriginal(); @@ -19,9 +21,17 @@ vi.mock("node:fs", async (importOriginal) => { if (content === undefined) throw new Error(`ENOENT: ${p}`); return content; }, - writeFileSync: (p: string, data: string) => { + writeFileSync: (p: string, data: string, options?: unknown) => { + writes.push({ path: p, options }); store.set(p, data); }, + renameSync: (from: string, to: string) => { + renames.push({ from, to }); + const content = store.get(from); + if (content === undefined) throw new Error(`ENOENT: ${from}`); + store.set(to, content); + store.delete(from); + }, }; }); @@ -30,6 +40,8 @@ const STATE_PATH = `${homedir()}/.nemoclaw/state/nemoclaw.json`; describe("blueprint/state", () => { beforeEach(() => { store.clear(); + writes.length = 0; + renames.length = 0; }); describe("loadState", () => { @@ -170,10 +182,19 @@ describe("blueprint/state", () => { expect(loaded.lastRunId).toBeNull(); }); - it("does nothing when no file exists", () => { + it("creates blank state when no file exists", () => { expect(() => { clearState(); }).not.toThrow(); + expect(store.has(STATE_PATH)).toBe(true); + const write = writes.at(-1); + const rename = renames.at(-1); + expect(write?.path.startsWith(`${STATE_PATH}.${process.pid}.`)).toBe(true); + expect(write?.path.endsWith(".tmp")).toBe(true); + expect(write?.options).toMatchObject({ mode: 0o600 }); + expect(rename).toEqual({ from: write?.path, to: STATE_PATH }); + expect(store.has(write?.path || "")).toBe(false); + expect(JSON.parse(store.get(STATE_PATH) || "{}").lastAction).toBeNull(); }); }); }); diff --git a/nemoclaw/src/blueprint/state.ts b/nemoclaw/src/blueprint/state.ts index b7e0f251094..01e2618a5b2 100644 --- a/nemoclaw/src/blueprint/state.ts +++ b/nemoclaw/src/blueprint/state.ts @@ -1,7 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { randomUUID } from "node:crypto"; +import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; @@ -144,17 +145,21 @@ export function loadState(): NemoClawState { } } +function writeStateFile(state: NemoClawState): void { + const finalPath = statePath(); + const tmpPath = `${finalPath}.${process.pid}.${randomUUID()}.tmp`; + writeFileSync(tmpPath, JSON.stringify(state, null, 2), { mode: 0o600 }); + renameSync(tmpPath, finalPath); +} + export function saveState(state: NemoClawState): void { ensureStateDir(); state.updatedAt = new Date().toISOString(); state.createdAt ??= state.updatedAt; - writeFileSync(statePath(), JSON.stringify(state, null, 2)); + writeStateFile(state); } export function clearState(): void { ensureStateDir(); - const path = statePath(); - if (existsSync(path)) { - writeFileSync(path, JSON.stringify(blankState(), null, 2)); - } + writeStateFile(blankState()); } diff --git a/src/lib/actions/sandbox/gateway-state.ts b/src/lib/actions/sandbox/gateway-state.ts index ebd7676c96c..71df5a30ec3 100644 --- a/src/lib/actions/sandbox/gateway-state.ts +++ b/src/lib/actions/sandbox/gateway-state.ts @@ -457,14 +457,12 @@ export async function ensureLiveSandboxOrExit( if (lookup.state === "identity_drift") { console.error(" Gateway SSH identity changed after restart — clearing stale host keys..."); const knownHostsPath = path.join(os.homedir(), ".ssh", "known_hosts"); - if (fs.existsSync(knownHostsPath)) { - try { - const kh = fs.readFileSync(knownHostsPath, "utf8"); - const cleaned = pruneKnownHostsEntries(kh); - if (cleaned !== kh) fs.writeFileSync(knownHostsPath, cleaned); - } catch { - /* best-effort cleanup */ - } + try { + const kh = fs.readFileSync(knownHostsPath, "utf8"); + const cleaned = pruneKnownHostsEntries(kh); + if (cleaned !== kh) fs.writeFileSync(knownHostsPath, cleaned); + } catch { + /* best-effort cleanup */ } const retry = await getReconciledSandboxGatewayState(sandboxName); if (retry.state === "present") { diff --git a/src/lib/actions/uninstall/plan.ts b/src/lib/actions/uninstall/plan.ts index bd45d2c3dba..893d481ff23 100644 --- a/src/lib/actions/uninstall/plan.ts +++ b/src/lib/actions/uninstall/plan.ts @@ -2,13 +2,21 @@ // SPDX-License-Identifier: Apache-2.0 import fs from "node:fs"; +import os from "node:os"; import { defaultUninstallPaths } from "../../domain/uninstall/paths"; -import { buildUninstallPlan, type UninstallPlan, type UninstallPlanOptions } from "../../domain/uninstall/plan"; +import { + buildUninstallPlan, + type UninstallPlan, + type UninstallPlanOptions, +} from "../../domain/uninstall/plan"; import { classifyNemoclawShim, type ShimClassification } from "../../domain/uninstall/shims"; export interface FileSystemDeps { + closeSync?: typeof fs.closeSync; + fstatSync?: typeof fs.fstatSync; lstatSync?: typeof fs.lstatSync; + openSync?: typeof fs.openSync; readFileSync?: typeof fs.readFileSync; } @@ -17,29 +25,80 @@ export interface HostUninstallPlanOptions extends Omit { env: { HOME: "/home/test", TMPDIR: "/tmp/test" } as NodeJS.ProcessEnv, fs: { lstatSync: (() => ({ isFile: () => false, isSymbolicLink: () => true })) as never, + openSync: (() => { + const error = new Error("symlink") as NodeJS.ErrnoException; + error.code = "ELOOP"; + throw error; + }) as never, }, }, ); diff --git a/src/lib/actions/uninstall/run-plan.ts b/src/lib/actions/uninstall/run-plan.ts index d0ce76169b5..d054211a288 100644 --- a/src/lib/actions/uninstall/run-plan.ts +++ b/src/lib/actions/uninstall/run-plan.ts @@ -1,13 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawnSync, type SpawnSyncOptions, type SpawnSyncReturns } from "node:child_process"; +import { type SpawnSyncOptions, type SpawnSyncReturns, spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { dockerSpawnSync } from "../../adapters/docker/exec"; -import { getAgentBranding, type AgentBranding } from "../../cli/branding"; +import { type AgentBranding, getAgentBranding } from "../../cli/branding"; import { sleepMs } from "../../core/wait"; import { defaultUninstallPaths, NEMOCLAW_OLLAMA_MODELS, NEMOCLAW_PROVIDERS, type UninstallPaths } from "../../domain/uninstall/paths"; import { buildUninstallPlan, type UninstallPlan } from "../../domain/uninstall/plan"; @@ -65,7 +65,23 @@ function defaultRunDocker(args: string[], options: SpawnSyncOptions = {}): RunRe } function defaultCommandExists(command: string, env: NodeJS.ProcessEnv): boolean { - return defaultRun("sh", ["-c", `command -v ${JSON.stringify(command)} >/dev/null 2>&1`], { env }).status === 0; + if (!command || command.includes("\0")) return false; + const hasPathSeparator = command.includes(path.sep) || command.includes(path.posix.sep) || command.includes(path.win32.sep); + const candidates = hasPathSeparator + ? [command] + : String(env.PATH || "") + .split(path.delimiter) + .filter(Boolean) + .map((entry) => path.join(entry, command)); + for (const candidate of candidates) { + try { + fs.accessSync(candidate, fs.constants.X_OK); + return true; + } catch { + // Try the next PATH entry. + } + } + return false; } function defaultReadLine(env: NodeJS.ProcessEnv): string | null { @@ -738,7 +754,7 @@ function executePlan( export function buildRunPlan(options: UninstallRunOptions, deps: UninstallRunDeps = {}): { paths: UninstallPaths; plan: UninstallPlan } { const env = { ...process.env, ...(deps.env ?? {}) }; - const home = env.HOME || os.tmpdir(); + const home = env.HOME || os.homedir(); const paths = defaultUninstallPaths({ home, repoRoot: path.resolve(__dirname, "..", "..", ".."), diff --git a/src/lib/adapters/http/probe.test.ts b/src/lib/adapters/http/probe.test.ts index 5224b8cc6fe..5cc81540f1a 100644 --- a/src/lib/adapters/http/probe.test.ts +++ b/src/lib/adapters/http/probe.test.ts @@ -210,6 +210,157 @@ describe("http-probe helpers", () => { expect(result.curlStatus).toBe(-110); expect(result.message).toContain("ETIMEDOUT"); }); + + it("rejects non-http probe URLs before spawning curl", () => { + let spawned = false; + const result = runCurlProbe(["-sS", "file:///etc/passwd"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + expect(result.message).toContain("curl probe URL must use http or https"); + }); + + it("rejects curl probe request bodies that read from local files", () => { + let spawned = false; + const result = runCurlProbe(["-sS", "--data-binary", "@/etc/passwd", "https://example.test/models"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + expect(result.message).toContain("must not read request data from a file"); + }); + + it.each([ + ["--upload-file", ["--upload-file", "/etc/passwd"]], + ["-T", ["-T/etc/passwd"]], + ["--netrc", ["--netrc"]], + ["--data-urlencode", ["--data-urlencode", "payload@/etc/passwd"]], + ["-K", ["-K/etc/passwd"]], + ["-b", ["-b/etc/passwd"]], + ["--cert", ["--cert", "/tmp/client.pem"]], + ["--key", ["--key=/tmp/client.key"]], + ["--proxy-cert", ["--proxy-cert", "/tmp/proxy.pem"]], + ["--proxy-key", ["--proxy-key=/tmp/proxy.key"]], + ])("rejects file-reading curl option %s before spawning", (_label, args) => { + let spawned = false; + const result = runCurlProbe(["-sS", ...args, "https://example.test/models"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + }); + + it.each([ + ["-H", ["-H", "@/etc/passwd"]], + ["-H inline", ["-H@/etc/passwd"]], + ["--header", ["--header=@/etc/passwd"]], + ])("rejects file-backed curl header option %s before spawning", (_label, args) => { + let spawned = false; + const result = runCurlProbe(["-sS", ...args, "https://example.test/models"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + expect(result.message).toContain("must not read headers from a file"); + }); + + it("applies file-read validation to streaming probe wrappers", () => { + let spawned = false; + const spawnSyncImpl = () => { + spawned = true; + throw new Error("should not spawn"); + }; + + const chat = runChatCompletionsStreamingProbe( + ["-sS", "--header", "@/etc/passwd", "https://example.test/v1/chat/completions"], + { spawnSyncImpl }, + ); + const responses = runStreamingEventProbe( + ["-sS", "--cert", "/tmp/client.pem", "https://example.test/v1/responses"], + { spawnSyncImpl }, + ); + + expect(spawned).toBe(false); + expect(chat.ok).toBe(false); + expect(responses.ok).toBe(false); + }); + + it.each([ + ["extra URL", ["https://evil.example/steal"]], + ["multi-transfer --next", ["--next"]], + ])("rejects %s before spawning", (_label, args) => { + let spawned = false; + const result = runCurlProbe(["-sS", ...args, "https://example.test/models"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + }); + + it.each([ + ["--data", ["--data"]], + ["-H", ["-H"]], + ["--max-time", ["--max-time"]], + ["--config", ["--config"]], + ])("rejects missing value for curl option %s before spawning", (_label, args) => { + let spawned = false; + const result = runCurlProbe(["-sS", ...args, "https://example.test/models"], { + spawnSyncImpl: () => { + spawned = true; + throw new Error("should not spawn"); + }, + }); + + expect(spawned).toBe(false); + expect(result.ok).toBe(false); + }); + + it("allows explicit trusted curl config files", () => { + const configPath = path.join(os.tmpdir(), "nemoclaw-trusted-curl.conf"); + let spawnedArgs: readonly string[] = []; + const result = runCurlProbe(["-sS", "--config", configPath, "https://example.test/models"], { + trustedConfigFiles: [configPath], + spawnSyncImpl: (_command, args) => { + spawnedArgs = args; + const outputPath = args[args.indexOf("-o") + 1]; + if (typeof outputPath === "string") { + fs.writeFileSync(outputPath, "{}"); + } + return { + pid: 1, + output: [], + stdout: "200", + stderr: "", + status: 0, + signal: null, + }; + }, + }); + + expect(result.ok).toBe(true); + expect(spawnedArgs).toContain(configPath); + }); }); describe("runChatCompletionsStreamingProbe", () => { diff --git a/src/lib/adapters/http/probe.ts b/src/lib/adapters/http/probe.ts index be5b7c2929d..1c2d57dd0cc 100644 --- a/src/lib/adapters/http/probe.ts +++ b/src/lib/adapters/http/probe.ts @@ -22,6 +22,8 @@ export interface CurlProbeOptions { env?: NodeJS.ProcessEnv; replaceEnv?: boolean; timeoutMs?: number; + /** Absolute or cwd-relative curl config files created by trusted NemoClaw callers. */ + trustedConfigFiles?: readonly string[]; spawnSyncImpl?: ( command: string, args: readonly string[], @@ -126,6 +128,180 @@ function sanitizeCurlUrl(value: string): string { } } +const CURL_CONFIG_OPTIONS = new Set(["--config", "-K"]); +const CURL_OPTIONS_THAT_READ_FILES = new Set([ + "--cookie", + "-b", + "--netrc-file", + "--upload-file", + "-T", + "--cert", + "--key", + "--proxy-cert", + "--proxy-key", +]); +const CURL_OPTIONS_THAT_READ_IMPLICIT_FILES = new Set(["--netrc", "--netrc-optional"]); +const CURL_DATA_OPTIONS = new Set([ + "--data", + "--data-raw", + "--data-binary", + "--data-ascii", + "--data-urlencode", + "--json", + "--form", + "-d", + "-F", +]); +const CURL_HEADER_OPTIONS = new Set(["--header", "--proxy-header", "-H"]); +const CURL_SAFE_FLAG_OPTIONS = new Set(["-s", "-S", "-sS", "-sf", "--compressed", "--get"]); +const CURL_SAFE_VALUE_OPTIONS = new Set(["--connect-timeout", "--max-time", "-X", "--request"]); +const CURL_FORBIDDEN_MULTI_TRANSFER_OPTIONS = new Set(["--next"]); +const CURL_SHORT_OPTIONS_WITH_VALUES = new Set(["-K", "-b", "-T", "-d", "-F", "-H", "-X"]); + +function normalizeHttpProbeUrl(rawUrl: unknown): string { + if (typeof rawUrl !== "string" || rawUrl.trim() === "") { + throw new Error("curl probe URL is required"); + } + const url = new URL(rawUrl); + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new Error(`curl probe URL must use http or https: ${url.protocol}`); + } + if (url.username || url.password) { + throw new Error("curl probe URL must not embed credentials"); + } + return url.toString(); +} + +function splitCurlOptionArg(arg: string): { option: string; inlineValue?: string } { + if (arg.startsWith("--")) { + const [option, inlineValue] = arg.includes("=") + ? arg.split(/=(.*)/s, 2) + : [arg, undefined]; + return { option, inlineValue }; + } + for (const option of CURL_SHORT_OPTIONS_WITH_VALUES) { + if (arg.startsWith(option) && arg.length > option.length) { + return { option, inlineValue: arg.slice(option.length) }; + } + } + return { option: arg }; +} + +function curlValueReadsFromFile(option: string, value: string): boolean { + if ((value.startsWith("@") && value !== "@-") || /(^|=)@[^-]/.test(value)) return true; + if (option === "--data-urlencode" && /^[^=]+@[^-]/.test(value)) return true; + if ((option === "--form" || option === "-F") && /(^|=)<[^-]/.test(value)) return true; + return false; +} + +function curlHeaderValueReadsFromFile(value: string): boolean { + return value.startsWith("@") && value !== "@-"; +} + +function getCurlOptionValue( + args: string[], + index: number, + option: string, + inlineValue: string | undefined, +): string { + if (inlineValue !== undefined) return inlineValue; + const value = args[index + 1]; + if (value === undefined) throw new Error(`curl probe option requires a value: ${option}`); + return value; +} + +function normalizeCurlConfigPath(value: string, opts: CurlProbeOptions): string { + if (value.trim() === "") throw new Error("curl probe config path is required"); + if (value.includes("\0")) throw new Error("curl probe config path must not contain NUL bytes"); + return path.resolve(opts.cwd ?? ROOT, value); +} + +function isTrustedCurlConfigPath(value: string, opts: CurlProbeOptions): boolean { + if (!opts.trustedConfigFiles?.length) return false; + const candidate = normalizeCurlConfigPath(value, opts); + return opts.trustedConfigFiles + .map((trustedPath) => normalizeCurlConfigPath(trustedPath, opts)) + .includes(candidate); +} + +function validateCurlProbeArgs( + argv: string[], + opts: CurlProbeOptions = {}, +): { args: string[]; url: string } { + const args = [...argv]; + const url = normalizeHttpProbeUrl(args.pop()); + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + const { option, inlineValue } = splitCurlOptionArg(arg); + if (CURL_FORBIDDEN_MULTI_TRANSFER_OPTIONS.has(option)) { + throw new Error(`curl probe option is not allowed because it creates multiple transfers: ${option}`); + } + if (CURL_OPTIONS_THAT_READ_IMPLICIT_FILES.has(option)) { + throw new Error(`curl probe option is not allowed because it reads local files: ${option}`); + } + if (CURL_OPTIONS_THAT_READ_FILES.has(option)) { + getCurlOptionValue(args, index, option, inlineValue); + if (inlineValue === undefined) index += 1; + throw new Error(`curl probe option is not allowed because it reads local files: ${option}`); + } + if (CURL_CONFIG_OPTIONS.has(option)) { + const value = getCurlOptionValue(args, index, option, inlineValue); + if (!isTrustedCurlConfigPath(value, opts)) { + throw new Error(`curl probe config file is not trusted: ${option}`); + } + if (inlineValue === undefined) index += 1; + continue; + } + if (CURL_HEADER_OPTIONS.has(option)) { + const value = getCurlOptionValue(args, index, option, inlineValue); + if (curlHeaderValueReadsFromFile(value)) { + throw new Error(`curl probe option must not read headers from a file: ${option}`); + } + if (inlineValue === undefined) index += 1; + continue; + } + if (arg === "--url" || arg.startsWith("--url=")) { + throw new Error("curl probe URLs must be passed as the final argv entry"); + } + if (CURL_DATA_OPTIONS.has(option)) { + const value = getCurlOptionValue(args, index, option, inlineValue); + if (curlValueReadsFromFile(option, value)) { + throw new Error(`curl probe option must not read request data from a file: ${option}`); + } + if (inlineValue === undefined) index += 1; + continue; + } + if (CURL_SAFE_VALUE_OPTIONS.has(option)) { + getCurlOptionValue(args, index, option, inlineValue); + if (inlineValue === undefined) index += 1; + continue; + } + if (CURL_SAFE_FLAG_OPTIONS.has(option)) { + continue; + } + if (!arg.startsWith("-")) { + throw new Error("curl probe received unexpected positional argument before URL"); + } + throw new Error(`curl probe option is not allowed: ${option}`); + } + return { args, url }; +} + +type CurlProbeMode = "json" | "chat-stream" | "event-stream"; + +function buildCurlProbeSpawnArgs( + args: string[], + url: string, + bodyFile: string, + mode: CurlProbeMode, +): string[] { + const outputArgs = + mode === "json" ? ["-o", bodyFile, "-w", "%{http_code}"] : ["-N", "-o", bodyFile]; + const statusArgs = mode === "chat-stream" ? ["-w", "%{http_code}"] : []; + // lgtm[js/file-access-to-http] URL/argv are validated; file-backed config paths must be explicitly trusted. + return [...args, ...outputArgs, ...statusArgs, url]; +} + function getCurlProbeTraceAttributes(argv: string[], opts: CurlProbeOptions): Record { const url = argv.at(-1) || ""; const methodIndex = argv.findIndex((arg) => arg === "-X" || arg === "--request"); @@ -212,13 +388,14 @@ export function runCurlProbe(argv: string[], opts: CurlProbeOptions = {}): CurlP function runCurlProbeImpl(argv: string[], opts: CurlProbeOptions = {}): CurlProbeResult { const bodyFile = secureTempFile("nemoclaw-curl-probe", ".json"); try { - const args = [...argv]; - const url = args.pop(); + const { args, url } = validateCurlProbeArgs(argv, opts); const spawnSyncImpl = opts.spawnSyncImpl ?? spawnSync; const timeout = resolveCurlProcessTimeoutMs(argv, opts); + const curlArgs = buildCurlProbeSpawnArgs(args, url, bodyFile, "json"); const result = spawnSyncImpl( "curl", - [...args, "-o", bodyFile, "-w", "%{http_code}", String(url || "")], + // lgtm[js/file-access-to-http] curlArgs were validated and rebuilt from safe probe fields. + curlArgs, { cwd: opts.cwd ?? ROOT, encoding: "utf8", @@ -320,13 +497,14 @@ function runChatCompletionsStreamingProbeImpl( ): CurlProbeResult { const bodyFile = secureTempFile("nemoclaw-chat-streaming-probe", ".sse"); try { - const args = [...argv]; - const url = args.pop(); + const { args, url } = validateCurlProbeArgs(argv, opts); const spawnSyncImpl = opts.spawnSyncImpl ?? spawnSync; const timeout = resolveCurlProcessTimeoutMs(argv, opts); + const curlArgs = buildCurlProbeSpawnArgs(args, url, bodyFile, "chat-stream"); const result = spawnSyncImpl( "curl", - [...args, "-N", "-o", bodyFile, "-w", "%{http_code}", String(url || "")], + // lgtm[js/file-access-to-http] curlArgs were validated and rebuilt from safe probe fields. + curlArgs, { cwd: opts.cwd ?? ROOT, encoding: "utf8", @@ -440,19 +618,24 @@ function runStreamingEventProbeImpl( ): StreamingProbeResult { const bodyFile = secureTempFile("nemoclaw-streaming-probe", ".sse"); try { - const args = [...argv]; - const url = args.pop(); + const { args, url } = validateCurlProbeArgs(argv, opts); const spawnSyncImpl = opts.spawnSyncImpl ?? spawnSync; const timeout = resolveCurlProcessTimeoutMs(argv, opts); - const result = spawnSyncImpl("curl", [...args, "-N", "-o", bodyFile, String(url || "")], { - cwd: opts.cwd ?? ROOT, - encoding: "utf8", - timeout, - env: { - ...process.env, - ...opts.env, + const curlArgs = buildCurlProbeSpawnArgs(args, url, bodyFile, "event-stream"); + const result = spawnSyncImpl( + "curl", + // lgtm[js/file-access-to-http] curlArgs were validated and rebuilt from safe probe fields. + curlArgs, + { + cwd: opts.cwd ?? ROOT, + encoding: "utf8", + timeout, + env: { + ...process.env, + ...opts.env, + }, }, - }); + ); const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : ""; diff --git a/src/lib/credentials/store.ts b/src/lib/credentials/store.ts index f8aa0c1c209..a432c7399c9 100644 --- a/src/lib/credentials/store.ts +++ b/src/lib/credentials/store.ts @@ -56,6 +56,23 @@ export const KNOWN_CREDENTIAL_ENV_KEYS: readonly string[] = [ // huge file. 1 MiB leaves plenty of headroom over any plausible mutation. const LEGACY_CREDS_FILE_MAX_BYTES = 1 * 1024 * 1024; +function noFollowFlag(): number | undefined { + return typeof fs.constants.O_NOFOLLOW === "number" ? fs.constants.O_NOFOLLOW : undefined; +} + +function openReadOnlyNoFollow(filePath: string): number { + const flag = noFollowFlag(); + if (flag === undefined) { + const stat = fs.lstatSync(filePath); + if (stat.isSymbolicLink()) { + const error = new Error(`Refusing to follow symlink: ${filePath}`) as NodeJS.ErrnoException; + error.code = "ELOOP"; + throw error; + } + } + return fs.openSync(filePath, fs.constants.O_RDONLY | (flag ?? 0)); +} + /** * Resolve the user's home directory and reject obviously unsafe choices * (e.g. `/tmp`, `/`) so we never use a world-readable path for state. @@ -235,18 +252,20 @@ export function listCredentialKeys(): string[] { * backup tools and same-user processes tend to read. */ function secureUnlink(filePath: string): void { + let opened = false; try { - const stat = fs.lstatSync(filePath); - if (stat.isSymbolicLink()) { - // The credentials path was a symlink; remove the link itself without - // touching whatever it pointed at. - fs.unlinkSync(filePath); + const flag = noFollowFlag(); + if (flag === undefined) { + const stat = fs.lstatSync(filePath); + if (stat.isFile() || stat.isSymbolicLink()) fs.unlinkSync(filePath); return; } - if (!stat.isFile()) return; - if (stat.size > 0) { - const fd = fs.openSync(filePath, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); - try { + const fd = fs.openSync(filePath, fs.constants.O_RDWR | flag); + opened = true; + try { + const stat = fs.fstatSync(fd); + if (!stat.isFile()) return; + if (stat.size > 0) { const chunkSize = Math.min(stat.size, 64 * 1024); const zeros = Buffer.alloc(chunkSize); let written = 0; @@ -256,15 +275,18 @@ function secureUnlink(filePath: string): void { written += len; } fs.fsyncSync(fd); - } finally { - fs.closeSync(fd); } + } finally { + fs.closeSync(fd); } } catch { - // best effort + // best effort; a final-component symlink either fails O_NOFOLLOW or is + // handled by the no-O_NOFOLLOW lstat fallback without touching its target. } try { - fs.unlinkSync(filePath); + if (opened || fs.lstatSync(filePath).isSymbolicLink()) { + fs.unlinkSync(filePath); + } } catch { // best effort } @@ -310,7 +332,7 @@ export function stageLegacyCredentialsToEnv(): string[] { // symlink planted at the credentials path. let fd: number; try { - fd = fs.openSync(legacyFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + fd = openReadOnlyNoFollow(legacyFile); } catch { return []; } @@ -421,7 +443,7 @@ export function removeLegacyCredentialsFileIfEmpty(): boolean { let fd: number; try { - fd = fs.openSync(legacyFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + fd = openReadOnlyNoFollow(legacyFile); } catch { return false; } diff --git a/src/lib/inference/health.ts b/src/lib/inference/health.ts index dba6cfdc3ac..1644ae7cea6 100644 --- a/src/lib/inference/health.ts +++ b/src/lib/inference/health.ts @@ -11,7 +11,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import type { CurlProbeResult } from "../adapters/http/probe"; +import type { CurlProbeOptions, CurlProbeResult } from "../adapters/http/probe"; import { runCurlProbe } from "../adapters/http/probe"; import { normalizeCredentialValue, resolveProviderCredential } from "../credentials/store"; import { getProviderSelectionConfig } from "./config"; @@ -42,7 +42,7 @@ export interface ProviderHealthStatus { } export interface ProviderHealthProbeOptions { - runCurlProbeImpl?: (argv: string[]) => CurlProbeResult; + runCurlProbeImpl?: (argv: string[], opts?: CurlProbeOptions) => CurlProbeResult; model?: string | null; getCredentialImpl?: (envName: string) => string | null | undefined; isWsl?: boolean; @@ -245,6 +245,7 @@ function probeNvidiaKimiK26Health( try { return runCurlProbeImpl( buildKimiStatusProbeCurlArgs(model, endpoint, authConfigPath, options.isWsl), + { trustedConfigFiles: [authConfigPath] }, ); } finally { cleanupAuthCurlConfig(authConfigPath); diff --git a/src/lib/inference/ollama/proxy.ts b/src/lib/inference/ollama/proxy.ts index 637470ef7f9..e693be542c0 100644 --- a/src/lib/inference/ollama/proxy.ts +++ b/src/lib/inference/ollama/proxy.ts @@ -463,6 +463,26 @@ function pullTimeoutErrorHint(timeoutMs: number): string { ].join("\n"); } +function normalizeOllamaPullModel(model): string { + const value = String(model || "").trim(); + if (!value || /[\0\r\n]/.test(value)) { + throw new Error("Invalid Ollama model id for pull request"); + } + return value; +} + +function buildLocalOllamaPullUrl(): string { + const host = getResolvedOllamaHost(); + const allowedHosts = new Set(["127.0.0.1", "localhost", "::1", OLLAMA_HOST_DOCKER_INTERNAL]); + if (!allowedHosts.has(host)) { + throw new Error(`Refusing to pull from unexpected Ollama host: ${host}`); + } + const url = new URL("http://127.0.0.1/api/pull"); + url.hostname = host; + url.port = String(OLLAMA_PORT); + return url.toString(); +} + function pullOllamaModelViaCli(model) { const timeoutMs = getOllamaPullTimeoutMs(); const result = spawnSync("bash", ["-c", `ollama pull ${shellQuote(model)}`], { @@ -485,13 +505,15 @@ function pullOllamaModelViaCli(model) { // keeps the CLI path so existing behavior is unchanged. function pullOllamaModelViaHttp(model) { return new Promise((resolve) => { - const host = getResolvedOllamaHost(); - const url = `http://${host}:${OLLAMA_PORT}/api/pull`; - const body = JSON.stringify({ model, stream: true }); + const url = buildLocalOllamaPullUrl(); + const body = JSON.stringify({ model: normalizeOllamaPullModel(model), stream: true }); const TIMEOUT_MS = getOllamaPullTimeoutMs(); const isTTY = Boolean(process.stdout.isTTY); const BAR_WIDTH = 40; + // The endpoint is restricted to the local Ollama hosts NemoClaw probes and + // the model id is normalized before being serialized as JSON request data. + // lgtm[js/file-access-to-http] const proc = spawn( "curl", [ diff --git a/src/lib/messaging/channels/slack/hooks/credential-validation.test.ts b/src/lib/messaging/channels/slack/hooks/credential-validation.test.ts index f30aa96636f..30cc50b314d 100644 --- a/src/lib/messaging/channels/slack/hooks/credential-validation.test.ts +++ b/src/lib/messaging/channels/slack/hooks/credential-validation.test.ts @@ -51,8 +51,9 @@ describe("Slack token validation", () => { it("validates bot tokens with auth.test", () => { let configPath = ""; let configText = ""; - vi.mocked(runCurlProbe).mockImplementation((args) => { + vi.mocked(runCurlProbe).mockImplementation((args, opts) => { configPath = curlConfigPath(args); + expect(opts?.trustedConfigFiles).toContain(configPath); configText = fs.readFileSync(configPath, "utf8"); expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); return probe('{"ok":true,"user_id":"U123"}'); diff --git a/src/lib/messaging/channels/slack/hooks/credential-validation.ts b/src/lib/messaging/channels/slack/hooks/credential-validation.ts index 65d75ad6c79..79c570394bb 100644 --- a/src/lib/messaging/channels/slack/hooks/credential-validation.ts +++ b/src/lib/messaging/channels/slack/hooks/credential-validation.ts @@ -100,7 +100,7 @@ function slackApiArgs(configPath: string, url: string): string[] { function runSlackApiProbe(token: string, url: string): CurlProbeResult { const { configPath, cleanup } = writeSlackCurlConfig(token); try { - return runCurlProbe(slackApiArgs(configPath, url)); + return runCurlProbe(slackApiArgs(configPath, url), { trustedConfigFiles: [configPath] }); } finally { cleanup(); } diff --git a/src/lib/runner-argv.test.ts b/src/lib/runner-argv.test.ts index bb05b789f7b..3ca80b05eee 100644 --- a/src/lib/runner-argv.test.ts +++ b/src/lib/runner-argv.test.ts @@ -1,8 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, it, expect } from "vitest"; import { createRequire } from "module"; +import { describe, expect, it } from "vitest"; const require = createRequire(import.meta.url); const runner = require("../../dist/lib/runner"); @@ -35,6 +35,13 @@ describe("run with argv array", () => { expect(() => runner.run(["echo", "hi"], { shell: true })).toThrow(/shell option is forbidden/); }); + it("rejects NUL bytes before spawning", () => { + expect(() => runner.run(["echo", "bad\0arg"], { suppressOutput: true })).toThrow( + /NUL bytes/, + ); + expect(() => runner.run(["\0echo"], { suppressOutput: true })).toThrow(/NUL bytes/); + }); + it("does not interpret shell metacharacters in arguments", () => { // If shell interpretation occurred, $(whoami) would be expanded const result = runner.runCapture(["echo", "$(whoami)", "&&", "rm", "-rf", "/"], { diff --git a/src/lib/runner.ts b/src/lib/runner.ts index 1503eed20a5..adfb1b3dcf8 100644 --- a/src/lib/runner.ts +++ b/src/lib/runner.ts @@ -43,6 +43,33 @@ function buildRunnerEnv(extraEnv?: NodeJS.ProcessEnv): Record { return buildSubprocessEnv(normalizedExtra); } +function rejectNulByte(value: string, label: string): string { + if (value.includes("\0")) { + throw new Error(`${label} must not contain NUL bytes`); + } + return value; +} + +function normalizeSpawnFile(file: string, callerName: string): string { + const normalized = rejectNulByte(String(file), `${callerName}: executable`); + if (normalized.length === 0) { + throw new Error(`${callerName}: executable must not be empty`); + } + return normalized; +} + +function normalizeSpawnArgs(args: readonly unknown[], callerName: string): string[] { + return args.map((arg, index) => rejectNulByte(String(arg), `${callerName}: argv[${index + 1}]`)); +} + +function normalizeArgv(cmd: readonly string[], callerName: string): [string, string[]] { + if (cmd.length === 0) { + throw new Error(`${callerName}: argv array must not be empty`); + } + const [file, ...args] = cmd; + return [normalizeSpawnFile(file, callerName), normalizeSpawnArgs(args, callerName)]; +} + function logOpenshellRuntimeHint(file: string, renderedCommand = ""): void { if ( file === "openshell" || @@ -65,8 +92,17 @@ function spawnAndHandle( stdio: RunnerOptions["stdio"], renderedCommand: string, ): SpawnResult { - const result = spawnSync(file, args, { + const safeFile = normalizeSpawnFile(file, "spawnAndHandle"); + const safeArgs = normalizeSpawnArgs(args, "spawnAndHandle"); + // All non-shell runner paths pass argv arrays and force shell=false; runShell + // and runInteractiveShell enter here with a literal `bash -c` executable and + // an explicitly named shell boundary. Extra environment values are filtered by + // buildRunnerEnv before spawn. + // lgtm[js/indirect-command-line-injection] + // lgtm[js/shell-command-injection-from-environment] + const result = spawnSync(safeFile, safeArgs, { ...opts, + shell: false, stdio, cwd: ROOT, env: buildRunnerEnv(opts.env), @@ -84,7 +120,7 @@ function spawnAndHandle( console.error( ` Command failed (exit ${result.status}): ${redact(renderedCommand).slice(0, 80)}`, ); - logOpenshellRuntimeHint(file, renderedCommand); + logOpenshellRuntimeHint(safeFile, renderedCommand); process.exit(result.status || 1); } return result; @@ -124,12 +160,7 @@ function runArrayCmd( defaultStdio: RunnerOptions["stdio"] = ["ignore", "pipe", "pipe"], callerName = "run", ): SpawnResult { - if (cmd.length === 0) { - throw new Error(`${callerName}: argv array must not be empty`); - } - - const exe = cmd[0]; - const args = cmd.slice(1); + const [exe, args] = normalizeArgv(cmd, callerName); const { ignoreError, suppressOutput, env: extraEnv, stdio: stdioCfg, ...spawnOpts } = opts; // Guard: re-enabling shell interpretation defeats the purpose of argv arrays. @@ -139,10 +170,13 @@ function runArrayCmd( const stdio = stdioCfg ?? defaultStdio; - // run() always uses argv arrays and rejects `shell: true` above. - // codeql[js/indirect-command-line-injection] + // run() always uses argv arrays, rejects `shell: true` above, and validates + // the executable/argv for process-spawn metacharacters such as NUL bytes. + // lgtm[js/indirect-command-line-injection] + // lgtm[js/shell-command-injection-from-environment] const result = spawnSync(exe, args, { ...spawnOpts, + shell: false, stdio, cwd: ROOT, env: buildRunnerEnv(extraEnv), @@ -216,12 +250,7 @@ function runCapture(cmd: readonly string[], opts: CaptureOptions = {}): string { if (!Array.isArray(cmd)) { throw new Error("runCapture no longer accepts shell strings; pass an argv array instead"); } - if (cmd.length === 0) { - throw new Error("runCapture: argv array must not be empty"); - } - - const exe = cmd[0]; - const args = cmd.slice(1); + const [exe, args] = normalizeArgv(cmd, "runCapture"); const { ignoreError, env: extraEnv, stdio: _stdio, ...spawnOpts } = opts; // Guard: re-enabling shell interpretation defeats the purpose of argv arrays. @@ -230,10 +259,14 @@ function runCapture(cmd: readonly string[], opts: CaptureOptions = {}): string { } try { - // runCapture() always uses argv arrays and rejects `shell: true` above. - // codeql[js/indirect-command-line-injection] + // runCapture() always uses argv arrays, rejects `shell: true` above, and + // validates the executable/argv for process-spawn metacharacters such as + // NUL bytes. + // lgtm[js/indirect-command-line-injection] + // lgtm[js/shell-command-injection-from-environment] const result = spawnSync(exe, args, { ...spawnOpts, + shell: false, cwd: ROOT, env: buildRunnerEnv(extraEnv), stdio: ["pipe", "pipe", "pipe"], @@ -284,12 +317,16 @@ function runCaptureEx(cmd: readonly string[], opts: Omit { @@ -131,6 +130,23 @@ describe("sanitizeConfigFile", () => { // Should not throw, file unchanged expect(readFileSync(configPath, "utf-8")).toBe("not json at all"); }); + + it("does not follow config-file symlinks while sanitizing", () => { + const targetPath = join(tmpDir, "target.json"); + const linkPath = join(tmpDir, "openclaw.json"); + writeFileSync(targetPath, JSON.stringify({ apiKey: "sk-secret" })); + try { + symlinkSync(targetPath, linkPath); + } catch (error) { + const code = error && typeof error === "object" ? (error as { code?: string }).code : ""; + if (code === "EPERM" || code === "EACCES") return; + throw error; + } + + sanitizeConfigFile(linkPath); + + expect(JSON.parse(readFileSync(targetPath, "utf-8"))).toEqual({ apiKey: "sk-secret" }); + }); }); describe("isSensitiveFile", () => { diff --git a/src/lib/security/credential-filter.ts b/src/lib/security/credential-filter.ts index c4d16a6e9ee..048d678c7f7 100644 --- a/src/lib/security/credential-filter.ts +++ b/src/lib/security/credential-filter.ts @@ -10,13 +10,49 @@ // Credentials must never be baked into sandbox filesystems or local backups. // They are injected at runtime via OpenShell's provider credential mechanism. -import { chmodSync, existsSync, readFileSync, writeFileSync } from "node:fs"; -import { basename } from "node:path"; +import { randomUUID } from "node:crypto"; +import { + closeSync, + constants, + fstatSync, + lstatSync, + openSync, + readFileSync, + renameSync, + writeFileSync, +} from "node:fs"; +import { basename, dirname, join } from "node:path"; function parseJson(text: string): T { return JSON.parse(text); } +function readRegularFileNoFollow(filePath: string): string | null { + let fd: number; + try { + if (typeof constants.O_NOFOLLOW !== "number") { + const stat = lstatSync(filePath); + if (!stat.isFile() || stat.isSymbolicLink()) return null; + } + const noFollowFlag = typeof constants.O_NOFOLLOW === "number" ? constants.O_NOFOLLOW : 0; + fd = openSync(filePath, constants.O_RDONLY | noFollowFlag); + } catch { + return null; + } + try { + if (!fstatSync(fd).isFile()) return null; + return String(readFileSync(fd, "utf-8")); + } finally { + closeSync(fd); + } +} + +function writeFileAtomically(filePath: string, contents: string): void { + const tmpPath = join(dirname(filePath), `.${basename(filePath)}.${process.pid}.${randomUUID()}.tmp`); + writeFileSync(tmpPath, contents, { mode: 0o600 }); + renameSync(tmpPath, filePath); +} + /** * JSON-like configuration value supported by credential stripping. */ @@ -145,10 +181,11 @@ export function stripCredentials(obj: ConfigValue): ConfigValue { * Removes the "gateway" section (contains auth tokens — regenerated at startup). */ export function sanitizeConfigFile(configPath: string): void { - if (!existsSync(configPath)) return; + const rawConfig = readRegularFileNoFollow(configPath); + if (rawConfig === null) return; let parsed: ConfigValue; try { - parsed = parseJson(readFileSync(configPath, "utf-8")); + parsed = parseJson(rawConfig); } catch { return; // Not valid JSON — skip (may be YAML for Hermes) } @@ -156,8 +193,7 @@ export function sanitizeConfigFile(configPath: string): void { const { gateway: _gateway, ...config } = parsed; const sanitized = stripCredentials(config); - writeFileSync(configPath, JSON.stringify(sanitized, null, 2)); - chmodSync(configPath, 0o600); + writeFileAtomically(configPath, JSON.stringify(sanitized, null, 2)); } /** diff --git a/src/lib/state/onboard-session.test.ts b/src/lib/state/onboard-session.test.ts index 8d33a910d76..36852c9bec8 100644 --- a/src/lib/state/onboard-session.test.ts +++ b/src/lib/state/onboard-session.test.ts @@ -880,8 +880,8 @@ describe("onboard session", () => { // ORIGINAL stale lock (dead PID 999999), isProcessAlive // returns false, and acquireOnboardLock enters the stale- // cleanup path calling unlinkIfInodeMatches. - // - stat #2 (inside unlinkIfInodeMatches): BEFORE the actual - // stat, swap the file for a fresh claim. stat #2 then sees + // - stat #1 (inside unlinkIfInodeMatches): BEFORE the actual + // stat, swap the file for a fresh claim. stat #1 then sees // a different inode → must skip the unlink. // // CodeRabbit correctly flagged the original test: swapping on @@ -891,10 +891,10 @@ describe("onboard session", () => { const originalStatSync = fs.statSync; const statSpy = vi.spyOn(fs, "statSync").mockImplementation((...args) => { statCallCount += 1; - // Just before stat #2 (inside unlinkIfInodeMatches), simulate + // Just before stat #1 (inside unlinkIfInodeMatches), simulate // the race: a concurrent fast process unlinks the stale lock - // and writes a fresh claim. stat #2 then sees a new inode. - if (statCallCount === 2) { + // and writes a fresh claim. stat #1 then sees a new inode. + if (statCallCount === 1) { // Write the fresh claim to a temp file first, then rename over // the stale lock. This guarantees a different inode even on // tmpfs/overlayfs which can reuse inodes after unlink+recreate. @@ -914,9 +914,9 @@ describe("onboard session", () => { }); try { - // The acquire call will see EEXIST (stale lock present), stat it, - // then the swap happens, then the second stat (inside the cleanup - // helper) sees a different inode → must NOT unlink. + // The acquire call will see EEXIST (stale lock present), read it + // through a pinned descriptor, then the stat inside the cleanup + // helper sees a different inode → must NOT unlink. const result = session.acquireOnboardLock("nemoclaw onboard --resume"); // The fresh lock that the simulated concurrent process wrote // should still be on disk after acquireOnboardLock returns. @@ -976,7 +976,7 @@ describe("onboard session", () => { const statSpy = vi.spyOn(fs, "statSync").mockImplementation((...args) => { if (args[0] === session.LOCK_FILE) { statCallCount += 1; - if (statCallCount === 3) { + if (statCallCount === 1) { const tmpClaim = session.LOCK_FILE + ".race-tmp"; fs.writeFileSync( tmpClaim, diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index 6ace002a27a..7de7b276dda 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -573,6 +573,29 @@ function parseLockFile(contents: string): LockInfo | null { } } +interface LockFileSnapshot { + info: LockInfo | null; + inode: bigint; + mtimeMs: number; +} + +function readLockFileSnapshot(): LockFileSnapshot { + const fd = fs.openSync(LOCK_FILE, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW ?? 0)); + try { + const stat = fs.fstatSync(fd, { bigint: true }); + if (!stat.isFile()) { + return { info: null, inode: stat.ino, mtimeMs: Number(stat.mtimeMs) }; + } + return { + info: parseLockFile(String(fs.readFileSync(fd, "utf8"))), + inode: stat.ino, + mtimeMs: Number(stat.mtimeMs), + }; + } finally { + fs.closeSync(fd); + } +} + const MALFORMED_STALE_SECONDS = 30; function isProcessAlive(pid: number): boolean { @@ -674,34 +697,25 @@ export function acquireOnboardLock(command: string | null = null): LockResult { // the same stale lock, and the slower one will unlink the fresh // lock the faster one just claimed, breaking mutual exclusion. // See issue #1281. - let existing: LockInfo | null; - let staleInode: bigint | null; + let snapshot: LockFileSnapshot; try { - const stat = fs.statSync(LOCK_FILE, { bigint: true }); - staleInode = stat.ino; - existing = parseLockFile(fs.readFileSync(LOCK_FILE, "utf8")); + snapshot = readLockFileSnapshot(); } catch (readError) { if (isErrnoException(readError) && readError.code === "ENOENT") { continue; } throw readError; } + const { info: existing, inode: staleInode } = snapshot; if (!existing) { // Malformed lock file. If the file is very recent (<30 s), a // concurrent process may be mid-write — leave it and retry. // Otherwise the file is stale debris from a crash between // openSync("wx") and writeSync() — remove it so subsequent // onboard runs are not permanently blocked (#2765). - try { - const lockStat = fs.statSync(LOCK_FILE); - const ageMs = Date.now() - lockStat.mtimeMs; - if (ageMs > MALFORMED_STALE_SECONDS * 1000) { - unlinkIfInodeMatches(LOCK_FILE, staleInode); - } - } catch (statErr) { - if (!(isErrnoException(statErr) && statErr.code === "ENOENT")) { - throw statErr; - } + const ageMs = Date.now() - snapshot.mtimeMs; + if (ageMs > MALFORMED_STALE_SECONDS * 1000) { + unlinkIfInodeMatches(LOCK_FILE, staleInode); } continue; } @@ -832,17 +846,16 @@ export function releaseOnboardLock(): void { // behavior so we never unlink a malformed lock and never unlink a // lock owned by another pid. try { - if (!fs.existsSync(LOCK_FILE)) return; - let existing: LockInfo | null = null; + let snapshot: LockFileSnapshot; try { - existing = parseLockFile(fs.readFileSync(LOCK_FILE, "utf8")); + snapshot = readLockFileSnapshot(); } catch (error) { if (isErrnoException(error) && error.code === "ENOENT") return; throw error; } - if (!existing) return; - if (existing.pid !== process.pid) return; - fs.unlinkSync(LOCK_FILE); + if (!snapshot.info) return; + if (snapshot.info.pid !== process.pid) return; + unlinkIfInodeMatches(LOCK_FILE, snapshot.inode); } catch { return; }