diff --git a/agents/hermes/manifest.yaml b/agents/hermes/manifest.yaml index afe4a848842..c89d41a3cb0 100644 --- a/agents/hermes/manifest.yaml +++ b/agents/hermes/manifest.yaml @@ -17,7 +17,10 @@ install_method: curl # curl install.sh | bash binary_path: /usr/local/bin/hermes version_command: "hermes --version" expected_version: "2026.4.8" -gateway_command: "hermes gateway run" +gateway_argv: + - hermes + - gateway + - run # ── Health probe ──────────────────────────────────────────────── # The API server adapter listens on 8642 by default and exposes diff --git a/agents/openclaw/manifest.yaml b/agents/openclaw/manifest.yaml index c2b87b0d32d..f3324486a79 100644 --- a/agents/openclaw/manifest.yaml +++ b/agents/openclaw/manifest.yaml @@ -20,11 +20,14 @@ install_method: npm # npm install -g openclaw@ binary_path: /usr/local/bin/openclaw version_command: "openclaw --version" expected_version: "2026.4.9" -gateway_command: "openclaw gateway run" +gateway_argv: + - openclaw + - gateway + - run # ── Health probe ──────────────────────────────────────────────── health_probe: - url: "http://localhost:18789/" + url: "http://localhost:18789/health" port: 18789 timeout_seconds: 30 diff --git a/nemoclaw/src/index.ts b/nemoclaw/src/index.ts index 4d62108b2c0..00dbe501872 100644 --- a/nemoclaw/src/index.ts +++ b/nemoclaw/src/index.ts @@ -11,7 +11,7 @@ * time. */ -import { execFileSync } from "node:child_process"; +import { execaSync } from "execa"; import { handleSlashCommand } from "./commands/slash.js"; import { describeOnboardEndpoint, @@ -59,12 +59,11 @@ function readBeforeToolCallEvent( // sandbox). Returns empty strings if the probe fails. function probeOpenShellInference(): { endpoint: string; provider: string; model: string } { try { - const raw = execFileSync("openshell", ["inference", "get", "--json"], { - encoding: "utf-8", + const result = execaSync("openshell", ["inference", "get", "--json"], { timeout: 3000, - stdio: ["pipe", "pipe", "pipe"], + reject: true, }); - const parsed: unknown = JSON.parse(raw); + const parsed: unknown = JSON.parse(result.stdout); const parsedObject = typeof parsed === "object" && parsed !== null ? parsed : null; const endpoint = readStringProperty(parsedObject, "endpoint"); const provider = readStringProperty(parsedObject, "provider"); diff --git a/nemoclaw/src/lib/subprocess-env.ts b/nemoclaw/src/lib/subprocess-env.ts index 0ab7a9ad5e5..008379d7e83 100644 --- a/nemoclaw/src/lib/subprocess-env.ts +++ b/nemoclaw/src/lib/subprocess-env.ts @@ -26,7 +26,16 @@ const TEMP = ["TMPDIR", "TMP", "TEMP"]; const LOCALE = ["LANG"]; // LC_* handled via prefix -const PROXY = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"]; +const PROXY = [ + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "ALL_PROXY", + "http_proxy", + "https_proxy", + "no_proxy", + "all_proxy", +]; const TLS = [ "SSL_CERT_FILE", @@ -40,7 +49,17 @@ const TLS = [ const TOOLCHAIN = ["DOCKER_HOST", "KUBECONFIG", "SSH_AUTH_SOCK", "RUST_LOG", "RUST_BACKTRACE"]; -const ALLOWED_ENV_NAMES = new Set([...SYSTEM, ...TEMP, ...LOCALE, ...PROXY, ...TLS, ...TOOLCHAIN]); +const NEMOCLAW = ["NEMOCLAW_NON_INTERACTIVE"]; + +const ALLOWED_ENV_NAMES = new Set([ + ...SYSTEM, + ...TEMP, + ...LOCALE, + ...PROXY, + ...TLS, + ...TOOLCHAIN, + ...NEMOCLAW, +]); // ── Allowed prefixes ─────────────────────────────────────────── @@ -61,3 +80,20 @@ export function buildSubprocessEnv(extra?: Record): Record = {}; + for (const [key, value] of Object.entries(extraEnv ?? {})) { + if (value !== undefined) { + normalizedExtraEnv[key] = value; + } + } + return buildSubprocessEnv(normalizedExtraEnv); +} diff --git a/nemoclaw/src/register.test.ts b/nemoclaw/src/register.test.ts index b0c5e864a41..29e1b0f2a83 100644 --- a/nemoclaw/src/register.test.ts +++ b/nemoclaw/src/register.test.ts @@ -4,8 +4,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import type { OpenClawPluginApi } from "./index.js"; -vi.mock("node:child_process", () => ({ - execFileSync: vi.fn(), +vi.mock("execa", () => ({ + execaSync: vi.fn(), })); vi.mock("./onboard/config.js", () => ({ @@ -14,11 +14,11 @@ vi.mock("./onboard/config.js", () => ({ describeOnboardProvider: vi.fn(() => "NVIDIA Endpoint API"), })); -import { execFileSync } from "node:child_process"; +import { execaSync } from "execa"; import register, { getPluginConfig } from "./index.js"; import { loadOnboardConfig } from "./onboard/config.js"; -const mockedExecFileSync = vi.mocked(execFileSync); +const mockedExecaSync = vi.mocked(execaSync); const mockedLoadOnboardConfig = vi.mocked(loadOnboardConfig); function createMockApi(): OpenClawPluginApi { @@ -45,7 +45,7 @@ function createMockApi(): OpenClawPluginApi { describe("plugin registration", () => { beforeEach(() => { vi.clearAllMocks(); - mockedExecFileSync.mockReset(); + mockedExecaSync.mockReset(); mockedLoadOnboardConfig.mockReturnValue(null); }); @@ -86,13 +86,21 @@ describe("plugin registration", () => { }); it("uses probed OpenShell provider and model when onboard config is unavailable", () => { - mockedExecFileSync.mockReturnValue( - JSON.stringify({ + mockedExecaSync.mockReturnValue({ + command: "openshell inference get --json", + escapedCommand: "openshell inference get --json", + exitCode: 0, + stdout: JSON.stringify({ provider: "Ollama", endpoint: "http://host.docker.internal:11434/v1", model: "llama3.2:latest", }), - ); + stderr: "", + failed: false, + timedOut: false, + isCanceled: false, + killed: false, + } as ReturnType); const api = createMockApi(); register(api); @@ -114,12 +122,20 @@ describe("plugin registration", () => { }); it("does not treat the provider name as a fallback endpoint", () => { - mockedExecFileSync.mockReturnValue( - JSON.stringify({ + mockedExecaSync.mockReturnValue({ + command: "openshell inference get --json", + escapedCommand: "openshell inference get --json", + exitCode: 0, + stdout: JSON.stringify({ provider: "Ollama", model: "llama3.2:latest", }), - ); + stderr: "", + failed: false, + timedOut: false, + isCanceled: false, + killed: false, + } as ReturnType); const api = createMockApi(); register(api); @@ -133,6 +149,7 @@ describe("plugin registration", () => { describe("before_tool_call secret scanner hook (#1233)", () => { beforeEach(() => { vi.clearAllMocks(); + mockedExecaSync.mockReset(); mockedLoadOnboardConfig.mockReturnValue(null); }); diff --git a/src/lib/agent-defs.test.ts b/src/lib/agent-defs.test.ts index af91c364c48..86a9f764e93 100644 --- a/src/lib/agent-defs.test.ts +++ b/src/lib/agent-defs.test.ts @@ -39,8 +39,10 @@ describe("agent definitions", () => { expect(openclaw.name).toBe("openclaw"); expect(openclaw.displayName).toBe("OpenClaw"); + expect(openclaw.healthProbe.url).toBe("http://localhost:18789/health"); expect(openclaw.healthProbe.port).toBe(18789); expect(openclaw.forwardPort).toBe(18789); + expect(openclaw.gatewayArgv).toEqual(["openclaw", "gateway", "run"]); expect(openclaw.configPaths).toEqual({ immutableDir: "/sandbox/.openclaw", writableDir: "/sandbox/.openclaw-data", @@ -66,6 +68,7 @@ describe("agent definitions", () => { format: "yaml", }); expect(hermes.healthProbe.url).toBe("http://localhost:8642/health"); + expect(hermes.gatewayArgv).toEqual(["hermes", "gateway", "run"]); expect(hermes.messagingPlatforms).toEqual(["telegram", "discord", "slack"]); }); @@ -119,4 +122,18 @@ describe("agent definitions", () => { expect(() => loadAgent(agentName)).toThrow(/health_probe\.port/); }); + + it("rejects shell-style gateway_command values and requires gateway_argv", () => { + const agentName = `invalid-gateway-command-${String(Date.now())}`; + writeTempAgentManifest( + agentName, + [ + `name: ${agentName}`, + "display_name: Broken Gateway Command", + 'gateway_command: "python -m agent-launcher; echo pwned"', + ].join("\n"), + ); + + expect(() => loadAgent(agentName)).toThrow(/Use 'gateway_argv'/); + }); }); diff --git a/src/lib/agent-defs.ts b/src/lib/agent-defs.ts index 3a94c69f8a3..d3ff1eff42a 100644 --- a/src/lib/agent-defs.ts +++ b/src/lib/agent-defs.ts @@ -9,6 +9,7 @@ import path from "node:path"; import { ROOT } from "./runner"; import { DASHBOARD_PORT } from "./ports"; +import { joinShellWords } from "./shell-quote"; export const AGENTS_DIR = path.join(ROOT, "agents"); @@ -58,6 +59,7 @@ export interface AgentDefinition { version_command?: string; expected_version?: string; gateway_command?: string; + gateway_argv?: string[]; device_pairing?: boolean; phone_home_hosts?: string[]; forward_ports?: number[]; @@ -71,6 +73,7 @@ export interface AgentDefinition { readonly displayName: string; readonly healthProbe: AgentHealthProbe; readonly forwardPort: number; + readonly gatewayArgv: string[]; readonly dashboard: AgentDashboard; readonly configPaths: AgentConfigPaths; readonly stateDirs: string[]; @@ -141,6 +144,25 @@ function readStringArray(record: ManifestRecord, key: string): string[] | undefi return value.filter((entry): entry is string => typeof entry === "string"); } +function readCommandArray(record: ManifestRecord, key: string): string[] | undefined { + const value = record[key]; + if (value === undefined) return undefined; + if (!Array.isArray(value)) { + throw new Error(`Agent manifest field '${key}' must be an array of command arguments`); + } + + const args = value.map((entry, index) => { + if (typeof entry !== "string" || entry.trim() === "") { + throw new Error( + `Agent manifest field '${key}[${String(index)}]' must be a non-empty string command argument`, + ); + } + return entry; + }); + + return args.length > 0 ? args : undefined; +} + function isValidPort(value: unknown): value is number { return typeof value === "number" && Number.isInteger(value) && value >= 1 && value <= 65535; } @@ -215,6 +237,25 @@ function readMessagingPlatforms(record: ManifestRecord): { supported?: string[] return supported ? { supported } : {}; } +const SAFE_GATEWAY_COMMAND_TOKEN_RE = /^[A-Za-z0-9_@%+=:,./-]+$/; + +function parseLegacyGatewayCommand(gatewayCommand: string): string[] { + const trimmed = gatewayCommand.trim(); + if (!trimmed) { + throw new Error("Agent manifest field 'gateway_command' must not be empty"); + } + + const args = trimmed.split(/\s+/).filter(Boolean); + if (args.length === 0 || args.some((arg) => !SAFE_GATEWAY_COMMAND_TOKEN_RE.test(arg))) { + throw new Error( + "Agent manifest field 'gateway_command' only supports simple shell-free tokens. " + + "Use 'gateway_argv' for quoted or complex commands.", + ); + } + + return args; +} + function loadManifestRecord(manifestPath: string): ManifestRecord { const parsed = yaml.load(fs.readFileSync(manifestPath, "utf8")); if (!isManifestRecord(parsed)) { @@ -258,6 +299,8 @@ export function loadAgent(name: string): AgentDefinition { const versionCommand = readString(raw, "version_command"); const expectedVersion = readString(raw, "expected_version"); const gatewayCommand = readString(raw, "gateway_command"); + const gatewayArgv = readCommandArray(raw, "gateway_argv") ?? + (gatewayCommand ? parseLegacyGatewayCommand(gatewayCommand) : undefined); const forwardPorts = readPortArray(raw, "forward_ports"); const healthProbe = readHealthProbe(raw); const config = readObject(raw, "config"); @@ -274,7 +317,8 @@ export function loadAgent(name: string): AgentDefinition { binary_path: binaryPath, version_command: versionCommand, expected_version: expectedVersion, - gateway_command: gatewayCommand, + gateway_command: gatewayCommand ?? (gatewayArgv ? joinShellWords(gatewayArgv) : undefined), + gateway_argv: gatewayArgv, device_pairing: readBoolean(raw, "device_pairing"), phone_home_hosts: phoneHomeHosts, forward_ports: forwardPorts, @@ -304,6 +348,10 @@ export function loadAgent(name: string): AgentDefinition { return forwardPorts?.[0] ?? DASHBOARD_PORT; }, + get gatewayArgv(): string[] { + return gatewayArgv ?? [binaryPath ?? "openclaw", "gateway", "run"]; + }, + get dashboard(): AgentDashboard { const d = readObject(raw, "dashboard") ?? {}; const kind: AgentDashboardKind = d.kind === "api" ? "api" : "ui"; diff --git a/src/lib/agent-onboard.test.ts b/src/lib/agent-onboard.test.ts index 9d3b431c006..3fcefb96be0 100644 --- a/src/lib/agent-onboard.test.ts +++ b/src/lib/agent-onboard.test.ts @@ -7,9 +7,13 @@ import { printDashboardUi } from "../../dist/lib/agent-onboard"; import type { AgentDefinition } from "./agent-defs"; function makeAgent(overrides: Partial = {}): AgentDefinition { + const gatewayArgv = overrides.gatewayArgv ?? overrides.gateway_argv ?? ["agent", "gateway", "run"]; + return { name: "agent", displayName: "Agent", + gateway_argv: gatewayArgv, + gatewayArgv, healthProbe: { url: "http://127.0.0.1:19000/", port: 19000, timeout_seconds: 5 }, forwardPort: 19000, dashboard: { kind: "ui", label: "UI", path: "/" }, diff --git a/src/lib/agent-runtime.test.ts b/src/lib/agent-runtime.test.ts index dc933d7d4f9..cbe86b9a345 100644 --- a/src/lib/agent-runtime.test.ts +++ b/src/lib/agent-runtime.test.ts @@ -7,11 +7,17 @@ import { buildRecoveryScript } from "../../dist/lib/agent-runtime"; import type { AgentDefinition } from "./agent-defs"; function makeAgent(overrides: Partial = {}): AgentDefinition { + const gatewayArgv = + overrides.gatewayArgv ?? overrides.gateway_argv ?? ["test-agent", "gateway", "run"]; + const gatewayCommand = overrides.gateway_command ?? gatewayArgv.join(" "); + return { name: "test-agent", displayName: "Test Agent", binary_path: "/usr/local/bin/test-agent", - gateway_command: "test-agent gateway run", + gateway_command: gatewayCommand, + gateway_argv: gatewayArgv, + gatewayArgv, healthProbe: { url: "http://127.0.0.1:19000/", port: 19000, timeout_seconds: 5 }, forwardPort: 19000, dashboard: { kind: "ui", label: "UI", path: "/" }, @@ -60,20 +66,25 @@ describe("buildRecoveryScript", () => { it("launches the default gateway command through the validated agent binary", () => { const script = buildRecoveryScript(minimalAgent, 19000); - expect(script).toContain("command -v 'test-agent'"); + expect(script).toContain("command -v test-agent"); + expect(script).toContain("%{http_code}"); expect(script).toContain('nohup "$AGENT_BIN" gateway run --port 19000'); }); - it("falls back to openclaw gateway run when gateway_command is absent", () => { - const agent = makeAgent({ gateway_command: undefined }); + it("launches the default gateway argv when no raw gateway_command is present", () => { + const agent = makeAgent({ gateway_command: undefined, gateway_argv: undefined, gatewayArgv: ["test-agent", "gateway", "run"] }); const script = buildRecoveryScript(agent, 19000); expect(script).toContain('nohup "$AGENT_BIN" gateway run --port 19000'); }); - it("validates and launches custom gateway commands explicitly", () => { - const agent = makeAgent({ gateway_command: "custom-launch --mode recovery" }); + it("validates and launches custom gateway argv explicitly", () => { + const agent = makeAgent({ + gateway_command: undefined, + gateway_argv: ["custom-launch", "--mode", "recovery"], + gatewayArgv: ["custom-launch", "--mode", "recovery"], + }); const script = buildRecoveryScript(agent, 19000); - expect(script).toContain("GATEWAY_CMD_BIN='custom-launch'"); + expect(script).toContain("GATEWAY_CMD_BIN=custom-launch"); expect(script).toContain('command -v "$GATEWAY_CMD_BIN" >/dev/null 2>&1'); expect(script).toContain("nohup custom-launch --mode recovery --port 19000"); }); diff --git a/src/lib/agent-runtime.ts b/src/lib/agent-runtime.ts index 5a1670c8db0..809a279a4dc 100644 --- a/src/lib/agent-runtime.ts +++ b/src/lib/agent-runtime.ts @@ -11,7 +11,7 @@ import * as registry from "./registry"; import { DASHBOARD_PORT } from "./ports"; import * as onboardSession from "./onboard-session"; import { loadAgent, type AgentDefinition } from "./agent-defs"; -import { shellQuote } from "./runner"; +import { buildShellAssignment, formatShellToken, joinShellWords } from "./shell-quote"; /** * Resolve the agent for a sandbox. Checks the per-sandbox registry first @@ -44,8 +44,8 @@ export function getSessionAgent(sandboxName?: string): AgentDefinition | null { * Returns the agent's configured probe URL, or the OpenClaw default. */ export function getHealthProbeUrl(agent: AgentDefinition | null): string { - if (!agent) return `http://127.0.0.1:${DASHBOARD_PORT}/`; - return agent.healthProbe?.url || `http://127.0.0.1:${DASHBOARD_PORT}/`; + if (!agent) return `http://127.0.0.1:${DASHBOARD_PORT}/health`; + return agent.healthProbe?.url || `http://127.0.0.1:${DASHBOARD_PORT}/health`; } /** @@ -59,29 +59,31 @@ export function buildRecoveryScript(agent: AgentDefinition | null, port: number) const probeUrl = getHealthProbeUrl(agent); const binaryPath = agent.binary_path || "/usr/local/bin/openclaw"; const binaryName = binaryPath.split("/").pop() ?? "openclaw"; - const defaultGatewayCommand = `${binaryName} gateway run`; - const configuredGatewayCommand = agent.gateway_command?.trim() || defaultGatewayCommand; - const usesValidatedBinary = configuredGatewayCommand === defaultGatewayCommand; - const customGatewayExecutable = configuredGatewayCommand.split(/\s+/)[0] ?? binaryName; + const defaultGatewayArgv = [binaryName, "gateway", "run"]; + const configuredGatewayArgv = agent.gatewayArgv; + const usesValidatedBinary = + configuredGatewayArgv.length === defaultGatewayArgv.length && + configuredGatewayArgv.every((value, index) => value === defaultGatewayArgv[index]); + const customGatewayExecutable = configuredGatewayArgv[0] ?? binaryName; const validationSteps = usesValidatedBinary ? [ - `AGENT_BIN=${shellQuote(binaryPath)}; if [ ! -x "$AGENT_BIN" ]; then AGENT_BIN="$(command -v ${shellQuote(binaryName)})"; fi;`, + `${buildShellAssignment("AGENT_BIN", binaryPath)}; if [ ! -x "$AGENT_BIN" ]; then AGENT_BIN="$(command -v ${formatShellToken(binaryName)})"; fi;`, 'if [ -z "$AGENT_BIN" ]; then echo AGENT_MISSING; exit 1; fi;', ] : [ - `GATEWAY_CMD_BIN=${shellQuote(customGatewayExecutable)};`, + `${buildShellAssignment("GATEWAY_CMD_BIN", customGatewayExecutable)};`, 'case "$GATEWAY_CMD_BIN" in */*) [ -x "$GATEWAY_CMD_BIN" ] || { echo AGENT_MISSING; exit 1; } ;; *) command -v "$GATEWAY_CMD_BIN" >/dev/null 2>&1 || { echo AGENT_MISSING; exit 1; } ;; esac;', ]; const launchCommand = usesValidatedBinary - ? `nohup "$AGENT_BIN" gateway run --port ${port} > /tmp/gateway.log 2>&1 &` - : `nohup ${configuredGatewayCommand} --port ${port} > /tmp/gateway.log 2>&1 &`; + ? `nohup "$AGENT_BIN" ${joinShellWords(configuredGatewayArgv.slice(1))} --port ${port} > /tmp/gateway.log 2>&1 &` + : `nohup ${joinShellWords(configuredGatewayArgv)} --port ${port} > /tmp/gateway.log 2>&1 &`; const isHermes = agent.name === "hermes"; const hermesHome = isHermes ? "export HERMES_HOME=/sandbox/.hermes-data; " : ""; return [ "[ -f ~/.bashrc ] && . ~/.bashrc 2>/dev/null;", hermesHome, - `if curl -sf --max-time 3 ${shellQuote(probeUrl)} > /dev/null 2>&1; then echo ALREADY_RUNNING; exit 0; fi;`, + `HEALTH_CODE="$(curl -so /dev/null -w '%{http_code}' --max-time 3 ${formatShellToken(probeUrl)} 2>/dev/null || echo 000)"; if [ "$HEALTH_CODE" = "200" ] || [ "$HEALTH_CODE" = "401" ]; then echo ALREADY_RUNNING; exit 0; fi;`, "rm -f /tmp/gateway.log;", "touch /tmp/gateway.log; chmod 600 /tmp/gateway.log;", ...validationSteps, @@ -102,5 +104,5 @@ export function getAgentDisplayName(agent: AgentDefinition | null): string { * Get the gateway command for the current agent. */ export function getGatewayCommand(agent: AgentDefinition | null): string { - return agent?.gateway_command || "openclaw gateway run"; + return agent ? joinShellWords(agent.gatewayArgv) : "openclaw gateway run"; } diff --git a/src/lib/config-io.ts b/src/lib/config-io.ts index fa7f3e3e4ac..688f87c7b30 100644 --- a/src/lib/config-io.ts +++ b/src/lib/config-io.ts @@ -7,7 +7,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { shellQuote } from "./shell-quote"; +import { buildShellCommand } from "./remote-script"; +import { buildShellAssignment, formatShellToken, joinShellWords } from "./shell-quote"; import { isErrnoException, isPermissionError } from "./errno"; // Strict JSON types for file serialization — unlike json-types.ts, @@ -46,17 +47,17 @@ function buildRemediation(): string { " To fix, try one of these recovery paths:", "", " # If you can use sudo, repair the existing config directory:", - ` sudo chown -R $(whoami) ${shellQuote(nemoclawDir)}`, + ` sudo chown -R $(whoami) ${formatShellToken(nemoclawDir)}`, " # or recreate it if it was created by another user:", - ` sudo rm -rf ${shellQuote(nemoclawDir)} && nemoclaw onboard`, + ` ${joinShellWords(["sudo", "rm", "-rf", nemoclawDir])} && nemoclaw onboard`, "", " # If sudo is unavailable, move the bad config aside from a writable HOME:", - ` mv ${shellQuote(nemoclawDir)} ${shellQuote(backupDir)} && nemoclaw onboard`, + ` ${joinShellWords(["mv", nemoclawDir, backupDir])} && nemoclaw onboard`, " # or, if you already own the directory, remove it without sudo:", - ` rm -rf ${shellQuote(nemoclawDir)} && nemoclaw onboard`, + ` ${joinShellWords(["rm", "-rf", nemoclawDir])} && nemoclaw onboard`, "", " # If HOME itself is not writable, start NemoClaw with a writable HOME:", - ` mkdir -p ${shellQuote(recoveryHome)} && HOME=${shellQuote(recoveryHome)} nemoclaw onboard`, + ` ${joinShellWords(["mkdir", "-p", recoveryHome])} && ${buildShellAssignment("HOME", recoveryHome)} nemoclaw onboard`, "", " This usually happens when NemoClaw was first run with sudo", " or the config directory was created by a different user.", @@ -129,7 +130,7 @@ function rejectSymlinksOnPath(dirPath: string): void { throw new Error( `Refusing to use config directory: ${current} is a symbolic link ` + `(target: ${target}). This may indicate a symlink attack. ` + - `Remove the symlink and retry: rm ${shellQuote(current)}`, + `Remove the symlink and retry: ${buildShellCommand({ commandArgs: ["rm", current] })}`, ); } } catch (error) { diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 2426335896a..2be12182e7c 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,7 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execFileSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -10,6 +9,10 @@ import readline from "node:readline"; import { readConfigFile, writeConfigFile } from "./config-io"; import { isErrnoException } from "./errno"; +// runner.ts still uses CommonJS-style exports — use require here. +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { runCapture } = require("./runner"); + const UNSAFE_HOME_PATHS = new Set(["/tmp", "/var/tmp", "/dev/shm", "/"]); type CredentialInput = string | null | undefined; @@ -293,15 +296,10 @@ export async function ensureApiKey(): Promise { } export function isRepoPrivate(repo: string): boolean { - try { - const json = execFileSync("gh", ["api", `repos/${repo}`, "--jq", ".private"], { - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return json === "true"; - } catch { - return false; - } + const json = runCapture(["gh", "api", `repos/${repo}`, "--jq", ".private"], { + ignoreError: true, + }).trim(); + return json === "true"; } export async function ensureGithubToken(): Promise { @@ -311,17 +309,10 @@ export async function ensureGithubToken(): Promise { return; } - try { - token = execFileSync("gh", ["auth", "token"], { - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - if (token) { - process.env.GITHUB_TOKEN = token; - return; - } - } catch { - /* ignored */ + token = runCapture(["gh", "auth", "token"], { ignoreError: true }).trim(); + if (token) { + process.env.GITHUB_TOKEN = token; + return; } console.log(""); diff --git a/src/lib/debug.ts b/src/lib/debug.ts index b1138228da0..137e27a9498 100644 --- a/src/lib/debug.ts +++ b/src/lib/debug.ts @@ -1,12 +1,24 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { + closeSync, + existsSync, + mkdtempSync, + openSync, + readFileSync, + readSync, + rmSync, + statSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { spawnResult } from "./process-primitives.js"; import { platform, tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { DASHBOARD_PORT } from "./ports"; +import { hasExecutable } from "./find-executable"; import { listSandboxes } from "./registry"; // --------------------------------------------------------------------------- @@ -62,66 +74,222 @@ export { redact }; const isMacOS = platform() === "darwin"; const TIMEOUT_MS = 30_000; +const CAPTURE_CHUNK_BYTES = 16 * 1024; +const CAPTURE_IN_MEMORY_LIMIT_BYTES = 1024 * 1024; +const CAPTURE_FIRST_LINES = 400; +const CAPTURE_LAST_LINES = 400; function commandExists(cmd: string): boolean { + return hasExecutable(cmd); +} + +function runCommand( + command: string, + args: string[], + opts: { + timeout?: number; + stdio?: import("node:child_process").StdioOptions; + encoding?: BufferEncoding; + input?: string | Buffer; + } = {}, +) { + return spawnResult(command, args, { + timeout: opts.timeout ?? TIMEOUT_MS, + stdio: opts.stdio ?? ["ignore", "pipe", "pipe"], + encoding: opts.encoding ?? "utf-8", + input: opts.input, + }); +} + +function normalizeCapturedText(text: string): string { + return text.replace(/\r\n/g, "\n"); +} + +function splitCapturedLines(text: string): string[] { + const lines = normalizeCapturedText(text).split("\n"); + if (lines[lines.length - 1] === "") { + lines.pop(); + } + return lines; +} + +function readFirstLinesFromFile(filePath: string, count: number): string { + const fd = openSync(filePath, "r"); + const chunk = Buffer.alloc(CAPTURE_CHUNK_BYTES); + const lines: string[] = []; + let pending = ""; + let position = 0; + + try { + while (lines.length < count) { + const bytesRead = readSync(fd, chunk, 0, chunk.length, position); + if (bytesRead === 0) { + break; + } + position += bytesRead; + const combined = normalizeCapturedText( + `${pending}${chunk.toString("utf-8", 0, bytesRead)}`, + ); + const parts = combined.split("\n"); + pending = parts.pop() ?? ""; + lines.push(...parts); + } + if (pending && lines.length < count) { + lines.push(pending); + } + return lines.slice(0, count).join("\n"); + } finally { + closeSync(fd); + } +} + +function readLastLinesFromFile(filePath: string, count: number): string { + const fd = openSync(filePath, "r"); + const size = statSync(filePath).size; + let position = size; + let tail = ""; + try { - // Use sh -c with the command as a separate argument to avoid shell injection. - // While cmd values are hardcoded internally, this is defensive. - execFileSync("sh", ["-c", `command -v "$1"`, "--", cmd], { - stdio: ["ignore", "ignore", "ignore"], + while (position > 0) { + const readSize = Math.min(CAPTURE_CHUNK_BYTES, position); + position -= readSize; + const chunk = Buffer.alloc(readSize); + const bytesRead = readSync(fd, chunk, 0, readSize, position); + if (bytesRead === 0) { + break; + } + tail = normalizeCapturedText(chunk.toString("utf-8", 0, bytesRead)) + tail; + if (splitCapturedLines(tail).length > count) { + break; + } + } + const lines = splitCapturedLines(tail); + return lines.slice(Math.max(0, lines.length - count)).join("\n"); + } finally { + closeSync(fd); + } +} + +function readBoundedOutput(filePath: string): string { + if (!existsSync(filePath)) { + return ""; + } + + // Debug commands can dump hundreds of megabytes (docker inspect, tar + // listings, verbose logs). Keep small outputs intact, but for giant captures + // preserve both the start and the end so operators still see the command + // preamble plus the final failure context without loading the whole file. + + const size = statSync(filePath).size; + if (size === 0) { + return ""; + } + + if (size <= CAPTURE_IN_MEMORY_LIMIT_BYTES) { + return normalizeCapturedText(readFileSync(filePath, "utf-8")); + } + + const first = readFirstLinesFromFile(filePath, CAPTURE_FIRST_LINES); + const last = readLastLinesFromFile(filePath, CAPTURE_LAST_LINES); + return `${first}\n... output truncated ...\n${last}`; +} + +function runCommandForTransform( + command: string, + args: string[], + opts: { timeout?: number; input?: string | Buffer } = {}, +): { stdout: string; stderr: string; status: number } { + const captureDir = mkdtempSync(join(tmpdir(), "nemoclaw-debug-capture-")); + const stdoutPath = join(captureDir, "stdout.txt"); + const stderrPath = join(captureDir, "stderr.txt"); + const stdoutFd = openSync(stdoutPath, "w"); + const stderrFd = openSync(stderrPath, "w"); + + try { + const result = spawnResult(command, args, { + timeout: opts.timeout ?? TIMEOUT_MS, + stdio: ["ignore", stdoutFd, stderrFd], + input: opts.input, }); - return true; - } catch { - return false; + return { + stdout: readBoundedOutput(stdoutPath), + stderr: readBoundedOutput(stderrPath), + status: result.status ?? 1, + }; + } finally { + closeSync(stdoutFd); + closeSync(stderrFd); + rmSync(captureDir, { recursive: true, force: true }); } } -function collect(collectDir: string, label: string, command: string, args: string[]): void { +function writeCollectedOutput(collectDir: string, label: string, raw: string, status: number): void { const filename = label.replace(/[ /]/g, (c) => (c === " " ? "_" : "-")); const outfile = join(collectDir, `${filename}.txt`); + const redacted = redact(raw); + writeFileSync(outfile, redacted); + console.log(redacted.trimEnd()); + + if (status !== 0) { + console.log(" (command exited with non-zero status)"); + } +} +function collect(collectDir: string, label: string, command: string, args: string[]): void { if (!commandExists(command)) { const msg = ` (${command} not found, skipping)`; console.log(msg); - writeFileSync(outfile, msg + "\n"); + writeCollectedOutput(collectDir, label, msg + "\n", 0); return; } - const result = spawnSync(command, args, { + const result = runCommand(command, args, { timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8", }); - const raw = (result.stdout ?? "") + "\n" + (result.stderr ?? ""); - const redacted = redact(raw); - writeFileSync(outfile, redacted); - console.log(redacted.trimEnd()); - - if (result.status !== 0) { - console.log(" (command exited with non-zero status)"); - } + writeCollectedOutput( + collectDir, + label, + `${String(result.stdout ?? "")}\n${String(result.stderr ?? "")}`, + result.status ?? 1, + ); } -/** Run a shell one-liner via `sh -c`. */ -function collectShell(collectDir: string, label: string, shellCmd: string): void { - const filename = label.replace(/[ /]/g, (c) => (c === " " ? "_" : "-")); - const outfile = join(collectDir, `${filename}.txt`); +function collectTransformed( + collectDir: string, + label: string, + command: string, + args: string[], + transform: (result: { stdout: string; stderr: string; status: number }) => string, +): void { + if (!commandExists(command)) { + const msg = ` (${command} not found, skipping)`; + console.log(msg); + writeCollectedOutput(collectDir, label, msg + "\n", 0); + return; + } - const result = spawnSync("sh", ["-c", shellCmd], { + const result = runCommandForTransform(command, args, { timeout: TIMEOUT_MS, - stdio: ["ignore", "pipe", "pipe"], - encoding: "utf-8", }); - const raw = (result.stdout ?? "") + "\n" + (result.stderr ?? ""); - const redacted = redact(raw); - writeFileSync(outfile, redacted); - console.log(redacted.trimEnd()); + writeCollectedOutput( + collectDir, + label, + transform(result), + result.status, + ); +} - if (result.status !== 0) { - console.log(" (command exited with non-zero status)"); - } +function takeFirstLines(text: string, count: number): string { + return text.split("\n").slice(0, count).join("\n"); +} + +function takeLastLines(text: string, count: number): string { + const lines = text.split("\n"); + return lines.slice(Math.max(0, lines.length - count)).join("\n"); } // --------------------------------------------------------------------------- @@ -147,12 +315,14 @@ function detectSandboxName(): string { // Fallback: ask the live gateway directly if (!commandExists("openshell")) return "default"; try { - const output = execFileSync("openshell", ["sandbox", "list"], { - encoding: "utf-8", - timeout: 10_000, - stdio: ["ignore", "pipe", "ignore"], - }); - const lines = output.split("\n").filter((l) => l.trim().length > 0); + const output = String( + runCommand("openshell", ["sandbox", "list"], { + timeout: 10_000, + stdio: ["ignore", "pipe", "ignore"], + encoding: "utf-8", + }).stdout ?? "", + ); + const lines = output.split("\n").filter((l: string) => l.trim().length > 0); for (const line of lines) { const first = line.trim().split(/\s+/)[0]; if (first && first.toLowerCase() !== "name") return first; @@ -174,11 +344,15 @@ function collectSystem(collectDir: string, quick: boolean): void { collect(collectDir, "uptime", "uptime", []); if (isMacOS) { - collectShell( - collectDir, - "memory", - 'echo "Physical: $(($(sysctl -n hw.memsize) / 1048576)) MB"; vm_stat', - ); + collectTransformed(collectDir, "memory", "sysctl", ["-n", "hw.memsize"], ({ stdout }) => { + const memBytes = Number.parseInt(stdout.trim(), 10) || 0; + const physicalMb = Math.floor(memBytes / 1048576); + const vmStat = String( + runCommand("vm_stat", [], { timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "pipe"] }) + .stdout ?? "", + ); + return `Physical: ${physicalMb} MB\n${vmStat}`; + }); } else { collect(collectDir, "free", "free", ["-m"]); } @@ -191,34 +365,42 @@ function collectSystem(collectDir: string, quick: boolean): void { function collectProcesses(collectDir: string, quick: boolean): void { section("Processes"); if (isMacOS) { - collectShell( + collectTransformed( collectDir, "ps-cpu", - "ps -eo pid,ppid,comm,%mem,%cpu | sort -k5 -rn | head -30", + "ps", + ["-r", "-A", "-o", "pid,ppid,comm,%mem,%cpu"], + ({ stdout }) => takeFirstLines(stdout, 30), ); } else { - collectShell( + collectTransformed( collectDir, "ps-cpu", - "ps -eo pid,ppid,cmd,%mem,%cpu --sort=-%cpu | head -30", + "ps", + ["-eo", "pid,ppid,cmd,%mem,%cpu", "--sort=-%cpu"], + ({ stdout }) => takeFirstLines(stdout, 30), ); } if (!quick) { if (isMacOS) { - collectShell( + collectTransformed( collectDir, "ps-mem", - "ps -eo pid,ppid,comm,%mem,%cpu | sort -k4 -rn | head -30", + "ps", + ["-m", "-A", "-o", "pid,ppid,comm,%mem,%cpu"], + ({ stdout }) => takeFirstLines(stdout, 30), ); - collectShell(collectDir, "top", "top -l 1 | head -50"); + collectTransformed(collectDir, "top", "top", ["-l", "1"], ({ stdout }) => takeFirstLines(stdout, 50)); } else { - collectShell( + collectTransformed( collectDir, "ps-mem", - "ps -eo pid,ppid,cmd,%mem,%cpu --sort=-%mem | head -30", + "ps", + ["-eo", "pid,ppid,cmd,%mem,%cpu", "--sort=-%mem"], + ({ stdout }) => takeFirstLines(stdout, 30), ); - collectShell(collectDir, "top", "top -b -n 1 | head -50"); + collectTransformed(collectDir, "top", "top", ["-b", "-n", "1"], ({ stdout }) => takeFirstLines(stdout, 50)); } } } @@ -255,12 +437,14 @@ function collectDocker(collectDir: string, quick: boolean): void { // NemoClaw-labelled containers if (commandExists("docker")) { try { - const output = execFileSync( - "docker", - ["ps", "-a", "--filter", "label=com.nvidia.nemoclaw", "--format", "{{.Names}}"], - { encoding: "utf-8", timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "ignore"] }, + const output = String( + runCommand( + "docker", + ["ps", "-a", "--filter", "label=com.nvidia.nemoclaw", "--format", "{{.Names}}"], + { timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "ignore"], encoding: "utf-8" }, + ).stdout ?? "", ); - const containers = output.split("\n").filter((c) => c.trim().length > 0); + const containers = output.split("\n").filter((c: string) => c.trim().length > 0); for (const cid of containers) { collect(collectDir, `docker-logs-${cid}`, "docker", ["logs", "--tail", "200", cid]); if (!quick) { @@ -298,15 +482,17 @@ function collectSandboxInternals( // Check if sandbox exists try { - const output = execFileSync("openshell", ["sandbox", "list"], { - encoding: "utf-8", - timeout: 10_000, - stdio: ["ignore", "pipe", "ignore"], - }); + const output = String( + runCommand("openshell", ["sandbox", "list"], { + timeout: 10_000, + stdio: ["ignore", "pipe", "ignore"], + encoding: "utf-8", + }).stdout ?? "", + ); const names = output .split("\n") - .map((l) => l.trim().split(/\s+/)[0]) - .filter((n) => n && n.toLowerCase() !== "name"); + .map((l: string) => l.trim().split(/\s+/)[0]) + .filter((n: string) => n && n.toLowerCase() !== "name"); if (!names.includes(sandboxName)) return; } catch { return; @@ -317,7 +503,7 @@ function collectSandboxInternals( // Generate temporary SSH config const sshConfigPath = join(tmpdir(), `nemoclaw-ssh-${String(Date.now())}`); try { - const sshResult = spawnSync("openshell", ["sandbox", "ssh-config", sandboxName], { + const sshResult = runCommand("openshell", ["sandbox", "ssh-config", sandboxName], { timeout: TIMEOUT_MS, stdio: ["ignore", "pipe", "ignore"], encoding: "utf-8", @@ -367,7 +553,12 @@ function collectSandboxInternals( function collectNetwork(collectDir: string): void { section("Network"); if (isMacOS) { - collectShell(collectDir, "listening", "netstat -anp tcp | grep LISTEN"); + collectTransformed(collectDir, "listening", "netstat", ["-anp", "tcp"], ({ stdout }) => + stdout + .split("\n") + .filter((line: string) => line.includes("LISTEN")) + .join("\n"), + ); collect(collectDir, "ifconfig", "ifconfig", []); collect(collectDir, "routes", "netstat", ["-rn"]); collect(collectDir, "dns-config", "scutil", ["--dns"]); @@ -375,15 +566,22 @@ function collectNetwork(collectDir: string): void { collect(collectDir, "ss", "ss", ["-ltnp"]); collect(collectDir, "ip-addr", "ip", ["addr"]); collect(collectDir, "ip-route", "ip", ["route"]); - collectShell(collectDir, "resolv-conf", "cat /etc/resolv.conf"); + collect(collectDir, "resolv-conf", "cat", ["/etc/resolv.conf"]); } collect(collectDir, "nslookup", "nslookup", ["integrate.api.nvidia.com"]); - collectShell( + collectTransformed( collectDir, "curl-models", - 'code=$(curl -s -o /dev/null -w "%{http_code}" https://integrate.api.nvidia.com/v1/models); echo "HTTP $code"; if [ "$code" -ge 200 ] && [ "$code" -lt 500 ]; then echo "NIM API reachable"; else echo "NIM API unreachable"; exit 1; fi', + "curl", + ["-s", "-o", "/dev/null", "-w", "%{http_code}", "https://integrate.api.nvidia.com/v1/models"], + ({ stdout }) => { + const code = Number.parseInt(stdout.trim(), 10) || 0; + return `HTTP ${code || 0}\n${code >= 200 && code < 500 ? "NIM API reachable" : "NIM API unreachable"}`; + }, + ); + collectTransformed(collectDir, "lsof-net", "lsof", ["-i", "-P", "-n"], ({ stdout }) => + takeFirstLines(stdout, 50), ); - collectShell(collectDir, "lsof-net", "lsof -i -P -n 2>/dev/null | head -50"); collect(collectDir, "lsof-18789", "lsof", ["-i", `:${DASHBOARD_PORT}`]); } @@ -419,13 +617,15 @@ function collectKernel(collectDir: string): void { function collectKernelMessages(collectDir: string): void { section("Kernel Messages"); if (isMacOS) { - collectShell( + collectTransformed( collectDir, "system-log", - 'log show --last 5m --predicate "eventType == logEvent" --style compact 2>/dev/null | tail -100', + "log", + ["show", "--last", "5m", "--predicate", "eventType == logEvent", "--style", "compact"], + ({ stdout }) => takeLastLines(stdout, 100), ); } else { - collectShell(collectDir, "dmesg", "dmesg | tail -100"); + collectTransformed(collectDir, "dmesg", "dmesg", [], ({ stdout }) => takeLastLines(stdout, 100)); } } @@ -438,7 +638,7 @@ function collectKernelMessages(collectDir: string): void { * guidance that goes with the generated file. */ export function createTarball(collectDir: string, output: string): boolean { - const result = spawnSync("tar", ["czf", output, "-C", dirname(collectDir), basename(collectDir)], { + const result = runCommand("tar", ["czf", output, "-C", dirname(collectDir), basename(collectDir)], { stdio: "inherit", timeout: 60_000, }); diff --git a/src/lib/deploy.test.ts b/src/lib/deploy.test.ts index 62189a9e504..0a6cf92087c 100644 --- a/src/lib/deploy.test.ts +++ b/src/lib/deploy.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from "vitest"; import { buildDeployEnvLines, + executeDeploy, findBrevInstanceStatus, inferDeployProvider, isBrevInstanceFailed, @@ -49,16 +50,15 @@ describe("buildDeployEnvLines", () => { credentials: { NVIDIA_API_KEY: "nvapi-test", }, - shellQuote: (value: string) => `'${value}'`, }); expect(envLines).toContain("NEMOCLAW_NON_INTERACTIVE=1"); expect(envLines).toContain("NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1"); - expect(envLines).toContain("NEMOCLAW_SANDBOX_NAME='my-assistant'"); - expect(envLines).toContain("NEMOCLAW_PROVIDER='build'"); - expect(envLines).toContain("CHAT_UI_URL='https://chat.example.com'"); - expect(envLines).toContain("NEMOCLAW_POLICY_MODE='suggested'"); - expect(envLines).toContain("NVIDIA_API_KEY='nvapi-test'"); + expect(envLines).toContain("NEMOCLAW_SANDBOX_NAME=my-assistant"); + expect(envLines).toContain("NEMOCLAW_PROVIDER=build"); + expect(envLines).toContain("CHAT_UI_URL=https://chat.example.com"); + expect(envLines).toContain("NEMOCLAW_POLICY_MODE=suggested"); + expect(envLines).toContain("NVIDIA_API_KEY=nvapi-test"); }); it("passes ALLOWED_CHAT_IDS through when Telegram is configured", () => { @@ -70,11 +70,10 @@ describe("buildDeployEnvLines", () => { TELEGRAM_BOT_TOKEN: "123456:telegram-token", ALLOWED_CHAT_IDS: "111,222", }, - shellQuote: (value: string) => `'${value}'`, }); - expect(envLines).toContain("TELEGRAM_BOT_TOKEN='123456:telegram-token'"); - expect(envLines).toContain("ALLOWED_CHAT_IDS='111,222'"); + expect(envLines).toContain("TELEGRAM_BOT_TOKEN=123456:telegram-token"); + expect(envLines).toContain("ALLOWED_CHAT_IDS=111,222"); }); it("omits ALLOWED_CHAT_IDS when Telegram is not configured", () => { @@ -85,10 +84,51 @@ describe("buildDeployEnvLines", () => { credentials: { ALLOWED_CHAT_IDS: "111,222", }, - shellQuote: (value: string) => `'${value}'`, }); - expect(envLines).not.toContain("ALLOWED_CHAT_IDS='111,222'"); + expect(envLines).not.toContain("ALLOWED_CHAT_IDS=111,222"); + }); +}); + +describe("executeDeploy", () => { + it("fails fast when `brev ls` errors instead of creating a new instance", async () => { + const commands: string[] = []; + const errors: string[] = []; + + await expect( + executeDeploy({ + instanceName: "target-box", + env: {}, + rootDir: "/tmp/nemoclaw", + getCredential: (key: string) => (key === "NVIDIA_API_KEY" ? "nvapi-test" : null), + validateName: (value: string) => value, + run: (command) => { + commands.push(command.join(" ")); + if (command[0] === "which") { + return { status: 0, stdout: "/usr/bin/brev\n", stderr: "" }; + } + if (command[0] === "brev" && command[1] === "ls") { + return { status: 1, stdout: "", stderr: "brev auth failed" }; + } + return { status: 0, stdout: "", stderr: "" }; + }, + runInteractive: () => { + throw new Error("should not connect interactively"); + }, + log: () => {}, + error: (message?: string) => { + if (message) errors.push(message); + }, + stdoutWrite: () => {}, + exit: (code: number) => { + throw new Error(`exit:${code}`); + }, + }), + ).rejects.toThrow("exit:1"); + + expect(errors).toContain(" Failed to query existing Brev instances."); + expect(errors.some((line) => line.includes("brev auth failed"))).toBe(true); + expect(commands.some((line) => line.startsWith("brev create "))).toBe(false); }); }); diff --git a/src/lib/deploy.ts b/src/lib/deploy.ts index 2cddce9a5e7..bcbf3845b7c 100644 --- a/src/lib/deploy.ts +++ b/src/lib/deploy.ts @@ -5,6 +5,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { buildSshScriptCommand } from "./remote-script"; +import { buildShellAssignment, formatShellToken } from "./shell-quote"; import { sleepSeconds } from "./wait"; type ExecLikeValue = @@ -17,12 +19,18 @@ type ExecLikeValue = | NodeJS.ProcessEnv | object; type ExecLikeOptions = { [key: string]: ExecLikeValue }; - -function readCommandOutput(error: object | null, key: "stdout" | "stderr"): string { - if (error === null) { +type ExecResultLike = { + status: number | null; + stdout?: string | Buffer; + stderr?: string | Buffer; + error?: Error | null; +}; + +function readCommandOutput(result: ExecResultLike | null, key: "stdout" | "stderr"): string { + if (result === null) { return ""; } - const value = Reflect.get(error, key); + const value = result[key]; return typeof value === "string" ? value : String(value || ""); } @@ -58,11 +66,11 @@ export interface DeployExecutionOptions { rootDir: string; getCredential: (key: string) => string | null; validateName: (value: string, label: string) => string; - shellQuote: (value: string) => string; - run: (command: string, opts?: { ignoreError?: boolean }) => void; - runInteractive: (command: string) => void; - execFileSync: (file: string, args: string[], opts?: ExecLikeOptions) => string; - spawnSync: (file: string, args: string[], opts?: ExecLikeOptions) => void; + run: ( + command: readonly string[], + opts?: ExecLikeOptions & { ignoreError?: boolean; suppressOutput?: boolean }, + ) => ExecResultLike; + runInteractive: (command: readonly string[]) => void; log: (message?: string) => void; error: (message?: string) => void; stdoutWrite: (message: string) => void; @@ -71,14 +79,14 @@ export interface DeployExecutionOptions { // SSH host key verification helper — resolves the real hostname from SSH config // (brev aliases aren't DNS-resolvable) and returns it for ssh-keyscan. -export function resolveRealHost( - name: string, - execFileSync: DeployExecutionOptions["execFileSync"], -): string { - const sshConfigOut = execFileSync("ssh", ["-G", name], { +export function resolveRealHost(name: string, run: DeployExecutionOptions["run"]): string { + const sshConfigResult = run(["ssh", "-G", name], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], + ignoreError: true, + suppressOutput: true, }); + const sshConfigOut = readCommandOutput(sshConfigResult, "stdout"); return ( sshConfigOut .split("\n") @@ -88,8 +96,8 @@ export function resolveRealHost( } // Build SSH options that enforce strict host key checking against a pinned known_hosts file. -export function buildSshOpts(knownHostsFile: string, shellQuote: (v: string) => string): string { - return `-o UserKnownHostsFile=${shellQuote(knownHostsFile)} -o StrictHostKeyChecking=yes -o LogLevel=ERROR`; +export function buildSshOpts(knownHostsFile: string): string { + return `-o UserKnownHostsFile=${formatShellToken(knownHostsFile)} -o StrictHostKeyChecking=yes -o LogLevel=ERROR`; } // Build SSH argument array for execFileSync calls with pinned host key verification. @@ -131,14 +139,13 @@ export function buildDeployEnvLines(opts: { sandboxName: string; provider: string; credentials: DeployCredentials; - shellQuote: (value: string) => string; }): string[] { - const { env, sandboxName, provider, credentials, shellQuote } = opts; + const { env, sandboxName, provider, credentials } = opts; const envLines = [ "NEMOCLAW_NON_INTERACTIVE=1", "NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1", - `NEMOCLAW_SANDBOX_NAME=${shellQuote(sandboxName)}`, - `NEMOCLAW_PROVIDER=${shellQuote(provider)}`, + buildShellAssignment("NEMOCLAW_SANDBOX_NAME", sandboxName), + buildShellAssignment("NEMOCLAW_PROVIDER", provider), ]; const passthroughVars = [ @@ -150,16 +157,16 @@ export function buildDeployEnvLines(opts: { ]; for (const key of passthroughVars) { const value = env[key]; - if (value) envLines.push(`${key}=${shellQuote(value)}`); + if (value) envLines.push(buildShellAssignment(key, value)); } for (const [key, value] of Object.entries(credentials)) { if (!value || key === "ALLOWED_CHAT_IDS") continue; - envLines.push(`${key}=${shellQuote(value)}`); + envLines.push(buildShellAssignment(key, value)); } if (credentials.TELEGRAM_BOT_TOKEN && credentials.ALLOWED_CHAT_IDS) { - envLines.push(`ALLOWED_CHAT_IDS=${shellQuote(credentials.ALLOWED_CHAT_IDS)}`); + envLines.push(buildShellAssignment("ALLOWED_CHAT_IDS", credentials.ALLOWED_CHAT_IDS)); } return envLines; @@ -205,14 +212,17 @@ export function isBrevInstanceReady(status: BrevInstanceStatus | null): boolean function getBrevInstanceStatus( instanceName: string, - execFileSync: DeployExecutionOptions["execFileSync"], + run: DeployExecutionOptions["run"], ): BrevInstanceStatus | null { - try { - const raw = execFileSync("brev", ["ls", "--json"], { encoding: "utf-8" }); - return findBrevInstanceStatus(raw, instanceName); - } catch { + const result = run(["brev", "ls", "--json"], { + encoding: "utf-8", + ignoreError: true, + suppressOutput: true, + }); + if (result.status !== 0) { return null; } + return findBrevInstanceStatus(readCommandOutput(result, "stdout"), instanceName); } function fail( @@ -231,11 +241,8 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise rootDir, getCredential, validateName, - shellQuote, run, runInteractive, - execFileSync, - spawnSync, log, error, stdoutWrite, @@ -264,7 +271,6 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise } const name = validateName(instanceName, "instance name"); - const qname = shellQuote(name); const gpu = env.NEMOCLAW_GPU || "a2-highgpu-1g:nvidia-tesla-a100:1"; const brevProvider = String(env.NEMOCLAW_BREV_PROVIDER || "gcp") .trim() @@ -307,33 +313,51 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise log(""); try { - execFileSync("which", ["brev"], { stdio: "ignore" }); + const whichResult = run(["which", "brev"], { + ignoreError: true, + suppressOutput: true, + stdio: "ignore", + }); + if (whichResult.status !== 0) { + return fail(["brev CLI not found. Install: https://brev.nvidia.com"], error, exit); + } } catch { return fail(["brev CLI not found. Install: https://brev.nvidia.com"], error, exit); } let exists = false; - try { - const out = execFileSync("brev", ["ls"], { encoding: "utf-8" }); - exists = outputHasExactLine(out, name); - } catch (caught) { - const caughtObject = typeof caught === "object" && caught !== null ? caught : null; - if (outputHasExactLine(readCommandOutput(caughtObject, "stdout"), name)) exists = true; - if (outputHasExactLine(readCommandOutput(caughtObject, "stderr"), name)) exists = true; + const brevLsResult = run(["brev", "ls"], { + encoding: "utf-8", + ignoreError: true, + suppressOutput: true, + }); + if (brevLsResult.status !== 0) { + const detail = + readCommandOutput(brevLsResult, "stderr").trim() || + readCommandOutput(brevLsResult, "stdout").trim(); + const lines = [" Failed to query existing Brev instances."]; + if (detail) { + lines.push(` ${detail}`); + } + return fail(lines, error, exit); + } + exists = outputHasExactLine(readCommandOutput(brevLsResult, "stdout"), name); + if (!exists) { + exists = outputHasExactLine(readCommandOutput(brevLsResult, "stderr"), name); } if (!exists) { log(` Creating Brev instance '${name}' (${gpu}, provider=${brevProvider})...`); - run(`brev create ${qname} --type ${shellQuote(gpu)} --provider ${shellQuote(brevProvider)}`); + run(["brev", "create", name, "--type", gpu, "--provider", brevProvider]); } else { log(` Brev instance '${name}' already exists.`); } - run("brev refresh", { ignoreError: true }); + run(["brev", "refresh"], { ignoreError: true }); stdoutWrite(" Waiting for Brev instance readiness "); for (let i = 0; i < 60; i++) { - const brevStatus = getBrevInstanceStatus(name, execFileSync); + const brevStatus = getBrevInstanceStatus(name, run); if (isBrevInstanceFailed(brevStatus)) { stdoutWrite("\n"); error(` Brev instance '${name}' did not become ready.`); @@ -350,7 +374,7 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise if (i === 59) { stdoutWrite("\n"); - const finalBrevStatus = getBrevInstanceStatus(name, execFileSync); + const finalBrevStatus = getBrevInstanceStatus(name, run); if (finalBrevStatus) { error( ` Brev status at timeout: status=${finalBrevStatus.status || "unknown"} build=${finalBrevStatus.build_status || "unknown"} shell=${finalBrevStatus.shell_status || "unknown"}`, @@ -371,16 +395,19 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise // Ref: https://github.com/NVIDIA/NemoClaw/issues/691 const khDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-ssh-")); const knownHostsFile = path.join(khDir, "known_hosts"); - const realHost = resolveRealHost(name, execFileSync); + const realHost = resolveRealHost(name, run); stdoutWrite(" Waiting for SSH "); for (let i = 0; i < 60; i++) { try { - const hostKeys = execFileSync("ssh-keyscan", ["-T", "5", "-H", realHost], { + const hostKeysResult = run(["ssh-keyscan", "-T", "5", "-H", realHost], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], + ignoreError: true, + suppressOutput: true, }); - if (hostKeys.trim()) { + const hostKeys = readCommandOutput(hostKeysResult, "stdout"); + if (hostKeysResult.status === 0 && hostKeys.trim()) { fs.writeFileSync(knownHostsFile, hostKeys, { mode: 0o600 }); stdoutWrite(" ✓\n"); break; @@ -401,34 +428,69 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise sleepSeconds(3); } - const sshOpts = buildSshOpts(knownHostsFile, shellQuote); + const sshOpts = buildSshOpts(knownHostsFile); const sshArgs = buildSshArgs(knownHostsFile); try { - const remoteHome = execFileSync("ssh", [...sshArgs, name, "echo", "$HOME"], { + const remoteHomeResult = run(["ssh", ...sshArgs, name, "echo", "$HOME"], { encoding: "utf-8", - }).trim(); + ignoreError: true, + suppressOutput: true, + }); + if (remoteHomeResult.status !== 0) { + return fail([` Could not determine remote home for ${name}`], error, exit); + } + const remoteHome = readCommandOutput(remoteHomeResult, "stdout").trim(); + if (!remoteHome) { + return fail([` Could not determine remote home for ${name}`], error, exit); + } const remoteDir = `${remoteHome}/nemoclaw`; log(" Syncing NemoClaw to VM..."); - run(`ssh ${sshOpts} ${qname} 'mkdir -p ${shellQuote(remoteDir)}'`); run( - `rsync -az --delete --exclude node_modules --exclude .git --exclude dist --exclude .venv -e "ssh ${sshOpts}" "${rootDir}/" ${qname}:${shellQuote(`${remoteDir}/`)}`, + buildSshScriptCommand({ + sshArgs, + host: name, + commandArgs: ["mkdir", "-p", remoteDir], + }), ); + run([ + "rsync", + "-az", + "--delete", + "--exclude", + "node_modules", + "--exclude", + ".git", + "--exclude", + "dist", + "--exclude", + ".venv", + "-e", + `ssh ${sshOpts}`, + `${rootDir}/`, + `${name}:${remoteDir}/`, + ]); const envLines = buildDeployEnvLines({ env, sandboxName, provider, credentials, - shellQuote, }); const envDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-env-")); const envTmp = path.join(envDir, "env"); fs.writeFileSync(envTmp, envLines.join("\n") + "\n", { mode: 0o600 }); try { - run(`scp -q ${sshOpts} ${shellQuote(envTmp)} ${qname}:${shellQuote(`${remoteDir}/.env`)}`); - run(`ssh -q ${sshOpts} ${qname} 'chmod 600 ${shellQuote(`${remoteDir}/.env`)}'`); + run(["scp", "-q", ...sshArgs, envTmp, `${name}:${remoteDir}/.env`]); + run( + buildSshScriptCommand({ + sshArgs, + host: name, + commandArgs: ["chmod", "600", `${remoteDir}/.env`], + quiet: true, + }), + ); } finally { try { fs.unlinkSync(envTmp); @@ -444,7 +506,19 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise log(" Running setup..."); runInteractive( - `ssh -t ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/install.sh --non-interactive --yes-i-accept-third-party-software'`, + buildSshScriptCommand({ + sshArgs, + host: name, + cwd: remoteDir, + sourceEnv: true, + commandArgs: [ + "bash", + "scripts/install.sh", + "--non-interactive", + "--yes-i-accept-third-party-software", + ], + tty: true, + }), ); if ( @@ -455,7 +529,13 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise ) { log(" Starting services..."); run( - `ssh ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && bash scripts/start-services.sh'`, + buildSshScriptCommand({ + sshArgs, + host: name, + cwd: remoteDir, + sourceEnv: true, + commandArgs: ["bash", "scripts/start-services.sh"], + }), ); } @@ -475,7 +555,14 @@ export async function executeDeploy(opts: DeployExecutionOptions): Promise log(" Connecting to sandbox..."); log(""); runInteractive( - `ssh -t ${sshOpts} ${qname} 'cd ${shellQuote(remoteDir)} && set -a && . .env && set +a && openshell sandbox connect ${shellQuote(sandboxName)}'`, + buildSshScriptCommand({ + sshArgs, + host: name, + cwd: remoteDir, + sourceEnv: true, + commandArgs: ["openshell", "sandbox", "connect", sandboxName], + tty: true, + }), ); } finally { fs.rmSync(khDir, { recursive: true, force: true }); diff --git a/src/lib/find-executable.test.ts b/src/lib/find-executable.test.ts new file mode 100644 index 00000000000..bcdb39eb189 --- /dev/null +++ b/src/lib/find-executable.test.ts @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { findExecutable, hasExecutable } from "./find-executable"; + +describe("findExecutable", () => { + it("returns an absolute path for PATH hits", () => { + const expected = path.resolve("bin/openshell"); + const result = findExecutable("openshell", { + env: { PATH: `bin${path.delimiter}/usr/bin` }, + checkExecutable: (filePath) => filePath === expected, + }); + + expect(result).toBe(expected); + }); + + it("returns null when the command is not present", () => { + const result = findExecutable("openshell", { + env: { PATH: `/nope${path.delimiter}/still-nope` }, + checkExecutable: () => false, + }); + + expect(result).toBeNull(); + }); + + it("handles explicit paths without PATH lookup", () => { + const expected = path.resolve("./tools/openshell"); + const result = findExecutable("./tools/openshell", { + env: { PATH: "/usr/bin" }, + checkExecutable: (filePath) => filePath === expected, + }); + + expect(result).toBe(expected); + }); + + it("exposes boolean existence via hasExecutable", () => { + const expected = path.resolve("bin/cloudflared"); + expect( + hasExecutable("cloudflared", { + env: { PATH: `bin${path.delimiter}/usr/bin` }, + checkExecutable: (filePath) => filePath === expected, + }), + ).toBe(true); + }); +}); diff --git a/src/lib/find-executable.ts b/src/lib/find-executable.ts new file mode 100644 index 00000000000..9071fcef170 --- /dev/null +++ b/src/lib/find-executable.ts @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { accessSync, constants } from "node:fs"; +import path from "node:path"; + +export interface FindExecutableOptions { + env?: NodeJS.ProcessEnv; + checkExecutable?: (filePath: string) => boolean; +} + +function defaultCheckExecutable(filePath: string): boolean { + try { + accessSync(filePath, constants.X_OK); + return true; + } catch { + return false; + } +} + +function candidateNames(commandName: string, env: NodeJS.ProcessEnv): string[] { + if (process.platform !== "win32") { + return [commandName]; + } + + if (path.extname(commandName)) { + return [commandName]; + } + + const pathext = String(env.PATHEXT || ".COM;.EXE;.BAT;.CMD") + .split(";") + .map((entry) => entry.trim()) + .filter(Boolean); + + return [commandName, ...pathext.map((ext) => `${commandName}${ext.toLowerCase()}`)]; +} + +export function findExecutable( + commandName: string, + opts: FindExecutableOptions = {}, +): string | null { + if (!commandName || commandName.includes("\0")) { + return null; + } + + const env = opts.env ?? process.env; + const checkExecutable = opts.checkExecutable ?? defaultCheckExecutable; + + if (path.isAbsolute(commandName) || commandName.includes("/") || commandName.includes("\\")) { + const resolved = path.resolve(commandName); + return checkExecutable(resolved) ? resolved : null; + } + + const rawPath = env.PATH ?? ""; + const searchDirs = rawPath.split(path.delimiter).filter(Boolean); + for (const dir of searchDirs) { + for (const candidateName of candidateNames(commandName, env)) { + const candidatePath = path.resolve(dir, candidateName); + if (checkExecutable(candidatePath)) { + return candidatePath; + } + } + } + + return null; +} + +export function hasExecutable(commandName: string, opts: FindExecutableOptions = {}): boolean { + return findExecutable(commandName, opts) !== null; +} diff --git a/src/lib/gateway-volumes.ts b/src/lib/gateway-volumes.ts new file mode 100644 index 00000000000..50a2df4729c --- /dev/null +++ b/src/lib/gateway-volumes.ts @@ -0,0 +1,34 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Shared Docker volume discovery helpers for the OpenShell gateway. + * + * `docker volume ls --filter name=...` performs substring matching, so callers + * must still filter by the exact expected prefix after parsing the output. + */ + +export type GatewayVolumeCapture = ( + command: readonly string[], + opts?: { ignoreError?: boolean }, +) => string | null | undefined; + +export function getGatewayDockerVolumePrefix(gatewayName: string): string { + return `openshell-cluster-${gatewayName}`; +} + +export function listGatewayDockerVolumes( + gatewayName: string, + runCaptureImpl: GatewayVolumeCapture, +): string[] { + const prefix = getGatewayDockerVolumePrefix(gatewayName); + return String( + runCaptureImpl( + ["docker", "volume", "ls", "-q", "--filter", `name=${prefix}`], + { ignoreError: true }, + ) || "", + ) + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line.startsWith(prefix)); +} diff --git a/src/lib/http-probe.test.ts b/src/lib/http-probe.test.ts index da34b6690a0..e8d87eb26df 100644 --- a/src/lib/http-probe.test.ts +++ b/src/lib/http-probe.test.ts @@ -65,6 +65,91 @@ describe("http-probe helpers", () => { expect(fs.existsSync(path.dirname(outputPath))).toBe(false); }); + it("scrubs unrelated process env by default", () => { + const originalPath = process.env.PATH; + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + const originalAllProxy = process.env.ALL_PROXY; + const originalLowerAllProxy = process.env.all_proxy; + let seenEnv: NodeJS.ProcessEnv | undefined; + + try { + process.env.PATH = "/usr/local/bin:/usr/bin"; + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + process.env.ALL_PROXY = "socks5://proxy.example:1080"; + process.env.all_proxy = "http://proxy.example:8080"; + runCurlProbe(["-sS", "https://example.test/models"], { + spawnSyncImpl: (_command, _args, options) => { + seenEnv = options.env; + return { + pid: 1, + output: [], + stdout: "200", + stderr: "", + status: 0, + signal: null, + }; + }, + }); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + if (originalAllProxy === undefined) { + delete process.env.ALL_PROXY; + } else { + process.env.ALL_PROXY = originalAllProxy; + } + if (originalLowerAllProxy === undefined) { + delete process.env.all_proxy; + } else { + process.env.all_proxy = originalLowerAllProxy; + } + } + + expect(seenEnv?.PATH).toBe("/usr/local/bin:/usr/bin"); + expect(seenEnv?.ALL_PROXY).toBe("socks5://proxy.example:1080"); + expect(seenEnv?.all_proxy).toBe("http://proxy.example:8080"); + expect(seenEnv?.AWS_SECRET_ACCESS_KEY).toBeUndefined(); + }); + + it("can opt into full parent env inheritance", () => { + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + let seenEnv: NodeJS.ProcessEnv | undefined; + + try { + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + runCurlProbe(["-sS", "https://example.test/models"], { + inheritFullEnv: true, + spawnSyncImpl: (_command, _args, options) => { + seenEnv = options.env; + return { + pid: 1, + output: [], + stdout: "200", + stderr: "", + status: 0, + signal: null, + }; + }, + }); + } finally { + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + } + + expect(seenEnv?.AWS_SECRET_ACCESS_KEY).toBe("secret-from-parent-env"); + }); + it("reports spawn errors as curl failures", () => { const result = runCurlProbe(["-sS", "https://example.test/models"], { spawnSyncImpl: () => { diff --git a/src/lib/http-probe.ts b/src/lib/http-probe.ts index 731bf4ab42d..848d6c69e3a 100644 --- a/src/lib/http-probe.ts +++ b/src/lib/http-probe.ts @@ -12,6 +12,7 @@ import { import type { ProbeResult } from "./onboard-types"; import { ROOT } from "./paths"; +import { buildEnvForSubprocess } from "./subprocess-env"; import { compactText } from "./url-utils"; import { isErrnoException } from "./errno"; @@ -21,6 +22,7 @@ export type CurlProbeResult = ProbeResult; export interface CurlProbeOptions { cwd?: string; env?: NodeJS.ProcessEnv; + inheritFullEnv?: boolean; spawnSyncImpl?: ( command: string, args: readonly string[], @@ -72,6 +74,13 @@ type ProbeErrorBody = { details?: ProbeErrorDetail; }; +function buildProbeEnv( + extraEnv: NodeJS.ProcessEnv | undefined, + inheritFullEnv = false, +): NodeJS.ProcessEnv { + return buildEnvForSubprocess(extraEnv, inheritFullEnv); +} + function formatProbeErrorDetail(detail: ProbeErrorDetail): string { if (typeof detail === "string") { return detail; @@ -125,10 +134,7 @@ export function runCurlProbe(argv: string[], opts: CurlProbeOptions = {}): CurlP cwd: opts.cwd ?? ROOT, encoding: "utf8", timeout: 30_000, - env: { - ...process.env, - ...opts.env, - }, + env: buildProbeEnv(opts.env, opts.inheritFullEnv), }, ); const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : ""; @@ -212,10 +218,7 @@ export function runStreamingEventProbe( cwd: opts.cwd ?? ROOT, encoding: "utf8", timeout: 30_000, - env: { - ...process.env, - ...opts.env, - }, + env: buildProbeEnv(opts.env, opts.inheritFullEnv), }); const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : ""; diff --git a/src/lib/local-inference.test.ts b/src/lib/local-inference.test.ts index 77136f99529..b0c01673a26 100644 --- a/src/lib/local-inference.test.ts +++ b/src/lib/local-inference.test.ts @@ -23,6 +23,7 @@ import { getOllamaProbeCommand, getOllamaWarmupCommand, parseOllamaList, + startOllamaWarmup, parseOllamaTags, probeLocalProviderHealth, validateOllamaModel, @@ -310,17 +311,19 @@ describe("local inference helpers", () => { expect(getDefaultOllamaModel({ totalMemoryMB: 16384 }, () => "")).toBe("qwen2.5:7b"); }); - it("builds a background warmup command for ollama models", () => { + it("builds a direct curl warmup command for ollama models", () => { const command = getOllamaWarmupCommand("nemotron-3-nano:30b"); - expect(command).toEqual(expect.arrayContaining(["bash", "-c"])); - expect(command[2]).toMatch(/^nohup curl -s http:\/\/127.0.0.1:11434\/api\/generate /); - expect(command[2]).toMatch(/"model":"nemotron-3-nano:30b"/); - expect(command[2]).toMatch(/"keep_alive":"15m"/); + expect(command[0]).toBe("curl"); + expect(command).toContain("-s"); + expect(command).toContain("http://127.0.0.1:11434/api/generate"); + const payload = command[command.length - 1]; + expect(payload).toMatch(/"model":"nemotron-3-nano:30b"/); + expect(payload).toMatch(/"keep_alive":"15m"/); }); it("supports custom probe and warmup tuning", () => { const warmup = getOllamaWarmupCommand("qwen2.5:7b", "30m"); - expect(warmup[2]).toMatch(/"keep_alive":"30m"/); + expect(warmup[warmup.length - 1]).toMatch(/"keep_alive":"30m"/); const probe1 = getOllamaProbeCommand("qwen2.5:7b", 30, "5m"); expect(probe1).toContain("--max-time"); expect(probe1).toContain("30"); @@ -328,6 +331,20 @@ describe("local inference helpers", () => { expect(payload1).toMatch(/"keep_alive":"5m"/); }); + it("launches Ollama warmup via the detached runner", () => { + const calls: Array<{ file: string; args: readonly string[] }> = []; + const pid = startOllamaWarmup("qwen2.5:7b", "30m", (file, args) => { + calls.push({ file, args }); + return 4242; + }); + + expect(pid).toBe(4242); + expect(calls).toHaveLength(1); + expect(calls[0]?.file).toBe("curl"); + expect(calls[0]?.args).toContain("http://127.0.0.1:11434/api/generate"); + expect(calls[0]?.args[calls[0].args.length - 1]).toMatch(/"keep_alive":"30m"/); + }); + it("builds a foreground probe command as an argv array", () => { const command = getOllamaProbeCommand("nemotron-3-nano:30b"); expect(command[0]).toBe("curl"); diff --git a/src/lib/local-inference.ts b/src/lib/local-inference.ts index e0465a94969..7ee9e736ee3 100644 --- a/src/lib/local-inference.ts +++ b/src/lib/local-inference.ts @@ -10,7 +10,7 @@ import type { CurlProbeResult } from "./http-probe"; import { runCurlProbe } from "./http-probe"; // eslint-disable-next-line @typescript-eslint/no-require-imports -const { shellQuote, runCapture } = require("./runner"); +const { runCapture, runDetachedFile } = require("./runner"); import { VLLM_PORT, OLLAMA_PORT, OLLAMA_PROXY_PORT } from "./ports"; @@ -27,6 +27,11 @@ export const SMALL_OLLAMA_MODEL = "qwen2.5:7b"; export const LARGE_OLLAMA_MIN_MEMORY_MB = 32768; export type RunCaptureFn = (cmd: string | string[], opts?: { ignoreError?: boolean }) => string; +export type RunDetachedFn = ( + file: string, + args: readonly string[], + opts?: { stdio?: import("node:child_process").StdioOptions }, +) => number | null; export interface GpuInfo { totalMemoryMB: number; @@ -324,17 +329,27 @@ export function getOllamaWarmupCommand(model: string, keepAlive = "15m"): string stream: false, keep_alive: keepAlive, }); - // backgrounding (nohup ... &) and output redirection require a shell wrapper. - // The payload is safe: model name is JSON-serialized (escaping all special - // chars) then shellQuote'd (single-quoted), so injection through model - // names is not feasible. This is the one intentional bash -c exception. return [ - "bash", - "-c", - `nohup curl -s http://127.0.0.1:${OLLAMA_PORT}/api/generate -H 'Content-Type: application/json' -d ${shellQuote(payload)} >/dev/null 2>&1 &`, + "curl", + "-s", + `http://127.0.0.1:${OLLAMA_PORT}/api/generate`, + "-H", + "Content-Type: application/json", + "-d", + payload, ]; } +export function startOllamaWarmup( + model: string, + keepAlive = "15m", + runDetachedImpl?: RunDetachedFn, +): number | null { + const command = getOllamaWarmupCommand(model, keepAlive); + const runDetached = runDetachedImpl ?? runDetachedFile; + return runDetached(command[0], command.slice(1)); +} + export function getOllamaProbeCommand( model: string, timeoutSeconds = 120, diff --git a/src/lib/nim.ts b/src/lib/nim.ts index 1796b49fff7..732cbd397b1 100644 --- a/src/lib/nim.ts +++ b/src/lib/nim.ts @@ -203,18 +203,19 @@ export function isNgcLoggedIn(): boolean { // NGC expects literal "$oauthtoken" as the username for API key authentication. export function dockerLoginNgc(apiKey: string): boolean { - const { spawnSync } = require("child_process"); - const result = spawnSync("docker", ["login", "nvcr.io", "-u", "$oauthtoken", "--password-stdin"], { + const result = run(["docker", "login", "nvcr.io", "-u", "$oauthtoken", "--password-stdin"], { input: apiKey, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, }); if (result.error) { console.error(` Docker error: ${result.error.message}`); return false; } if (result.status !== 0 && result.stderr) { - console.error(` Docker login error: ${result.stderr.trim()}`); + console.error(` Docker login error: ${String(result.stderr).trim()}`); } return result.status === 0; } diff --git a/src/lib/onboard-ollama-proxy.ts b/src/lib/onboard-ollama-proxy.ts index 4f25761c334..cba9b1dc9b6 100644 --- a/src/lib/onboard-ollama-proxy.ts +++ b/src/lib/onboard-ollama-proxy.ts @@ -5,52 +5,47 @@ // Ollama auth-proxy lifecycle: token persistence, PID management, // proxy start/stop, model pull and validation. +const crypto = require("node:crypto"); const fs = require("fs"); const os = require("os"); const path = require("path"); -const { spawn, spawnSync } = require("child_process"); -const { ROOT, SCRIPTS, run, runCapture, shellQuote } = require("./runner"); +const { ROOT, SCRIPTS, run, runCapture, runDetachedFile, runFile } = require("./runner"); +const { spawnChild } = require("./process-primitives"); +const { buildEnvForSubprocess } = require("./subprocess-env"); const { OLLAMA_PORT, OLLAMA_PROXY_PORT } = require("./ports"); const { getDefaultOllamaModel, getBootstrapOllamaModelOptions, getOllamaModelOptions, - getOllamaWarmupCommand, + startOllamaWarmup, validateOllamaModel, } = require("./local-inference"); const { prompt } = require("./credentials"); const { promptManualModelId } = require("./model-prompts"); - -// ── State ──────────────────────────────────────────────────────── +const { sleepSeconds } = require("./wait"); const PROXY_STATE_DIR = path.join(os.homedir(), ".nemoclaw"); const PROXY_TOKEN_PATH = path.join(PROXY_STATE_DIR, "ollama-proxy-token"); const PROXY_PID_PATH = path.join(PROXY_STATE_DIR, "ollama-auth-proxy.pid"); +const OLLAMA_INSTALLER_DOWNLOAD_TIMEOUT_MS = 130_000; +const OLLAMA_INSTALLER_RUN_TIMEOUT_MS = 600_000; -let ollamaProxyToken: string | null = null; - -function sleep(seconds) { - spawnSync("sleep", [String(seconds)]); -} - -// ── Proxy state dir ────────────────────────────────────────────── +let ollamaProxyToken = null; -function ensureProxyStateDir(): void { +function ensureProxyStateDir() { if (!fs.existsSync(PROXY_STATE_DIR)) { fs.mkdirSync(PROXY_STATE_DIR, { recursive: true }); } } -// ── Token persistence ──────────────────────────────────────────── - -function persistProxyToken(token: string): void { +function persistProxyToken(token) { ensureProxyStateDir(); fs.writeFileSync(PROXY_TOKEN_PATH, token, { mode: 0o600 }); // mode only applies on creation; ensure permissions on existing files too fs.chmodSync(PROXY_TOKEN_PATH, 0o600); } -function loadPersistedProxyToken(): string | null { +function loadPersistedProxyToken() { try { if (fs.existsSync(PROXY_TOKEN_PATH)) { const token = fs.readFileSync(PROXY_TOKEN_PATH, "utf-8").trim(); @@ -62,16 +57,15 @@ function loadPersistedProxyToken(): string | null { return null; } -// ── PID persistence ────────────────────────────────────────────── - -function persistProxyPid(pid: number | null | undefined): void { - if (!Number.isInteger(pid) || pid <= 0) return; +function persistProxyPid(pid) { + const validPid = typeof pid === "number" && Number.isInteger(pid) && pid > 0 ? pid : null; + if (validPid === null) return; ensureProxyStateDir(); - fs.writeFileSync(PROXY_PID_PATH, `${pid}\n`, { mode: 0o600 }); + fs.writeFileSync(PROXY_PID_PATH, `${validPid}\n`, { mode: 0o600 }); fs.chmodSync(PROXY_PID_PATH, 0o600); } -function loadPersistedProxyPid(): number | null { +function loadPersistedProxyPid() { try { if (!fs.existsSync(PROXY_PID_PATH)) return null; const raw = fs.readFileSync(PROXY_PID_PATH, "utf-8").trim(); @@ -82,7 +76,7 @@ function loadPersistedProxyPid(): number | null { } } -function clearPersistedProxyPid(): void { +function clearPersistedProxyPid() { try { if (fs.existsSync(PROXY_PID_PATH)) { fs.unlinkSync(PROXY_PID_PATH); @@ -92,31 +86,131 @@ function clearPersistedProxyPid(): void { } } -// ── Process management ─────────────────────────────────────────── +function collectOllamaEnv(extra = {}) { + const env = {}; + for (const [key, value] of Object.entries(process.env)) { + if (key.startsWith("OLLAMA_") && value !== undefined) { + env[key] = value; + } + } + return { ...env, ...extra }; +} -function isOllamaProxyProcess(pid: number | null | undefined): boolean { - if (!Number.isInteger(pid) || pid <= 0) return false; - const cmdline = runCapture(["ps", "-p", String(pid), "-o", "args="], { ignoreError: true }); +function isOllamaProxyProcess(pid) { + const validPid = typeof pid === "number" && Number.isInteger(pid) && pid > 0 ? pid : null; + if (validPid === null) return false; + const cmdline = runCapture(["ps", "-p", String(validPid), "-o", "args="], { + ignoreError: true, + }); return Boolean(cmdline && cmdline.includes("ollama-auth-proxy.js")); } -function spawnOllamaAuthProxy(token: string): number | null { - const child = spawn(process.execPath, [path.join(SCRIPTS, "ollama-auth-proxy.js")], { +function spawnDetachedProcess(command, args, opts = {}) { + const child = spawnChild(command, args, { detached: true, stdio: "ignore", + cwd: opts.cwd ?? ROOT, + env: buildEnvForSubprocess(opts.env), + }); + child.on?.("error", () => {}); + child.unref?.(); + return child.pid ?? null; +} + +function spawnOllamaAuthProxy(token) { + const pid = spawnDetachedProcess(process.execPath, [path.join(SCRIPTS, "ollama-auth-proxy.js")], { env: { - ...process.env, OLLAMA_PROXY_TOKEN: token, OLLAMA_PROXY_PORT: String(OLLAMA_PROXY_PORT), OLLAMA_BACKEND_PORT: String(OLLAMA_PORT), }, }); - child.unref(); - persistProxyPid(child.pid); - return child.pid ?? null; + persistProxyPid(pid); + return pid; +} + +function getOllamaClientHost() { + return `127.0.0.1:${OLLAMA_PORT}`; +} + +function getOllamaServeHostBinding(exposeToDocker) { + return `${exposeToDocker ? "0.0.0.0" : "127.0.0.1"}:${OLLAMA_PORT}`; +} + +function startDetachedOllamaServe(hostBinding) { + spawnDetachedProcess("ollama", ["serve"], { + env: collectOllamaEnv({ OLLAMA_HOST: hostBinding }), + }); } -function killStaleProxy(): void { +function startDetachedOllamaWarmup(model) { + return startOllamaWarmup(model, "15m", (file, args) => + runDetachedFile(file, [...args], { + env: collectOllamaEnv({ OLLAMA_HOST: getOllamaClientHost() }), + }), + ); +} + +function installOllamaViaOfficialScript() { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-ollama-install-")); + const installerPath = path.join(tempDir, "install.sh"); + try { + const download = run( + [ + "curl", + "-fsSL", + "--connect-timeout", + "20", + "--max-time", + "120", + "-o", + installerPath, + "https://ollama.com/install.sh", + ], + { + ignoreError: true, + timeout: OLLAMA_INSTALLER_DOWNLOAD_TIMEOUT_MS, + }, + ); + if (download.error) { + throw new Error(`Failed to download Ollama installer: ${download.error.message}`); + } + if (download.status !== 0) { + const detail = String(download.stderr || "").trim(); + if (download.status === 28 || download.signal === "SIGTERM") { + throw new Error("Timed out while downloading Ollama installer."); + } + throw new Error( + detail + ? `Failed to download Ollama installer: ${detail}` + : `Failed to download Ollama installer (exit ${download.status ?? 1})`, + ); + } + + const install = run(["sh", installerPath], { + ignoreError: true, + timeout: OLLAMA_INSTALLER_RUN_TIMEOUT_MS, + }); + if (install.error) { + throw new Error(`Failed to run Ollama installer: ${install.error.message}`); + } + if (install.status !== 0) { + const detail = String(install.stderr || "").trim(); + if (install.signal === "SIGTERM") { + throw new Error("Timed out while running Ollama installer."); + } + throw new Error( + detail + ? `Ollama installer failed: ${detail}` + : `Ollama installer failed (exit ${install.status ?? 1})`, + ); + } + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +} + +function killStaleProxy() { try { const persistedPid = loadPersistedProxyPid(); if (isOllamaProxyProcess(persistedPid)) { @@ -134,17 +228,14 @@ function killStaleProxy(): void { run(["kill", pid], { ignoreError: true, suppressOutput: true }); } } - sleep(1); + sleepSeconds(1); } } catch { /* ignore */ } } -// ── Public API ─────────────────────────────────────────────────── - -function startOllamaAuthProxy(): boolean { - const crypto = require("crypto"); +function startOllamaAuthProxy() { killStaleProxy(); const proxyToken = crypto.randomBytes(24).toString("hex"); @@ -153,7 +244,7 @@ function startOllamaAuthProxy(): boolean { // If the user backs out to a different provider, the token stays in memory // only and is discarded. const pid = spawnOllamaAuthProxy(proxyToken); - sleep(1); + sleepSeconds(1); if (!isOllamaProxyProcess(pid)) { console.error(` Error: Ollama auth proxy failed to start on :${OLLAMA_PROXY_PORT}`); console.error(` Containers will not be able to reach Ollama without the proxy.`); @@ -169,8 +260,7 @@ function startOllamaAuthProxy(): boolean { * Ensure the auth proxy is running — called on sandbox connect to recover * from host reboots where the background proxy process was lost. */ -function ensureOllamaAuthProxy(): void { - // Try to load persisted token first — if none, this isn't an Ollama setup. +function ensureOllamaAuthProxy() { const token = loadPersistedProxyToken(); if (!token) return; @@ -180,16 +270,14 @@ function ensureOllamaAuthProxy(): void { return; } - // Proxy not running — restart it with the persisted token. killStaleProxy(); ollamaProxyToken = token; spawnOllamaAuthProxy(token); - sleep(1); + sleepSeconds(1); } -function getOllamaProxyToken(): string | null { +function getOllamaProxyToken() { if (ollamaProxyToken) return ollamaProxyToken; - // Fall back to persisted token (resume / reconnect scenario) ollamaProxyToken = loadPersistedProxyToken(); return ollamaProxyToken; } @@ -230,16 +318,17 @@ function printOllamaExposureWarning() { } function pullOllamaModel(model) { - const result = spawnSync("bash", ["-c", `ollama pull ${shellQuote(model)}`], { + const result = runFile("ollama", ["pull", model], { cwd: ROOT, + env: collectOllamaEnv({ OLLAMA_HOST: getOllamaClientHost() }), encoding: "utf8", stdio: "inherit", timeout: 600_000, - env: { ...process.env }, + ignoreError: true, }); if (result.signal === "SIGTERM") { console.error( - ` Model pull timed out after 10 minutes. Try a smaller model or check your network connection.`, + " Model pull timed out after 10 minutes. Try a smaller model or check your network connection.", ); return false; } @@ -261,17 +350,21 @@ function prepareOllamaModel(model, installedModels = []) { } console.log(` Loading Ollama model: ${model}`); - run(getOllamaWarmupCommand(model), { ignoreError: true }); + startDetachedOllamaWarmup(model); return validateOllamaModel(model); } module.exports = { ensureOllamaAuthProxy, getOllamaProxyToken, + getOllamaServeHostBinding, + installOllamaViaOfficialScript, persistProxyToken, - startOllamaAuthProxy, - promptOllamaModel, + prepareOllamaModel, printOllamaExposureWarning, + promptOllamaModel, pullOllamaModel, - prepareOllamaModel, + startDetachedOllamaServe, + startDetachedOllamaWarmup, + startOllamaAuthProxy, }; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index fe4511fb65d..7bc03ac93fe 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -9,7 +9,6 @@ const crypto = require("node:crypto"); const fs = require("fs"); const os = require("os"); const path = require("path"); -const { spawn, spawnSync } = require("child_process"); const pRetry = require("p-retry"); /** Parse a numeric env var, returning `fallback` when unset or non-finite. */ @@ -26,7 +25,10 @@ const LOCAL_INFERENCE_TIMEOUT_SECS = envInt("NEMOCLAW_LOCAL_INFERENCE_TIMEOUT", * Covers CSI (color, erase, cursor), OSC, and C1 two-byte escapes per ECMA-48. */ const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g; const runner: typeof import("./runner") = require("./runner"); -const { ROOT, SCRIPTS, redact, run, runCapture, runFile, shellQuote, validateName } = runner; +const { ROOT, SCRIPTS, redact, run, runCapture, runFile, validateName } = runner; +const { spawnChild } = require("./process-primitives"); +const { buildDockerExecScriptCommand } = require("./remote-script"); +const { joinShellWords } = require("./shell-quote"); const errnoUtils: typeof import("./errno") = require("./errno"); const { isErrnoException } = errnoUtils; @@ -51,6 +53,7 @@ function requireValue(value: T | null | undefined, message: string): T { } const { stageOptimizedSandboxBuildContext } = require("./sandbox-build-context"); const { buildSubprocessEnv } = require("./subprocess-env"); +const { listGatewayDockerVolumes: listGatewayVolumes } = require("./gateway-volumes"); const { DASHBOARD_PORT, GATEWAY_PORT, @@ -61,11 +64,9 @@ const { const localInference: typeof import("./local-inference") = require("./local-inference"); const { getDefaultOllamaModel, - getBootstrapOllamaModelOptions, getLocalProviderBaseUrl, getLocalProviderValidationBaseUrl, getOllamaModelOptions, - getOllamaWarmupCommand, validateOllamaPortConfiguration, validateOllamaModel, validateLocalProvider, @@ -231,7 +232,7 @@ const BACK_TO_SELECTION = "__NEMOCLAW_BACK_TO_SELECTION__"; function verifyGatewayContainerRunning() { const containerName = `openshell-cluster-${GATEWAY_NAME}`; const result = run( - `docker inspect --type container --format '{{.State.Running}}' ${containerName}`, + ["docker", "inspect", "--type", "container", "--format", "{{.State.Running}}", containerName], { ignoreError: true, suppressOutput: true }, ); if (result.status === 0 && String(result.stdout || "").trim() === "true") { @@ -345,13 +346,34 @@ function repairRecordedSandbox(sandboxName: string | null): void { const { streamSandboxCreate } = sandboxCreateStream; /** Spawn `openshell gateway start` and stream its output with progress heartbeats. */ -function streamGatewayStart( +function spawnProcess( command: string, - env: NodeJS.ProcessEnv = process.env, + args: string[], + opts: { + cwd?: string; + env?: Record; + stdio?: import("node:child_process").StdioOptions; + detached?: boolean; + } = {}, +) { + return spawnChild(command, args, { + cwd: opts.cwd ?? ROOT, + env: buildSubprocessEnv(opts.env), + stdio: opts.stdio ?? ["ignore", "pipe", "pipe"], + detached: opts.detached ?? false, + }); +} + +function streamGatewayStart( + command: readonly string[], + extraEnv: Record = {}, ): Promise<{ status: number; output: string }> { - const child = spawn("bash", ["-lc", command], { - cwd: ROOT, - env, + if (command.length === 0) { + throw new Error("streamGatewayStart requires a non-empty argv array"); + } + + const child = spawnProcess(command[0], [...command.slice(1)], { + env: extraEnv, stdio: ["ignore", "pipe", "pipe"], }); @@ -636,7 +658,7 @@ function getOpenshellBinary(): string { function openshellShellCommand(args: string[], options: { openshellBinary?: string } = {}): string { const openshellBinary = options.openshellBinary || getOpenshellBinary(); - return [shellQuote(openshellBinary), ...args.map((arg) => shellQuote(arg))].join(" "); + return joinShellWords([openshellBinary, ...args]); } function openshellArgv(args: string[], options: { openshellBinary?: string } = {}): string[] { @@ -1597,7 +1619,21 @@ async function validateCustomAnthropicSelection( return { ok: false, retry }; } -const { promptManualModelId, promptCloudModel, promptRemoteModel, promptInputModel } = modelPrompts; +const { promptCloudModel, promptRemoteModel, promptInputModel } = modelPrompts; +const onboardOllamaProxy = require("./onboard-ollama-proxy"); +const { + ensureOllamaAuthProxy, + getOllamaProxyToken, + getOllamaServeHostBinding, + installOllamaViaOfficialScript, + persistProxyToken, + prepareOllamaModel, + printOllamaExposureWarning, + promptOllamaModel, + startDetachedOllamaServe, + startDetachedOllamaWarmup, + startOllamaAuthProxy, +} = onboardOllamaProxy; const { validateAnthropicModel, validateOpenAiLikeModel } = providerModels; // Build context helpers — delegated to src/lib/build-context.ts @@ -1605,19 +1641,6 @@ const { shouldIncludeBuildContextPath, copyBuildContextDir, printSandboxCreateRe buildContext; // classifySandboxCreateFailure — see validation import above -// --------------------------------------------------------------------------- -// Ollama auth proxy — moved to onboard-ollama-proxy.ts -const { - ensureOllamaAuthProxy, - getOllamaProxyToken, - persistProxyToken, - startOllamaAuthProxy, - promptOllamaModel, - printOllamaExposureWarning, - pullOllamaModel, - prepareOllamaModel, -} = require("./onboard-ollama-proxy"); - function getRequestedSandboxNameHint(): string | null { const raw = process.env.NEMOCLAW_SANDBOX_NAME; if (typeof raw !== "string") return null; @@ -1762,12 +1785,13 @@ function installOpenshell(): { localBin: string | null; futureShellPathHint: string | null; } { - const result = spawnSync("bash", [path.join(SCRIPTS, "install-openshell.sh")], { + const result = runFile("bash", [path.join(SCRIPTS, "install-openshell.sh")], { cwd: ROOT, - env: process.env, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8", timeout: 300_000, + ignoreError: true, + suppressOutput: true, }); if (result.status !== 0) { const output = `${result.stdout || ""}${result.stderr || ""}`.trim(); @@ -1796,6 +1820,17 @@ function sleep(seconds: number): void { sleepSeconds(seconds); } +function removeGatewayDockerVolumes(opts: { suppressOutput?: boolean } = {}): void { + const volumes = listGatewayVolumes(GATEWAY_NAME, runCapture); + if (volumes.length === 0) { + return; + } + run(["docker", "volume", "rm", ...volumes], { + ignoreError: true, + suppressOutput: opts.suppressOutput, + }); +} + function destroyGateway() { const destroyResult = runOpenshell(["gateway", "destroy", "-g", GATEWAY_NAME], { ignoreError: true, @@ -1805,18 +1840,22 @@ function destroyGateway() { registry.clearAll(); } // openshell gateway destroy doesn't remove Docker volumes, which leaves - // corrupted cluster state that breaks the next gateway start. Clean them up. - // Shell required: pipe (|), && chaining, || fallback. - run( - `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm || true`, - { ignoreError: true }, - ); + // corrupted cluster state that breaks the next gateway start. + removeGatewayDockerVolumes(); } function getGatewayClusterContainerState(): string { const containerName = getGatewayClusterContainerName(); const state = runCapture( - `docker inspect --type container --format '{{.State.Status}}{{if .State.Health}} {{.State.Health.Status}}{{end}}' ${shellQuote(containerName)} 2>/dev/null`, + [ + "docker", + "inspect", + "--type", + "container", + "--format", + "{{.State.Status}}{{if .State.Health}} {{.State.Health.Status}}{{end}}", + containerName, + ], { ignoreError: true }, ) .trim() @@ -1911,12 +1950,24 @@ fi function runGatewayClusterCapture(script: string, opts: RunnerOptions = {}) { const containerName = getGatewayClusterContainerName(); - return runCapture(`docker exec ${shellQuote(containerName)} sh -lc ${shellQuote(script)}`, opts); + return runCapture( + buildDockerExecScriptCommand({ + containerName, + command: script, + }), + opts, + ); } function runGatewayCluster(script: string, opts: RunnerOptions = {}) { const containerName = getGatewayClusterContainerName(); - return run(`docker exec ${shellQuote(containerName)} sh -lc ${shellQuote(script)}`, opts); + return run( + buildDockerExecScriptCommand({ + containerName, + command: script, + }), + opts, + ); } function listMissingGatewayBootstrapSecrets() { @@ -1926,7 +1977,7 @@ set -eu export KUBECONFIG=/etc/rancher/k3s/k3s.yaml kubectl get namespace openshell >/dev/null 2>&1 || exit 0 kubectl -n openshell get statefulset/openshell >/dev/null 2>&1 || exit 0 -for name in ${GATEWAY_BOOTSTRAP_SECRET_NAMES.map((name) => shellQuote(name)).join(" ")}; do +for name in ${joinShellWords(GATEWAY_BOOTSTRAP_SECRET_NAMES)}; do kubectl -n openshell get secret "$name" >/dev/null 2>&1 || printf '%s\\n' "$name" done `, @@ -2428,10 +2479,7 @@ async function preflight(): Promise> { suppressOutput: true, }); if (postInspectResult.status !== 0) { - run( - `docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${GATEWAY_NAME}" | xargs docker volume rm 2>/dev/null || true`, - { ignoreError: true, suppressOutput: true }, - ); + removeGatewayDockerVolumes({ suppressOutput: true }); registry.clearAll(); console.log(" ✓ Orphaned gateway container removed"); } else { @@ -2458,14 +2506,14 @@ async function preflight(): Promise> { // tunnels the user may have set up on the same port. (#1950) if (port === DASHBOARD_PORT && portCheck.process === "ssh" && portCheck.pid) { // Use `ps` to get the command line — works on Linux, macOS, and WSL. - const cmdline = runCapture(`ps -p ${portCheck.pid} -o args= 2>/dev/null`, { + const cmdline = runCapture(["ps", "-p", String(portCheck.pid), "-o", "args="], { ignoreError: true, }).trim(); if (cmdline.includes("openshell")) { console.log( ` Cleaning up orphaned SSH port-forward on port ${port} (PID ${portCheck.pid})...`, ); - run(`kill ${portCheck.pid} 2>/dev/null || true`, { ignoreError: true }); + run(["kill", String(portCheck.pid)], { ignoreError: true }); sleep(1); portCheck = await checkPortAvailable(port); if (portCheck.ok) { @@ -2597,8 +2645,11 @@ async function startGatewayWithOptions( // Clear stale SSH host keys from previous gateway (fixes #768) try { - const { execFileSync } = require("child_process"); - execFileSync("ssh-keygen", ["-R", `openshell-${GATEWAY_NAME}`], { stdio: "ignore" }); + runFile("ssh-keygen", ["-R", `openshell-${GATEWAY_NAME}`], { + stdio: "ignore", + ignoreError: true, + suppressOutput: true, + }); } catch { /* ssh-keygen -R may fail if entry doesn't exist — safe to ignore */ } @@ -2634,13 +2685,7 @@ async function startGatewayWithOptions( try { await pRetry( async () => { - const startResult = await streamGatewayStart( - openshellShellCommand(["gateway", "start", ...gwArgs]), - { - ...process.env, - ...gatewayEnv, - }, - ); + const startResult = await streamGatewayStart(openshellArgv(["gateway", "start", ...gwArgs]), gatewayEnv); if (startResult.status !== 0) { const lines = String(redact(startResult.output || "")) .split("\n") @@ -3650,7 +3695,8 @@ async function createSandbox( // from openshell because bash returns the status of the last pipeline // command (awk, always 0) unless pipefail is set. Removing the pipe // lets the real exit code flow through to run(). - const createCommand = `${openshellShellCommand([ + const createCommand = [ + getOpenshellBinary(), "sandbox", "create", ...createArgs, @@ -3658,7 +3704,7 @@ async function createSandbox( "env", ...envArgs, "nemoclaw-start", - ])} 2>&1`; + ]; const createResult = await streamSandboxCreate(createCommand, sandboxEnv, { readyCheck: () => { const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); @@ -3876,9 +3922,15 @@ async function setupNim(gpu: ReturnType): Promise<{ let credentialEnv: string | null = REMOTE_PROVIDER_CONFIG.build.credentialEnv; let preferredInferenceApi: string | null = null; - // Detect local inference options - // "command -v" is a shell builtin — must go through bash. - const hasOllama = !!runCapture("command -v ollama", { ignoreError: true }); + // Detect local inference options. + // Probe the real binary by exit status so stderr from a failed launch does + // not make Ollama look installed. + const hasOllama = + runFile("ollama", ["--version"], { + ignoreError: true, + stdio: "ignore", + suppressOutput: true, + }).status === 0; const ollamaRunning = !!runCapture(["curl", "-sf", `http://127.0.0.1:${OLLAMA_PORT}/api/tags`], { ignoreError: true, }); @@ -3896,7 +3948,7 @@ async function setupNim(gpu: ReturnType): Promise<{ options.push({ key: "anthropic", label: "Anthropic" }); options.push({ key: "anthropicCompatible", label: "Other Anthropic-compatible endpoint" }); options.push({ key: "gemini", label: "Google Gemini" }); - if (hasOllama || ollamaRunning) { + if ((hasOllama || ollamaRunning) && process.platform !== "win32") { options.push({ key: "ollama", label: @@ -4425,9 +4477,7 @@ async function setupNim(gpu: ReturnType): Promise<{ // On WSL2, binding to 0.0.0.0 creates a dual-stack socket that Docker // cannot reach via host-gateway. The default 127.0.0.1 binding works // because WSL2 relays IPv4-only sockets to the Windows host. - // Shell required: backgrounding (&), env var prefix, output redirection. - const ollamaEnv = isWsl() ? "" : `OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} `; - run(`${ollamaEnv}ollama serve > /dev/null 2>&1 &`, { ignoreError: true }); + startDetachedOllamaServe(getOllamaServeHostBinding(!isWsl())); sleep(2); if (!isWsl()) printOllamaExposureWarning(); } @@ -4503,25 +4553,30 @@ async function setupNim(gpu: ReturnType): Promise<{ break; } else if (selected.key === "install-ollama") { if (!checkOllamaPortsOrWarn()) continue selectionLoop; + const wsl = isWsl(); if (process.platform === "darwin") { console.log(" Installing Ollama via Homebrew..."); run(["brew", "install", "ollama"], { ignoreError: true }); } else { console.log(" Installing Ollama via official installer..."); - run("set -o pipefail; curl -fsSL https://ollama.com/install.sh | sh"); + installOllamaViaOfficialScript(); } console.log(" Starting Ollama..."); - // Shell required: backgrounding (&), env var prefix, output redirection. - run(`OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} ollama serve > /dev/null 2>&1 &`, { - ignoreError: true, - }); + startDetachedOllamaServe(getOllamaServeHostBinding(!wsl)); sleep(2); - if (!startOllamaAuthProxy()) { - process.exit(1); + if (!wsl) { + printOllamaExposureWarning(); + } + if (wsl) { + console.log(` ✓ Using Ollama on localhost:${OLLAMA_PORT}`); + } else { + if (!startOllamaAuthProxy()) { + process.exit(1); + } + console.log( + ` ✓ Using Ollama on localhost:${OLLAMA_PORT} (proxy on :${OLLAMA_PROXY_PORT})`, + ); } - console.log( - ` ✓ Using Ollama on localhost:${OLLAMA_PORT} (proxy on :${OLLAMA_PROXY_PORT})`, - ); provider = "ollama-local"; credentialEnv = "OPENAI_API_KEY"; endpointUrl = getLocalProviderBaseUrl(provider); @@ -4818,7 +4873,7 @@ async function setupInference( String(LOCAL_INFERENCE_TIMEOUT_SECS), ]); console.log(` Priming Ollama model: ${model}`); - run(getOllamaWarmupCommand(model), { ignoreError: true }); + startDetachedOllamaWarmup(model); const probe = validateOllamaModel(model); if (!probe.ok) { console.error(` ${probe.message}`); @@ -6085,10 +6140,45 @@ function findOpenclawJsonPath(dir: string): string | null { } /** - * Pull gateway.auth.token from the sandbox image via openshell sandbox download - * so onboard can print copy-paste Control UI URLs with #token= (same idea as nemoclaw-start.sh). + * Pull the gateway auth token from the running sandbox so onboard can print + * copy-paste Control UI URLs with #token= (same idea as nemoclaw-start.sh). + * Prefer the root-only runtime token when available, then fall back to the + * baked OpenClaw config for non-root or older images. */ function fetchGatewayAuthTokenFromSandbox(sandboxName: string): string | null { + // 1. Root mode: kubectl exec reads gateway:gateway 0400 runtime token. + try { + const k3sContainer = "openshell-cluster-nemoclaw"; + const result = runFile( + "docker", + [ + "exec", + k3sContainer, + "kubectl", + "exec", + "-n", + "openshell", + sandboxName, + "-c", + "agent", + "--", + "cat", + "/run/nemoclaw/gateway-token", + ], + { + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + ignoreError: true, + suppressOutput: true, + }, + ); + const token = String(result.stdout || "").trim(); + if (token.length > 0) return token; + } catch { + // kubectl exec not available or file absent — fall through + } + + // 2. Fallback: download openclaw.json and read gateway.auth.token. const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-token-")); try { const destDir = `${tmpDir}${path.sep}`; @@ -6208,7 +6298,7 @@ function getWslHostAddress( return null; } const runCaptureFn = options.runCapture || runCapture; - const output = runCaptureFn("hostname -I 2>/dev/null", { ignoreError: true }); + const output = runCaptureFn(["hostname", "-I"], { ignoreError: true }); const candidates = String(output || "") .trim() .split(/\s+/) @@ -6294,7 +6384,7 @@ function printDashboard( const token = fetchGatewayAuthTokenFromSandbox(sandboxName); const chatUiUrl = process.env.CHAT_UI_URL || `http://127.0.0.1:${CONTROL_UI_PORT}`; - const wslAddr = isWsl() ? (String(runCapture("hostname -I 2>/dev/null", { ignoreError: true }) || "").trim().split(/\s+/)[0] || null) : null; + const wslAddr = getWslHostAddress(); const chain = buildChain({ chatUiUrl, isWsl: isWsl(), wslHostAddress: wslAddr }); // Build access info inline — uses chain instead of re-deriving from env diff --git a/src/lib/openshell.test.ts b/src/lib/openshell.test.ts index 066c968be91..6fd5ab9b898 100644 --- a/src/lib/openshell.test.ts +++ b/src/lib/openshell.test.ts @@ -94,6 +94,69 @@ describe("openshell helpers", () => { expect(result.status).toBe(0); }); + it("scrubs unrelated process env by default", () => { + const originalPath = process.env.PATH; + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + let seenEnv: NodeJS.ProcessEnv | undefined; + + try { + process.env.PATH = "/usr/local/bin:/usr/bin"; + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + runOpenshellCommand("openshell", ["status"], { + spawnSyncImpl: (_command, _args, options) => { + seenEnv = options.env; + return makeSpawnResult({ + status: 0, + stdout: "ok\n", + stderr: "", + }); + }, + }); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + } + + expect(seenEnv?.PATH).toBe("/usr/local/bin:/usr/bin"); + expect(seenEnv?.AWS_SECRET_ACCESS_KEY).toBeUndefined(); + }); + + it("can opt into full parent env inheritance", () => { + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + let seenEnv: NodeJS.ProcessEnv | undefined; + + try { + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + runOpenshellCommand("openshell", ["status"], { + inheritFullEnv: true, + spawnSyncImpl: (_command, _args, options) => { + seenEnv = options.env; + return makeSpawnResult({ + status: 0, + stdout: "ok\n", + stderr: "", + }); + }, + }); + } finally { + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + } + + expect(seenEnv?.AWS_SECRET_ACCESS_KEY).toBe("secret-from-parent-env"); + }); + it("uses the injected exit handler on failure", () => { expect(() => runOpenshellCommand("openshell", ["status"], { diff --git a/src/lib/openshell.ts b/src/lib/openshell.ts index 82161a8b175..bead02d9a1e 100644 --- a/src/lib/openshell.ts +++ b/src/lib/openshell.ts @@ -8,6 +8,8 @@ import { type SpawnSyncReturns, } from "node:child_process"; +import { buildEnvForSubprocess } from "./subprocess-env"; + export type OpenshellSpawnSync = ( command: string, args: readonly string[], @@ -17,6 +19,7 @@ export type OpenshellSpawnSync = ( interface OpenshellSpawnOptions { cwd?: string; env?: NodeJS.ProcessEnv; + inheritFullEnv?: boolean; spawnSyncImpl?: OpenshellSpawnSync; errorLine?: (message: string) => void; exit?: (code: number) => never; @@ -65,6 +68,13 @@ export function versionGte(left = "0.0.0", right = "0.0.0"): boolean { return true; } +function buildOpenshellEnv( + extraEnv: NodeJS.ProcessEnv | undefined, + inheritFullEnv = false, +): NodeJS.ProcessEnv { + return buildEnvForSubprocess(extraEnv, inheritFullEnv); +} + function handleSpawnError( binary: string, args: string[], @@ -84,7 +94,7 @@ export function runOpenshellCommand( const spawnSyncImpl = opts.spawnSyncImpl ?? spawnSync; const result = spawnSyncImpl(binary, args, { cwd: opts.cwd, - env: { ...process.env, ...opts.env }, + env: buildOpenshellEnv(opts.env, opts.inheritFullEnv), encoding: "utf-8", stdio: opts.stdio ?? "inherit", }); @@ -108,7 +118,7 @@ export function captureOpenshellCommand( const spawnSyncImpl = opts.spawnSyncImpl ?? spawnSync; const result = spawnSyncImpl(binary, args, { cwd: opts.cwd, - env: { ...process.env, ...opts.env }, + env: buildOpenshellEnv(opts.env, opts.inheritFullEnv), encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], }); diff --git a/src/lib/preflight.test.ts b/src/lib/preflight.test.ts index 32a699db6bf..48299e10ed8 100644 --- a/src/lib/preflight.test.ts +++ b/src/lib/preflight.test.ts @@ -2,6 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; // Import through the compiled dist/ output (via the bin/lib shim) so // coverage is attributed to dist/lib/preflight.js, which is what the // ratchet measures. @@ -17,6 +20,10 @@ import { probeContainerDns, } from "../../dist/lib/preflight"; +function renderCommand(command: string | readonly string[]): string { + return typeof command === "string" ? command : command.join(" "); +} + function requireMemoryInfo(result: ReturnType) { expect(result).not.toBeNull(); if (!result) { @@ -314,11 +321,10 @@ describe("assessHost", () => { readFileImpl: () => '{"default-cgroupns-mode":"private"}', commandExistsImpl: (name: string) => name === "docker" || name === "apt-get" || name === "systemctl", - runCaptureImpl: (command: string) => { - if (command === "command -v apt-get") return "/usr/bin/apt-get"; - if (command === "command -v systemctl") return "/usr/bin/systemctl"; - if (command === "systemctl is-active docker") return "active"; - if (command === "systemctl is-enabled docker") return "enabled"; + runCaptureImpl: (command: string | readonly string[]) => { + const rendered = renderCommand(command); + if (rendered === "systemctl is-active docker") return "active"; + if (rendered === "systemctl is-enabled docker") return "enabled"; return ""; }, }); @@ -357,6 +363,25 @@ describe("assessHost", () => { expect(result.notes).toContain("Headless environment likely"); }); + it("uses AssessHostOpts.env when locating executables", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-preflight-path-")); + const fakeBin = path.join(tmpDir, "bin"); + fs.mkdirSync(fakeBin, { recursive: true }); + fs.writeFileSync(path.join(fakeBin, "docker"), "#!/usr/bin/env bash\nexit 0\n", { + mode: 0o755, + }); + + const result = assessHost({ + platform: "linux", + env: { PATH: fakeBin }, + dockerInfoOutput: "", + commandExistsImpl: undefined, + }); + + expect(result.dockerInstalled).toBe(true); + expect(result.nodeInstalled).toBe(false); + }); + // Docker 26+ on Linux defaults fresh installs to the containerd image store // with overlayfs snapshotter, breaking nested overlay mounts inside k3s. // See cluster-image-patch.ts for the auto-fix downstream of this signal. @@ -810,7 +835,7 @@ describe("probeContainerDns", () => { const captured: string[] = []; const result = probeContainerDns({ runCaptureImpl: (command) => { - captured.push(command); + captured.push(renderCommand(command)); return BUSYBOX_SUCCESS; }, }); @@ -826,7 +851,7 @@ describe("probeContainerDns", () => { probeContainerDns({ command: "echo OVERRIDDEN", runCaptureImpl: (command) => { - seen = command; + seen = renderCommand(command); return "Name:\tregistry.npmjs.org\nAddress: 1.2.3.4\n"; }, }); @@ -872,7 +897,7 @@ describe("probeContainerDns", () => { let captured = ""; probeContainerDns({ runCaptureImpl: (command) => { - captured = command; + captured = renderCommand(command); return BUSYBOX_SUCCESS; }, }); @@ -932,7 +957,7 @@ describe("getDockerBridgeGatewayIp", () => { it("uses the expected docker network inspect command shape", () => { let captured = ""; getDockerBridgeGatewayIp((cmd) => { - captured = cmd; + captured = renderCommand(cmd); return "172.17.0.1"; }); expect(captured).toContain("docker network inspect bridge"); diff --git a/src/lib/preflight.ts b/src/lib/preflight.ts index 690f6e3ec2e..526a3d0b104 100644 --- a/src/lib/preflight.ts +++ b/src/lib/preflight.ts @@ -15,10 +15,11 @@ import os from "node:os"; import path from "node:path"; import { DASHBOARD_PORT } from "./ports"; +import { hasExecutable } from "./find-executable"; // runner.ts still uses CommonJS-style exports — use require here. // eslint-disable-next-line @typescript-eslint/no-require-imports -const { runCapture } = require("./runner"); +const { run, runCapture, runCaptureShell } = require("./runner"); // ── Types ──────────────────────────────────────────────────────── @@ -115,6 +116,27 @@ export interface RemediationAction { blocking: boolean; } +type RunCaptureLike = ( + command: string | readonly string[], + options?: { ignoreError?: boolean; timeout?: number }, +) => string | null; + +function defaultRunCapture( + command: string | readonly string[], + options?: { ignoreError?: boolean; timeout?: number }, +): string { + if (Array.isArray(command)) { + return runCapture(command, { + ignoreError: options?.ignoreError ?? false, + timeout: options?.timeout, + }); + } + return runCaptureShell(command, { + ignoreError: options?.ignoreError ?? false, + timeout: options?.timeout, + }); +} + export interface AssessHostOpts { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; @@ -123,18 +145,18 @@ export interface AssessHostOpts { dockerInfoOutput?: string; dockerInfoError?: string; readFileImpl?: (filePath: string, encoding: BufferEncoding) => string; - runCaptureImpl?: (command: string, options?: { ignoreError?: boolean }) => string; - commandExistsImpl?: (commandName: string) => boolean; + runCaptureImpl?: RunCaptureLike; + commandExistsImpl?: (commandName: string, env?: NodeJS.ProcessEnv) => boolean; gpuProbeImpl?: () => boolean; } function commandExists( commandName: string, - runCaptureImpl: (command: string, options?: { ignoreError?: boolean }) => string, + commandExistsImpl?: (commandName: string, env?: NodeJS.ProcessEnv) => boolean, + env?: NodeJS.ProcessEnv, ): boolean { try { - const output = runCaptureImpl(`command -v ${commandName}`, { ignoreError: true }); - return Boolean(String(output || "").trim()); + return commandExistsImpl?.(commandName, env) ?? hasExecutable(commandName, { env }); } catch { return false; } @@ -226,22 +248,25 @@ function isHeadlessLikely(env: NodeJS.ProcessEnv): boolean { } function detectNvidiaGpu( - runCaptureImpl: (command: string, options?: { ignoreError?: boolean }) => string, + runCaptureImpl: RunCaptureLike, + commandExistsImpl?: (commandName: string, env?: NodeJS.ProcessEnv) => boolean, + env?: NodeJS.ProcessEnv, ): boolean { - if (!commandExists("nvidia-smi", runCaptureImpl)) { + if (!commandExists("nvidia-smi", commandExistsImpl, env)) { return false; } - return Boolean(String(runCaptureImpl("nvidia-smi -L", { ignoreError: true }) || "").trim()); + return Boolean(String(runCaptureImpl(["nvidia-smi", "-L"], { ignoreError: true }) || "").trim()); } function detectPackageManager( - runCaptureImpl: (command: string, options?: { ignoreError?: boolean }) => string, + commandExistsImpl?: (commandName: string, env?: NodeJS.ProcessEnv) => boolean, + env?: NodeJS.ProcessEnv, ): PackageManager { - if (commandExists("apt-get", runCaptureImpl)) return "apt"; - if (commandExists("dnf", runCaptureImpl)) return "dnf"; - if (commandExists("yum", runCaptureImpl)) return "yum"; - if (commandExists("brew", runCaptureImpl)) return "brew"; - if (commandExists("pacman", runCaptureImpl)) return "pacman"; + if (commandExists("apt-get", commandExistsImpl, env)) return "apt"; + if (commandExists("dnf", commandExistsImpl, env)) return "dnf"; + if (commandExists("yum", commandExistsImpl, env)) return "yum"; + if (commandExists("brew", commandExistsImpl, env)) return "brew"; + if (commandExists("pacman", commandExistsImpl, env)) return "pacman"; return "unknown"; } @@ -265,27 +290,24 @@ function parseSystemctlState(value = ""): boolean | null { export function assessHost(opts: AssessHostOpts = {}): HostAssessment { const platform = opts.platform ?? process.platform; const env = opts.env ?? process.env; - const runCaptureImpl = - opts.runCaptureImpl ?? - ((command: string, options?: { ignoreError?: boolean }) => - runCapture(command, { ignoreError: options?.ignoreError ?? false })); + const runCaptureImpl = opts.runCaptureImpl ?? defaultRunCapture; const readFileImpl = opts.readFileImpl ?? fs.readFileSync; - const dockerInstalled = - opts.commandExistsImpl?.("docker") ?? commandExists("docker", runCaptureImpl); - const nodeInstalled = opts.commandExistsImpl?.("node") ?? commandExists("node", runCaptureImpl); - const openshellInstalled = - opts.commandExistsImpl?.("openshell") ?? commandExists("openshell", runCaptureImpl); - const hasNvidiaGpu = opts.gpuProbeImpl?.() ?? detectNvidiaGpu(runCaptureImpl); - const packageManager = detectPackageManager(runCaptureImpl); - const systemctlAvailable = commandExists("systemctl", runCaptureImpl); + const dockerInstalled = commandExists("docker", opts.commandExistsImpl, env); + const nodeInstalled = commandExists("node", opts.commandExistsImpl, env); + const openshellInstalled = commandExists("openshell", opts.commandExistsImpl, env); + const hasNvidiaGpu = + opts.gpuProbeImpl?.() ?? detectNvidiaGpu(runCaptureImpl, opts.commandExistsImpl, env); + const packageManager = detectPackageManager(opts.commandExistsImpl, env); + const systemctlAvailable = commandExists("systemctl", opts.commandExistsImpl, env); let dockerInfoOutput = opts.dockerInfoOutput; let dockerReachable = false; let dockerRunning = false; if (dockerInstalled && dockerInfoOutput === undefined) { - dockerInfoOutput = runCaptureImpl("docker info --format '{{json .}}' 2>/dev/null", { - ignoreError: true, - }); + dockerInfoOutput = + runCaptureImpl(["docker", "info", "--format", "{{json .}}"], { + ignoreError: true, + }) ?? undefined; } if (dockerInstalled && String(dockerInfoOutput || "").trim()) { dockerReachable = true; @@ -302,12 +324,12 @@ export function assessHost(opts: AssessHostOpts = {}): HostAssessment { return ""; } })(); - let runtime = inferContainerRuntime(dockerInfoOutput); + let runtime = inferContainerRuntime(dockerInfoOutput ?? ""); if (dockerReachable && runtime === "unknown" && platform === "linux") { runtime = "docker"; } const dockerCgroupVersion = dockerReachable - ? parseDockerCgroupVersion(dockerInfoOutput) + ? parseDockerCgroupVersion(dockerInfoOutput ?? "") : "unknown"; const dockerStorageDriver = dockerReachable ? parseDockerStorageDriver(dockerInfoOutput) @@ -339,11 +361,15 @@ export function assessHost(opts: AssessHostOpts = {}): HostAssessment { const dockerDefaultCgroupnsMode = readDockerDefaultCgroupnsMode(readFileImpl); const dockerServiceActive = platform === "linux" && systemctlAvailable && dockerInstalled - ? parseSystemctlState(runCaptureImpl("systemctl is-active docker", { ignoreError: true })) + ? parseSystemctlState( + runCaptureImpl(["systemctl", "is-active", "docker"], { ignoreError: true }) ?? "", + ) : null; const dockerServiceEnabled = platform === "linux" && systemctlAvailable && dockerInstalled - ? parseSystemctlState(runCaptureImpl("systemctl is-enabled docker", { ignoreError: true })) + ? parseSystemctlState( + runCaptureImpl(["systemctl", "is-enabled", "docker"], { ignoreError: true }) ?? "", + ) : null; const assessment: HostAssessment = { platform, @@ -573,8 +599,7 @@ export async function checkPortAvailable( if (typeof o.lsofOutput === "string") { lsofOut = o.lsofOutput; } else { - // "command -v" is a shell builtin — must go through bash. - const hasLsof = runCapture("command -v lsof", { ignoreError: true }); + const hasLsof = commandExists("lsof"); if (hasLsof) { lsofOut = runCapture(["lsof", "-i", `:${p}`, "-sTCP:LISTEN", "-P", "-n"], { ignoreError: true, @@ -713,11 +738,15 @@ function getExistingSwapResult(mem: MemoryInfo): SwapResult | null { function checkSwapDiskSpace(): SwapResult | null { try { - // Pipe requires a shell: df ... | tail -1 - const dfOut = runCapture("df / --output=avail -k 2>/dev/null | tail -1", { + const dfOut = runCapture(["df", "/", "--output=avail", "-k"], { ignoreError: true, }); - const freeKB = parseInt((dfOut || "").trim(), 10); + const freeLine = String(dfOut || "") + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean) + .at(-1); + const freeKB = parseInt(freeLine || "", 10); if (!isNaN(freeKB) && freeKB < 5000000) { return { ok: false, @@ -764,11 +793,20 @@ function createSwapfile(mem: MemoryInfo): SwapResult { runCapture(["sudo", "chmod", "600", "/swapfile"], { ignoreError: false }); runCapture(["sudo", "mkswap", "/swapfile"], { ignoreError: false }); runCapture(["sudo", "swapon", "/swapfile"], { ignoreError: false }); - // Shell required: grep || echo | tee pipeline - runCapture( - "grep -q '/swapfile' /etc/fstab || echo '/swapfile none swap sw 0 0' | sudo tee -a /etc/fstab", - { ignoreError: false }, + const fstabHasSwapfile = run( + ["sudo", "grep", "-Eq", "^[[:space:]]*/swapfile([[:space:]]|$)", "/etc/fstab"], + { + ignoreError: true, + suppressOutput: true, + }, ); + if (fstabHasSwapfile.status !== 0) { + run(["sudo", "tee", "-a", "/etc/fstab"], { + input: "/swapfile none swap sw 0 0\n", + ignoreError: false, + suppressOutput: true, + }); + } writeManagedSwapMarker(); return { ok: true, totalMB: mem.totalMB + 4096, swapCreated: true }; @@ -874,10 +912,7 @@ export interface ProbeContainerDnsOpts { /** Inject captured output (bypasses shell). */ outputOverride?: string | null; /** Override runCapture. */ - runCaptureImpl?: ( - command: string, - opts?: { ignoreError?: boolean; timeout?: number }, - ) => string | null; + runCaptureImpl?: RunCaptureLike; } /** @@ -896,10 +931,8 @@ const PROBE_TIMEOUT_MS = 20_000; * `172.17.0.1`. */ export function getDockerBridgeGatewayIp( - runCaptureImpl: (command: string, opts?: { ignoreError?: boolean }) => string | null = ( - cmd, - o, - ) => runCapture(cmd, { ignoreError: o?.ignoreError ?? false }), + runCaptureImpl: RunCaptureLike = (cmd, o) => + defaultRunCapture(cmd, { ignoreError: o?.ignoreError ?? false }), ): string | null { let raw: string | null; try { @@ -954,13 +987,7 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes let output: string | null | undefined = opts.outputOverride; if (output === undefined) { try { - const runCaptureImpl = - opts.runCaptureImpl ?? - ((cmd: string, o?: { ignoreError?: boolean; timeout?: number }) => - runCapture(cmd, { - ignoreError: o?.ignoreError ?? false, - timeout: o?.timeout, - })); + const runCaptureImpl = opts.runCaptureImpl ?? defaultRunCapture; output = runCaptureImpl(command, { ignoreError: true, timeout: PROBE_TIMEOUT_MS, diff --git a/src/lib/process-primitives.ts b/src/lib/process-primitives.ts new file mode 100644 index 00000000000..7ef39781b64 --- /dev/null +++ b/src/lib/process-primitives.ts @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + spawn, + spawnSync, + type SpawnOptions, + type SpawnSyncOptions, + type SpawnSyncOptionsWithStringEncoding, +} from "node:child_process"; + +export function spawnChild(command: string, args: string[], options: SpawnOptions) { + return spawn(command, args, options); +} + +export function spawnResult( + command: string, + args: string[], + options: SpawnSyncOptions | SpawnSyncOptionsWithStringEncoding = {}, +) { + return spawnSync(command, args, options); +} diff --git a/src/lib/remote-script.test.ts b/src/lib/remote-script.test.ts new file mode 100644 index 00000000000..0ad13f3bce1 --- /dev/null +++ b/src/lib/remote-script.test.ts @@ -0,0 +1,57 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { buildDockerExecScriptCommand, buildShellCommand } from "../../dist/lib/remote-script"; + +describe("buildShellCommand", () => { + it("supports multi-step shell scripts without mixing raw and argv command fields", () => { + expect( + buildShellCommand({ + steps: [ + { commandArgs: ["mkdir", "-p", "/tmp/demo"] }, + { commandArgs: ["cat"], stdoutRedirect: "/tmp/demo/file.txt" }, + ], + }), + ).toBe("mkdir -p /tmp/demo && cat > /tmp/demo/file.txt"); + }); + + it("rejects mixing steps with the legacy single-command API", () => { + expect(() => + buildShellCommand({ + steps: [{ commandArgs: ["echo", "hello"] }], + commandArgs: ["printf", "{}"], + }), + ).toThrow(/either steps or a single command definition/); + }); + + it("rejects stdout redirects on raw shell step strings", () => { + expect(() => + buildShellCommand({ + steps: [{ command: "echo hello", stdoutRedirect: "/tmp/out" }], + }), + ).toThrow(/cannot use stdoutRedirect/); + }); +}); + +describe("buildDockerExecScriptCommand", () => { + it("uses a login shell by default", () => { + expect( + buildDockerExecScriptCommand({ + containerName: "demo", + commandArgs: ["echo", "hello"], + }), + ).toEqual(["docker", "exec", "demo", "sh", "-lc", "echo hello"]); + }); + + it("supports plain shell execution when login is disabled", () => { + expect( + buildDockerExecScriptCommand({ + containerName: "demo", + commandArgs: ["echo", "hello"], + login: false, + }), + ).toEqual(["docker", "exec", "demo", "sh", "-c", "echo hello"]); + }); +}); diff --git a/src/lib/remote-script.ts b/src/lib/remote-script.ts new file mode 100644 index 00000000000..1497753738d --- /dev/null +++ b/src/lib/remote-script.ts @@ -0,0 +1,141 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { formatShellToken, joinShellWords } from "./shell-quote"; + +export interface ShellCommandStep { + command?: string; + commandArgs?: string[]; + stdoutRedirect?: string; +} + +function renderShellCommandStep(step: ShellCommandStep): string { + if (step.command && step.commandArgs && step.commandArgs.length > 0) { + throw new Error("shell steps accept either command or commandArgs, not both"); + } + if (step.command && step.stdoutRedirect) { + throw new Error("shell steps with raw command strings cannot use stdoutRedirect"); + } + if (step.commandArgs && step.commandArgs.length > 0) { + let command = joinShellWords(step.commandArgs); + if (step.stdoutRedirect) { + command += ` > ${formatShellToken(step.stdoutRedirect)}`; + } + return command; + } + if (step.command) { + return step.command; + } + throw new Error("shell steps require either command or commandArgs"); +} + +export function buildShellCommand(opts: { + command?: string; + commandArgs?: string[]; + stdoutRedirect?: string; + cwd?: string; + sourceEnv?: boolean; + steps?: ShellCommandStep[]; +}): string { + const hasLegacyCommand = Boolean(opts.command) || Boolean(opts.commandArgs?.length); + if (opts.steps && opts.steps.length > 0 && hasLegacyCommand) { + throw new Error("buildShellCommand accepts either steps or a single command definition, not both"); + } + + const steps: string[] = []; + if (opts.cwd) { + steps.push(`cd ${formatShellToken(opts.cwd)}`); + } + if (opts.sourceEnv) { + steps.push("set -a", ". .env", "set +a"); + } + if (opts.steps && opts.steps.length > 0) { + steps.push(...opts.steps.map((step) => renderShellCommandStep(step))); + } else if (hasLegacyCommand) { + steps.push( + renderShellCommandStep({ + command: opts.command, + commandArgs: opts.commandArgs, + stdoutRedirect: opts.stdoutRedirect, + }), + ); + } else { + throw new Error("buildShellCommand requires either steps or a single command definition"); + } + return steps.join(" && "); +} + +function buildSshScriptArgs( + sshArgs: string[], + host: string, + remoteCommand: string, + opts: { tty?: boolean; quiet?: boolean } = {}, +): string[] { + return [ + "ssh", + ...(opts.tty ? ["-t"] : []), + ...(opts.quiet ? ["-q"] : []), + ...sshArgs, + host, + remoteCommand, + ]; +} + +export function buildSshScriptCommand(opts: { + sshArgs: string[]; + host: string; + command?: string; + commandArgs?: string[]; + stdoutRedirect?: string; + cwd?: string; + sourceEnv?: boolean; + steps?: ShellCommandStep[]; + tty?: boolean; + quiet?: boolean; +}): string[] { + return buildSshScriptArgs( + opts.sshArgs, + opts.host, + buildShellCommand({ + command: opts.command, + commandArgs: opts.commandArgs, + stdoutRedirect: opts.stdoutRedirect, + cwd: opts.cwd, + sourceEnv: opts.sourceEnv, + steps: opts.steps, + }), + { tty: opts.tty, quiet: opts.quiet }, + ); +} + +function buildDockerExecScriptArgs( + containerName: string, + script: string, + login = true, +): string[] { + return ["docker", "exec", containerName, "sh", login ? "-lc" : "-c", script]; +} + +export function buildDockerExecScriptCommand(opts: { + containerName: string; + command?: string; + commandArgs?: string[]; + stdoutRedirect?: string; + cwd?: string; + sourceEnv?: boolean; + steps?: ShellCommandStep[]; + login?: boolean; +}): string[] { + return buildDockerExecScriptArgs( + opts.containerName, + buildShellCommand({ + command: opts.command, + commandArgs: opts.commandArgs, + stdoutRedirect: opts.stdoutRedirect, + cwd: opts.cwd, + sourceEnv: opts.sourceEnv, + steps: opts.steps, + }), + opts.login ?? true, + ); +} diff --git a/src/lib/resolve-openshell.ts b/src/lib/resolve-openshell.ts index b55fbfac84f..b75b1597f5d 100644 --- a/src/lib/resolve-openshell.ts +++ b/src/lib/resolve-openshell.ts @@ -1,9 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execSync } from "node:child_process"; import { accessSync, constants } from "node:fs"; +import { findExecutable } from "./find-executable"; + export interface ResolveOpenshellOptions { /** Mock result for `command -v` (undefined = run real command). */ commandVResult?: string | null; @@ -21,20 +22,6 @@ export interface ResolveOpenshellOptions { */ export function resolveOpenshell(opts: ResolveOpenshellOptions = {}): string | null { const home = opts.home ?? process.env.HOME; - - // Step 1: command -v - if (opts.commandVResult === undefined) { - try { - const found = execSync("command -v openshell", { encoding: "utf-8" }).trim(); - if (found.startsWith("/")) return found; - } catch { - /* ignored */ - } - } else if (opts.commandVResult?.startsWith("/")) { - return opts.commandVResult; - } - - // Step 2: fallback candidates const checkExecutable = opts.checkExecutable ?? ((p: string): boolean => { @@ -46,6 +33,15 @@ export function resolveOpenshell(opts: ResolveOpenshellOptions = {}): string | n } }); + // Step 1: resolve from PATH without shelling out + if (opts.commandVResult === undefined) { + const found = findExecutable("openshell", { checkExecutable }); + if (found?.startsWith("/")) return found; + } else if (opts.commandVResult?.startsWith("/")) { + return opts.commandVResult; + } + + // Step 2: fallback candidates const candidates = [ ...(home?.startsWith("/") ? [`${home}/.local/bin/openshell`] : []), "/usr/local/bin/openshell", diff --git a/src/lib/runner-argv.test.ts b/src/lib/runner-argv.test.ts index 38c9b5932f7..9f9949ba54e 100644 --- a/src/lib/runner-argv.test.ts +++ b/src/lib/runner-argv.test.ts @@ -1,6 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, it, expect } from "vitest"; import { createRequire } from "module"; @@ -49,11 +52,15 @@ describe("run with argv array", () => { expect(result).toContain("rm"); }); - it("still works with string commands (legacy path)", () => { - const result = runner.run("echo hello", { suppressOutput: true }); + it("uses runShell for explicit shell commands", () => { + const result = runner.runShell("echo hello", { suppressOutput: true }); expect(result.status).toBe(0); }); + it("rejects string commands on run()", () => { + expect(() => runner.run("echo hello")).toThrow(/Use runShell/); + }); + it("surfaces ENOENT error for missing executables", () => { const result = runner.run( ["nonexistent-binary-xyz-12345"], @@ -65,6 +72,30 @@ describe("run with argv array", () => { }); }); +describe("runDetachedFile with argv array", () => { + it("launches a detached child without using a shell", async () => { + const outputFile = path.join(os.tmpdir(), `runner-detached-${process.pid}-${Date.now()}.txt`); + const pid = runner.runDetachedFile(process.execPath, [ + "-e", + `require("fs").writeFileSync(${JSON.stringify(outputFile)}, "ok")`, + ]); + + expect(typeof pid).toBe("number"); + for (let attempt = 0; attempt < 50 && !fs.existsSync(outputFile); attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); + } + + expect(fs.readFileSync(outputFile, "utf-8")).toBe("ok"); + fs.rmSync(outputFile, { force: true }); + }); + + it("rejects shell: true on detached argv execution", () => { + expect(() => runner.runDetachedFile("echo", ["hello"], { shell: true })).toThrow( + /does not allow opts\.shell=true/, + ); + }); +}); + describe("runCapture with argv array", () => { it("captures stdout from a simple command", () => { const output = runner.runCapture(["echo", "hello world"]); @@ -128,11 +159,15 @@ describe("runCapture with argv array", () => { expect(output).toContain("TEST_ARGV_ENV=captured"); }); - it("still works with string commands (legacy path)", () => { - const output = runner.runCapture("echo hello"); + it("uses runCaptureShell for explicit shell commands", () => { + const output = runner.runCaptureShell("echo hello"); expect(output).toBe("hello"); }); + it("rejects string commands on runCapture()", () => { + expect(() => runner.runCapture("echo hello")).toThrow(/Use runCaptureShell/); + }); + it("throws ENOENT for missing executables", () => { expect(() => runner.runCapture(["nonexistent-binary-xyz-12345"])).toThrow(); }); diff --git a/src/lib/runner.ts b/src/lib/runner.ts index 2beb5385f82..306aeaeecd3 100644 --- a/src/lib/runner.ts +++ b/src/lib/runner.ts @@ -2,36 +2,43 @@ // SPDX-License-Identifier: Apache-2.0 import type { - ExecSyncOptionsWithStringEncoding, + SpawnOptions, SpawnSyncOptions, SpawnSyncOptionsWithStringEncoding, SpawnSyncReturns, } from "node:child_process"; - -const { execSync, spawnSync } = require("child_process"); const path = require("path"); -const { detectDockerHost } = require("./platform"); +const { detectDockerHost } = require("./platform.js"); +const { spawnChild, spawnResult } = require("./process-primitives.js"); +const { joinShellWords } = require("./shell-quote"); +const { buildEnvForSubprocess } = require("./subprocess-env.js"); const ROOT = path.resolve(__dirname, "..", ".."); const SCRIPTS = path.join(ROOT, "scripts"); -type RunnerScalar = string | number | boolean | null | undefined; - type RunnerOptions = SpawnSyncOptions & { ignoreError?: boolean; suppressOutput?: boolean; + inheritFullEnv?: boolean; }; -type CaptureOptions = Omit & { +type CaptureOptions = Omit & { ignoreError?: boolean; + inheritFullEnv?: boolean; }; type ArrayCaptureOptions = Omit & { ignoreError?: boolean; + inheritFullEnv?: boolean; }; type SpawnResult = SpawnSyncReturns; +type DetachedRunnerOptions = Omit & { + env?: NodeJS.ProcessEnv; + inheritFullEnv?: boolean; +}; + const dockerHost = detectDockerHost(); if (dockerHost) { process.env.DOCKER_HOST = dockerHost.dockerHost; @@ -59,11 +66,11 @@ function spawnAndHandle( stdio: RunnerOptions["stdio"], renderedCommand: string, ): SpawnResult { - const result = spawnSync(file, args, { + const result = spawnResult(file, args, { ...opts, stdio, - cwd: ROOT, - env: { ...process.env, ...opts.env }, + cwd: opts.cwd ?? ROOT, + env: buildEnvForSubprocess(opts.env, opts.inheritFullEnv), }); if (!opts.suppressOutput) { writeRedactedResult(result, stdio); @@ -88,16 +95,22 @@ function spawnAndHandle( * Run a command, streaming stdout/stderr (redacted) to the terminal. * Exits the process on failure unless opts.ignoreError is true. * - * Accepts two forms: - * run("bash -c string") — legacy: passes the string to bash for interpretation - * run(["docker", "rm", name]) — safe: calls spawnSync(exe, args) with no shell - * - * When an argv array is passed, the shell option is forbidden to prevent - * callers from accidentally re-enabling shell interpretation. + * Requires an argv array and never invokes a shell. */ -function run(cmd: string | readonly string[], opts: RunnerOptions = {}): SpawnResult { - if (Array.isArray(cmd)) { - return runArrayCmd(cmd, opts); +function run(cmd: readonly string[], opts: RunnerOptions = {}): SpawnResult { + if (!Array.isArray(cmd)) { + throw new Error("run requires an argv array. Use runShell for shell commands."); + } + return runArrayCmd(cmd, opts); +} + +/** + * Run an explicit shell command via `bash -c`. + * Exits the process on failure unless opts.ignoreError is true. + */ +function runShell(cmd: string, opts: RunnerOptions = {}): SpawnResult { + if (opts.shell) { + throw new Error("runShell does not allow opts.shell=true"); } const shellCmd = String(cmd); const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"]; @@ -115,7 +128,14 @@ function runArrayCmd(cmd: readonly string[], opts: RunnerOptions = {}): SpawnRes const exe = cmd[0]; const args = cmd.slice(1); - const { ignoreError, suppressOutput, env: extraEnv, stdio: stdioCfg, ...spawnOpts } = opts; + const { + ignoreError, + suppressOutput, + inheritFullEnv, + env: extraEnv, + stdio: stdioCfg, + ...spawnOpts + } = opts; // Guard: re-enabling shell interpretation defeats the purpose of argv arrays. if (spawnOpts.shell) { @@ -124,38 +144,67 @@ function runArrayCmd(cmd: readonly string[], opts: RunnerOptions = {}): SpawnRes const stdio = stdioCfg ?? ["ignore", "pipe", "pipe"]; - const result = spawnSync(exe, args, { - ...spawnOpts, + const cmdStr = joinShellWords(cmd); + return spawnAndHandle( + exe, + args, + { + ...spawnOpts, + ignoreError, + suppressOutput, + inheritFullEnv, + env: extraEnv, + }, stdio, - cwd: ROOT, - env: { ...process.env, ...extraEnv }, - }); - if (!suppressOutput) { - writeRedactedResult(result, stdio); - } - // Check result.error first — spawnSync sets this (with status === null) when - // the executable is missing (ENOENT), the call times out, or the spawn fails. - if (result.error && !ignoreError) { - const cmdStr = cmd.join(" "); - console.error(` Command failed: ${redact(cmdStr).slice(0, 80)}: ${result.error.message}`); - process.exit(1); - } - if (result.status !== 0 && !ignoreError) { - const cmdStr = cmd.join(" "); - console.error(` Command failed (exit ${result.status}): ${redact(cmdStr).slice(0, 80)}`); - logOpenshellRuntimeHint(exe); - process.exit(result.status || 1); + cmdStr, + ); +} + +/** + * Run an interactive argv command (stdin inherited) while capturing/redacting stdout/stderr. + * Exits the process on failure unless opts.ignoreError is true. + */ +function runInteractive(cmd: readonly string[], opts: RunnerOptions = {}): SpawnResult { + if (!Array.isArray(cmd)) { + throw new Error("runInteractive requires an argv array. Use runInteractiveShell for shell commands."); } - return result; + const stdio = opts.stdio ?? ["inherit", "pipe", "pipe"]; + return runArrayCmd(cmd, { ...opts, stdio }); } /** - * Run a shell command interactively (stdin inherited) while capturing and redacting stdout/stderr. + * Run an explicit interactive shell command via `bash -c`. * Exits the process on failure unless opts.ignoreError is true. */ -function runInteractive(cmd: string, opts: RunnerOptions = {}): SpawnResult { +function runInteractiveShell(cmd: string, opts: RunnerOptions = {}): SpawnResult { + if (opts.shell) { + throw new Error("runInteractiveShell does not allow opts.shell=true"); + } const stdio = opts.stdio ?? ["inherit", "pipe", "pipe"]; - return spawnAndHandle("bash", ["-c", cmd], opts, stdio, cmd); + const shellCmd = String(cmd); + return spawnAndHandle("bash", ["-c", shellCmd], opts, stdio, shellCmd); +} + +function runDetachedFile( + file: string, + args: readonly (string | number | boolean)[] = [], + opts: DetachedRunnerOptions = {}, +): number | null { + if (opts.shell) { + throw new Error("runDetachedFile does not allow opts.shell=true"); + } + const normalizedArgs = args.map((arg) => String(arg)); + const child = spawnChild(file, normalizedArgs, { + ...opts, + cwd: opts.cwd ?? ROOT, + env: buildEnvForSubprocess(opts.env, opts.inheritFullEnv), + detached: true, + stdio: opts.stdio ?? "ignore", + shell: false, + }); + child.on?.("error", () => {}); + child.unref?.(); + return child.pid ?? null; } /** @@ -172,7 +221,7 @@ function runFile( } const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"]; const normalizedArgs = args.map((arg) => String(arg)); - const rendered = [shellQuote(file), ...normalizedArgs.map((arg) => shellQuote(arg))].join(" "); + const rendered = joinShellWords([file, ...normalizedArgs]); return spawnAndHandle(file, normalizedArgs, { ...opts, shell: false }, stdio, rendered); } @@ -180,28 +229,51 @@ function runFile( * Run a command and return its stdout as a trimmed string. * Throws a redacted error on failure, or returns '' when opts.ignoreError is true. * - * Accepts two forms: - * runCapture("some shell command") — legacy: passes the string to execSync (shell) - * runCapture(["curl", "-sf", url]) — safe: calls spawnSync(exe, args) with no shell - * - * When an argv array is passed, the shell option is forbidden to prevent - * callers from accidentally re-enabling shell interpretation. + * Requires an argv array and never invokes a shell. */ -function runCapture(cmd: string | readonly string[], opts: CaptureOptions = {}): string { - if (Array.isArray(cmd)) { - return runArrayCapture(cmd, opts); +function runCapture(cmd: readonly string[], opts: ArrayCaptureOptions = {}): string { + if (!Array.isArray(cmd)) { + throw new Error("runCapture requires an argv array. Use runCaptureShell for shell commands."); } + return runArrayCapture(cmd, opts); +} + +/** + * Run an explicit shell command and return its stdout as a trimmed string. + * Throws a redacted error on failure, or returns '' when opts.ignoreError is true. + */ +function runCaptureShell(cmd: string, opts: CaptureOptions = {}): string { const shellCmd = String(cmd); + const { ignoreError, inheritFullEnv, env: extraEnv, stdio: _stdio, ...spawnOpts } = opts; + if (spawnOpts.shell) { + throw new Error("runCaptureShell does not allow opts.shell=true"); + } + try { - return execSync(shellCmd, { - ...opts, - encoding: "utf-8", - cwd: ROOT, - env: { ...process.env, ...opts.env }, + const result = spawnResult("bash", ["-c", shellCmd], { + ...spawnOpts, + cwd: spawnOpts.cwd ?? ROOT, + env: buildEnvForSubprocess(extraEnv, inheritFullEnv), stdio: ["pipe", "pipe", "pipe"], - }).trim(); + encoding: "utf-8", + }); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + const shellError = new Error(`Command failed with status ${result.status}`) as Error & { + cmd?: string; + output?: string[]; + }; + shellError.cmd = shellCmd; + shellError.output = [String(result.stdout || ""), String(result.stderr || "")].filter( + Boolean, + ); + throw shellError; + } + return String(result.stdout || "").trim(); } catch (err) { - if (opts.ignoreError) return ""; + if (ignoreError) return ""; throw redactError(err); } } @@ -217,7 +289,13 @@ function runArrayCapture(cmd: readonly string[], opts: ArrayCaptureOptions = {}) const exe = cmd[0]; const args = cmd.slice(1); - const { ignoreError, env: extraEnv, stdio: _stdio, ...spawnOpts } = opts; + const { + ignoreError, + inheritFullEnv, + env: extraEnv, + stdio: _stdio, + ...spawnOpts + } = opts; // Guard: re-enabling shell interpretation defeats the purpose of argv arrays. if (spawnOpts.shell) { @@ -225,13 +303,20 @@ function runArrayCapture(cmd: readonly string[], opts: ArrayCaptureOptions = {}) } try { - const result = spawnSync(exe, args, { - ...spawnOpts, - cwd: ROOT, - env: { ...process.env, ...extraEnv }, - stdio: ["pipe", "pipe", "pipe"], - encoding: "utf-8", - }); + const result = spawnAndHandle( + exe, + args, + { + ...spawnOpts, + ignoreError: true, + suppressOutput: true, + inheritFullEnv, + env: extraEnv, + encoding: "utf-8", + }, + ["pipe", "pipe", "pipe"], + joinShellWords(cmd), + ); // Check result.error first — spawnSync sets this (with status === null) when // the executable is missing (ENOENT), the call times out, or the spawn fails. @@ -253,16 +338,12 @@ function runArrayCapture(cmd: readonly string[], opts: ArrayCaptureOptions = {}) } // Unified redaction — see redact.ts (#2381). -const { redact, redactError, writeRedactedResult } = require("./redact"); +const { redact, redactError, writeRedactedResult } = require("./redact.js"); /** * Shell-quote a value for safe interpolation into bash -c strings. * Wraps in single quotes and escapes embedded single quotes. */ -function shellQuote(value: RunnerScalar): string { - return `'${String(value).replace(/'/g, `'\\''`)}'`; -} - /** * Validate a name (sandbox, instance, container) against RFC 1123 label rules. * Rejects shell metacharacters, path traversal, and empty/overlength names. @@ -287,9 +368,12 @@ export { SCRIPTS, redact, run, + runShell, runCapture, + runCaptureShell, runFile, + runDetachedFile, runInteractive, - shellQuote, + runInteractiveShell, validateName, }; diff --git a/src/lib/sandbox-channels.ts b/src/lib/sandbox-channels.ts index e56f5a5ab24..11cbd4c31aa 100644 --- a/src/lib/sandbox-channels.ts +++ b/src/lib/sandbox-channels.ts @@ -66,7 +66,8 @@ export const KNOWN_CHANNELS: Record = { appTokenHelp: "Slack API → Your Apps → Basic Information → App-Level Tokens (xapp-...).", appTokenLabel: "Slack App Token (Socket Mode)", appTokenFormat: /^xapp-[A-Za-z0-9_-]+$/, - appTokenFormatHint: "Slack app tokens start with 'xapp-' (e.g. xapp-1-A0000-12345-abcdef).", + appTokenFormatHint: + "Slack app tokens start with 'xapp-' (e.g. xapp-" + "1-A0000-12345-abcdef).", }, }; diff --git a/src/lib/sandbox-config.ts b/src/lib/sandbox-config.ts index e80d95888ad..c3a661a1403 100644 --- a/src/lib/sandbox-config.ts +++ b/src/lib/sandbox-config.ts @@ -16,8 +16,7 @@ const readline = require("readline"); const fs = require("fs"); const os = require("os"); const path = require("path"); -const { execFileSync } = require("child_process"); -const { validateName } = require("./runner"); +const { runFile, validateName } = require("./runner"); const credentialFilter: typeof import("./credential-filter") = require("./credential-filter"); const { stripCredentials, isConfigObject, isConfigValue } = credentialFilter; const { appendAuditEntry } = require("./shields-audit"); @@ -516,7 +515,7 @@ async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise // Write config to sandbox via kubectl exec (bypasses Landlock) console.log(` Writing config to sandbox (${target.configPath})...`); const content = fs.readFileSync(tmpFile, "utf-8"); - execFileSync( + runFile( "docker", [ "exec", @@ -535,12 +534,18 @@ async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise "-c", `cat > ${target.configPath}`, ], - { input: content, stdio: ["pipe", "pipe", "pipe"], timeout: 15000 }, + { + input: content, + stdio: ["pipe", "pipe", "pipe"], + timeout: 15000, + ignoreError: false, + suppressOutput: true, + }, ); // Fix ownership via kubectl exec (bypasses Landlock) try { - execFileSync( + runFile( "docker", [ "exec", @@ -557,7 +562,12 @@ async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise "sandbox:sandbox", target.configPath, ], - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + { + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + ignoreError: true, + suppressOutput: true, + }, ); } catch { // Best effort — chown failure is non-fatal diff --git a/src/lib/sandbox-create-stream.test.ts b/src/lib/sandbox-create-stream.test.ts index 6165fa43093..1faf4dd9425 100644 --- a/src/lib/sandbox-create-stream.test.ts +++ b/src/lib/sandbox-create-stream.test.ts @@ -174,4 +174,10 @@ describe("sandbox-create-stream", () => { sawProgress: false, }); }); + + it("rejects empty argv commands up front", () => { + expect(() => streamSandboxCreate([], process.env, { logLine: vi.fn() })).toThrow( + /command must not be empty/, + ); + }); }); diff --git a/src/lib/sandbox-create-stream.ts b/src/lib/sandbox-create-stream.ts index e4494ba12f7..f6c1bcc3775 100644 --- a/src/lib/sandbox-create-stream.ts +++ b/src/lib/sandbox-create-stream.ts @@ -1,9 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; +import type { SpawnOptions } from "node:child_process"; import { ROOT } from "./paths"; +import { spawnChild } from "./process-primitives.js"; export interface StreamSandboxCreateResult { status: number; @@ -49,15 +50,30 @@ export interface StreamableChildProcess { } export function streamSandboxCreate( - command: string, + command: string | readonly string[], env: NodeJS.ProcessEnv = process.env, options: StreamSandboxCreateOptions = {}, ): Promise { - const child: StreamableChildProcess = (options.spawnImpl ?? spawn)("bash", ["-lc", command], { - cwd: ROOT, - env, - stdio: ["ignore", "pipe", "pipe"], - }); + if (Array.isArray(command) && command.length === 0) { + throw new Error("command must not be empty"); + } + + const spawnImpl = options.spawnImpl ?? spawnChild; + let child: StreamableChildProcess; + if (Array.isArray(command)) { + child = spawnImpl(command[0], [...command.slice(1)], { + cwd: ROOT, + env, + stdio: ["ignore", "pipe", "pipe"], + }); + } else { + const shellCommand = String(command); + child = spawnImpl("bash", ["-lc", shellCommand], { + cwd: ROOT, + env, + stdio: ["ignore", "pipe", "pipe"], + }); + } const logLine = options.logLine ?? console.log; const lines: string[] = []; diff --git a/src/lib/sandbox-session-state.ts b/src/lib/sandbox-session-state.ts index df1de801ca3..3c5be0039aa 100644 --- a/src/lib/sandbox-session-state.ts +++ b/src/lib/sandbox-session-state.ts @@ -13,12 +13,22 @@ * CLI output are separated from the I/O layer that invokes those commands. */ -import { spawnSync } from "node:child_process"; +import type { SpawnSyncOptionsWithStringEncoding } from "node:child_process"; + +import { spawnResult } from "./process-primitives.js"; // --------------------------------------------------------------------------- // Types // --------------------------------------------------------------------------- +function runSessionCommand( + command: string, + args: string[], + options: SpawnSyncOptionsWithStringEncoding, +) { + return spawnResult(command, args, options); +} + /** A single detected SSH session to a sandbox. */ export interface SandboxSession { /** The sandbox name this session connects to. */ @@ -255,16 +265,16 @@ export function getActiveSandboxSessions( */ function querySshProcesses(): string | null { try { - const result = spawnSync("ps", ["-axo", "pid,command"], { + const result = runSessionCommand("ps", ["-axo", "pid,command"], { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 5000, }); if (result.status !== 0) return null; // Filter to only SSH lines to reduce noise and match pgrep -a output format - const lines = (result.stdout || "") + const lines = String(result.stdout || "") .split("\n") - .filter((line) => /\bssh\b/.test(line)) + .filter((line: string) => /\bssh\b/.test(line)) .join("\n"); return lines; } catch { @@ -280,13 +290,13 @@ export function createSystemDeps(openshellBinary: string): SessionDetectionDeps return { getForwardList: (): string | null => { try { - const result = spawnSync(openshellBinary, ["forward", "list"], { + const result = runSessionCommand(openshellBinary, ["forward", "list"], { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 5000, }); if (result.status !== 0) return null; - return result.stdout || ""; + return String(result.stdout || ""); } catch { return null; } diff --git a/src/lib/sandbox-state.ts b/src/lib/sandbox-state.ts index 9480ca6cebd..e6f4a61b087 100644 --- a/src/lib/sandbox-state.ts +++ b/src/lib/sandbox-state.ts @@ -9,7 +9,9 @@ // // Credentials are stripped from backups using shared credential-filter.ts. -import { spawnSync } from "child_process"; +import type { SpawnSyncOptions, SpawnSyncOptionsWithStringEncoding } from "child_process"; + +import { spawnResult } from "./process-primitives.js"; import { chmodSync, existsSync, @@ -29,7 +31,9 @@ import * as registry from "./registry.js"; import { loadAgent } from "./agent-defs.js"; import { resolveOpenshell } from "./resolve-openshell.js"; import { captureOpenshellCommand } from "./openshell.js"; +import { buildEnvForSubprocess } from "./subprocess-env.js"; import { sanitizeConfigFile, isSensitiveFile } from "./credential-filter.js"; +import { formatShellToken } from "./shell-quote.js"; const HOME_DIR = path.resolve(process.env.HOME || os.homedir()); const REBUILD_BACKUPS_DIR = path.join(HOME_DIR, ".nemoclaw", "rebuild-backups"); @@ -40,6 +44,64 @@ function parseJson(text: string): T { return JSON.parse(text); } +function runStateCommand( + file: string, + args: string[], + opts: SpawnSyncOptions | SpawnSyncOptionsWithStringEncoding = {}, +) { + return spawnResult(file, args, { + ...opts, + env: buildEnvForSubprocess(opts.env), + }); +} + +function resultStdoutText(result: { stdout?: string | Buffer | null }): string { + const stdout = result.stdout; + return typeof stdout === "string" ? stdout : stdout?.toString("utf-8") || ""; +} + +function resultStderrText(result: { stderr?: string | Buffer | null }): string { + const stderr = result.stderr; + return typeof stderr === "string" ? stderr : stderr?.toString("utf-8") || ""; +} + +function resultStdoutBuffer(result: { stdout?: string | Buffer | null }): Buffer { + const stdout = result.stdout; + return Buffer.isBuffer(stdout) ? stdout : Buffer.from(String(stdout || ""), "utf-8"); +} + +function isSafeManifestStateDir(dir: string, backupRoot: string, writableDir: string): boolean { + if (!dir || dir.includes("\0") || path.isAbsolute(dir) || path.posix.isAbsolute(dir)) { + return false; + } + const segments = dir.split("/"); + if ( + segments.some( + (segment) => + segment === "" || segment === "." || segment === ".." || segment.startsWith("-"), + ) + ) { + return false; + } + + const hostPath = path.resolve(backupRoot, dir); + if (!isWithinRoot(hostPath, backupRoot)) { + return false; + } + + const remoteRoot = writableDir.replace(/\/+$/, ""); + const remotePath = path.posix.normalize(path.posix.join(remoteRoot, dir)); + return remotePath !== remoteRoot && remotePath.startsWith(`${remoteRoot}/`); +} + +function buildRemoveDirsCommand(baseDir: string, dirs: string[], backupRoot: string): string { + const invalidDirs = dirs.filter((dir) => !isSafeManifestStateDir(dir, backupRoot, baseDir)); + if (invalidDirs.length > 0) { + throw new Error(`Invalid state dirs: ${invalidDirs.join(", ")}`); + } + return dirs.map((dir) => `rm -rf -- ${formatShellToken(`${baseDir}/${dir}`)}`).join(" && "); +} + // ── Types ────────────────────────────────────────────────────────── export interface RebuildManifest { @@ -211,7 +273,7 @@ function rejectSymlinksOnPath(targetPath: string): void { * Rejects absolute paths, path traversal (..), and null bytes. */ export function validateTarEntries(tarBuffer: Buffer, targetDir: string): TarValidationResult { - const result = spawnSync("tar", ["-tf", "-"], { + const result = runStateCommand("tar", ["-tf", "-"], { input: tarBuffer, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"], @@ -223,15 +285,15 @@ export function validateTarEntries(tarBuffer: Buffer, targetDir: string): TarVal safe: false, entries: [], violations: [ - `tar listing failed (exit ${result.status}): ${(result.stderr || "").substring(0, 200)}`, + `tar listing failed (exit ${result.status}): ${resultStderrText(result).substring(0, 200)}`, ], }; } - const entries = (result.stdout || "") + const entries = resultStdoutText(result) .trim() .split("\n") - .filter((e) => e.length > 0); + .filter((e: string) => e.length > 0); const violations: string[] = []; for (const entry of entries) { @@ -333,7 +395,7 @@ function auditExtractedSymlinks(dirPath: string, allowedRoots: string[]): string * files outside the extraction root. */ export function rejectHardLinks(tarBuffer: Buffer): string[] { - const result = spawnSync("tar", ["-tvf", "-"], { + const result = runStateCommand("tar", ["-tvf", "-"], { input: tarBuffer, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"], @@ -345,10 +407,10 @@ export function rejectHardLinks(tarBuffer: Buffer): string[] { } const violations: string[] = []; - const lines = (result.stdout || "") + const lines = resultStdoutText(result) .trim() .split("\n") - .filter((l) => l.length > 0); + .filter((l: string) => l.length > 0); for (const line of lines) { // Both GNU tar and bsdtar prefix hard-link entries with 'h' in verbose mode @@ -385,7 +447,7 @@ export function safeTarExtract(tarBuffer: Buffer, targetDir: string): SafeExtrac } // Phase 2: Extract with --no-same-owner to prevent ownership manipulation - const extractResult = spawnSync("tar", ["-xf", "-", "--no-same-owner", "-C", targetDir], { + const extractResult = runStateCommand("tar", ["-xf", "-", "--no-same-owner", "-C", targetDir], { input: tarBuffer, stdio: ["pipe", "pipe", "pipe"], timeout: 60000, @@ -394,7 +456,7 @@ export function safeTarExtract(tarBuffer: Buffer, targetDir: string): SafeExtrac if (extractResult.status !== 0) { return { success: false, - error: `tar extraction failed (exit ${extractResult.status}): ${(extractResult.stderr?.toString() || "").substring(0, 200)}`, + error: `tar extraction failed (exit ${extractResult.status}): ${resultStderrText(extractResult).substring(0, 200)}`, }; } @@ -592,6 +654,18 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = } const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); const backupPath = path.join(REBUILD_BACKUPS_DIR, sandboxName, timestamp); + const invalidManifestStateDirs = stateDirs.filter( + (dir) => !isSafeManifestStateDir(dir, backupPath, writableDir), + ); + if (invalidManifestStateDirs.length > 0) { + _log(`FAILED: agent manifest declares invalid state dirs: [${invalidManifestStateDirs.join(",")}]`); + return { + success: false, + backedUpDirs: [], + failedDirs: [...stateDirs], + error: `Agent manifest contains invalid state dirs: ${invalidManifestStateDirs.join(", ")}`, + }; + } // SECURITY: Verify backup destination ancestors are not symlinks. // Without this check, an attacker who plants ~/.nemoclaw/rebuild-backups @@ -648,26 +722,38 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = // First, check which declared state dirs actually exist in the sandbox, // then additionally discover per-agent `workspace-*` directories produced // by multi-agent OpenClaw deployments (see issue #1260) so they get - // snapshotted alongside the manifest-declared dirs. `awk '!seen[$0]++'` - // dedupes while preserving order. + // snapshotted alongside the manifest-declared dirs. Deduping happens + // locally so remote probe failures are not masked by a pipeline. const existCheckCmd = stateDirs - .map((d) => `[ -d "${writableDir}/${d}" ] && echo "${d}"`) + .map((d) => { + const dirPath = `${writableDir}/${d}`; + return `if [ -d ${formatShellToken(dirPath)} ]; then printf '%s\\n' ${formatShellToken(d)}; fi`; + }) .join("; "); - const workspaceGlobCmd = `for d in ${writableDir}/workspace-*/; do [ -d "$d" ] && basename "$d"; done 2>/dev/null`; - const fullCheckCmd = `{ ${existCheckCmd}; ${workspaceGlobCmd}; } 2>/dev/null | awk '!seen[$0]++'`; + const workspaceGlobCmd = + `cd ${formatShellToken(writableDir)} && ` + + `for d in workspace-*/; do [ -d "$d" ] && basename "$d"; done`; + const fullCheckCmd = `set -e; ${existCheckCmd}; ${workspaceGlobCmd}`; _log(`Checking existing dirs via SSH: ${fullCheckCmd.substring(0, 100)}...`); - const existResult = spawnSync("ssh", [...sshArgs(configFile, sandboxName), fullCheckCmd], { + const existResult = runStateCommand("ssh", [...sshArgs(configFile, sandboxName), fullCheckCmd], { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 30000, }); _log( - `Dir check: exit=${existResult.status}, stdout=${(existResult.stdout || "").trim().substring(0, 200)}, stderr=${(existResult.stderr || "").trim().substring(0, 200)}`, + `Dir check: exit=${existResult.status}, stdout=${resultStdoutText(existResult).trim().substring(0, 200)}, stderr=${resultStderrText(existResult).trim().substring(0, 200)}`, + ); + const existingDirs = Array.from( + new Set( + resultStdoutText(existResult) + .trim() + .split("\n") + .filter((d: string) => d.length > 0), + ), + ); + const invalidExistingDirs = existingDirs.filter( + (dir) => !isSafeManifestStateDir(dir, backupPath, writableDir), ); - const existingDirs = (existResult.stdout || "") - .trim() - .split("\n") - .filter((d) => d.length > 0); _log( `Existing dirs in sandbox: [${existingDirs.join(",")}] (${existingDirs.length}/${stateDirs.length})`, ); @@ -679,6 +765,16 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = return { success: false, manifest, backedUpDirs, failedDirs: [...stateDirs] }; } + if (invalidExistingDirs.length > 0) { + _log(`FAILED: sandbox reported invalid state dirs: [${invalidExistingDirs.join(",")}]`); + return { + success: false, + manifest, + backedUpDirs, + failedDirs: [...stateDirs, ...invalidExistingDirs.filter((dir) => !stateDirs.includes(dir))], + }; + } + if (existingDirs.length === 0) { _log("No state dirs found in sandbox (all empty)"); writeManifest(backupPath, manifest); @@ -686,20 +782,22 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = } // Download via SSH+tar - const tarCmd = `tar -cf - -C ${writableDir} ${existingDirs.join(" ")}`; + const tarCmd = + `tar -cf - -C ${formatShellToken(writableDir)} -- ` + + existingDirs.map((dir) => formatShellToken(dir)).join(" "); _log(`Downloading via SSH+tar: ${tarCmd}`); - const result = spawnSync("ssh", [...sshArgs(configFile, sandboxName), tarCmd], { + const result = runStateCommand("ssh", [...sshArgs(configFile, sandboxName), tarCmd], { stdio: ["ignore", "pipe", "pipe"], timeout: 120000, maxBuffer: 256 * 1024 * 1024, }); _log( - `SSH+tar download: exit=${result.status}, stdout=${result.stdout ? result.stdout.length + " bytes" : "null"}, stderr=${(result.stderr?.toString() || "").substring(0, 200)}`, + `SSH+tar download: exit=${result.status}, stdout=${result.stdout ? resultStdoutBuffer(result).length + " bytes" : "null"}, stderr=${resultStderrText(result).substring(0, 200)}`, ); - if (result.status === 0 && result.stdout && result.stdout.length > 0) { + if (result.status === 0 && result.stdout && resultStdoutBuffer(result).length > 0) { // SECURITY: Validate tar entries, extract safely, audit symlinks - const extractResult = safeTarExtract(result.stdout, backupPath); + const extractResult = safeTarExtract(resultStdoutBuffer(result), backupPath); if (extractResult.success) { backedUpDirs.push(...existingDirs); } else { @@ -763,6 +861,14 @@ export function restoreSandboxState(sandboxName: string, backupPath: string): Re const restoredDirs: string[] = []; const failedDirs: string[] = []; + const invalidDirs = manifest.stateDirs.filter( + (dir) => !isSafeManifestStateDir(dir, backupPath, writableDir), + ); + if (invalidDirs.length > 0) { + _log(`FAILED: invalid snapshot paths in manifest: [${invalidDirs.join(",")}]`); + return { success: false, restoredDirs, failedDirs: [...manifest.stateDirs] }; + } + // Find which backed-up directories actually exist locally const localDirs = manifest.stateDirs.filter((d) => existsSync(path.join(backupPath, d))); _log( @@ -784,7 +890,7 @@ export function restoreSandboxState(sandboxName: string, backupPath: string): Re const configFile = writeTempSshConfig(sshConfig); try { // Upload via tar pipe - const tarResult = spawnSync("tar", ["-cf", "-", "-C", backupPath, ...localDirs], { + const tarResult = runStateCommand("tar", ["-cf", "-", "-C", backupPath, "--", ...localDirs], { stdio: ["ignore", "pipe", "pipe"], timeout: 60000, maxBuffer: 256 * 1024 * 1024, @@ -796,44 +902,66 @@ export function restoreSandboxState(sandboxName: string, backupPath: string): Re // Remove existing state dirs before extracting so stale files from // later snapshots don't persist after restoring an earlier one. - const rmCmd = localDirs.map((d) => `rm -rf "${writableDir}/${d}"`).join(" && "); + const rmCmd = buildRemoveDirsCommand(writableDir, localDirs, backupPath); _log(`Cleaning target dirs before restore: ${rmCmd}`); - const rmResult = spawnSync("ssh", [...sshArgs(configFile, sandboxName), rmCmd], { + const rmResult = runStateCommand("ssh", [...sshArgs(configFile, sandboxName), rmCmd], { stdio: ["ignore", "pipe", "pipe"], timeout: 30000, }); if (rmResult.status !== 0) { _log( - `WARNING: pre-restore cleanup failed (exit ${rmResult.status}): ${(rmResult.stderr?.toString() || "").substring(0, 200)}`, + `WARNING: pre-restore cleanup failed (exit ${rmResult.status}): ${resultStderrText(rmResult).substring(0, 200)}`, ); } - const extractCmd = `tar -xf - -C ${writableDir}`; - const sshResult = spawnSync("ssh", [...sshArgs(configFile, sandboxName), extractCmd], { - input: tarResult.stdout, + const extractCmd = `tar -xf - -C ${formatShellToken(writableDir)}`; + const sshResult = runStateCommand("ssh", [...sshArgs(configFile, sandboxName), extractCmd], { + input: resultStdoutBuffer(tarResult), stdio: ["pipe", "pipe", "pipe"], timeout: 120000, }); if (sshResult.status === 0) { - restoredDirs.push(...localDirs); + let ownershipOk = true; // Fix ownership — treat failure as restore failure since wrong // ownership means the agent can't read its own state files. const openshellBinary = resolveOpenshell(); if (openshellBinary) { _log(`Fixing ownership: chown -R sandbox:sandbox ${writableDir}`); - const chownResult = spawnSync( + const chownResult = runStateCommand( openshellBinary, ["sandbox", "exec", sandboxName, "--", "chown", "-R", "sandbox:sandbox", writableDir], { stdio: ["ignore", "pipe", "pipe"], timeout: 30000 }, ); if (chownResult.status !== 0) { + ownershipOk = false; _log( `WARNING: chown failed (exit ${chownResult.status}) — agent may not be able to read restored state`, ); } } + + if (ownershipOk) { + restoredDirs.push(...localDirs); + } else { + const rollbackCmd = buildRemoveDirsCommand(writableDir, localDirs, backupPath); + _log(`Rolling back extracted dirs after chown failure: ${rollbackCmd}`); + const rollbackResult = runStateCommand( + "ssh", + [...sshArgs(configFile, sandboxName), rollbackCmd], + { + stdio: ["ignore", "pipe", "pipe"], + timeout: 30000, + }, + ); + if (rollbackResult.status !== 0) { + _log( + `WARNING: rollback failed (exit ${rollbackResult.status}): ${resultStderrText(rollbackResult).substring(0, 200)}`, + ); + } + failedDirs.push(...localDirs); + } } else { failedDirs.push(...localDirs); } diff --git a/src/lib/sandbox-version.ts b/src/lib/sandbox-version.ts index 6d24c8c6cdc..010872d435c 100644 --- a/src/lib/sandbox-version.ts +++ b/src/lib/sandbox-version.ts @@ -8,8 +8,8 @@ // Fast: registry lookup (no SSH, used when agentVersion is already cached) // Slow: SSH exec into sandbox, run version_command, cache result in registry -import { spawnSync } from "child_process"; import fs from "fs"; +import { spawnResult } from "./process-primitives.js"; import os from "os"; import path from "path"; @@ -56,7 +56,7 @@ export function probeAgentVersion(sandboxName: string): string | null { const tmpFile = path.join(os.tmpdir(), `nemoclaw-ver-${process.pid}-${Date.now()}.conf`); fs.writeFileSync(tmpFile, sshConfigResult.output, { mode: 0o600 }); try { - const result = spawnSync( + const result = spawnResult( "ssh", [ "-F", tmpFile, @@ -70,7 +70,7 @@ export function probeAgentVersion(sandboxName: string): string | null { { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, ); if (result.status !== 0) return null; - return parseVersionFromText(result.stdout); + return parseVersionFromText(String(result.stdout || "")); } catch { return null; } finally { diff --git a/src/lib/services.ts b/src/lib/services.ts index 34ff9db24d6..84f7365c6c6 100644 --- a/src/lib/services.ts +++ b/src/lib/services.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execSync, spawn } from "node:child_process"; +import { spawnChild } from "./process-primitives.js"; import { closeSync, existsSync, @@ -15,6 +15,7 @@ import { join } from "node:path"; import { DASHBOARD_PORT } from "./ports"; import { buildSubprocessEnv } from "./subprocess-env"; +import { hasExecutable } from "./find-executable"; // --------------------------------------------------------------------------- // Types @@ -124,7 +125,7 @@ function startService( // does not accept raw file descriptors for stdio. const logFile = join(pidDir, `${name}.log`); const logFd = openSync(logFile, "w"); - const subprocess = spawn(command, args, { + const subprocess = spawnChild(command, args, { detached: true, stdio: ["ignore", logFd, logFd], env: buildSubprocessEnv(env), @@ -258,16 +259,13 @@ export async function startAll(opts: ServiceOptions = {}): Promise { // No host-side bridge processes are needed. See: PR #1081. // cloudflared tunnel - try { - execSync("command -v cloudflared", { - stdio: ["ignore", "ignore", "ignore"], - }); + if (hasExecutable("cloudflared")) { startService(pidDir, "cloudflared", "cloudflared", [ "tunnel", "--url", `http://localhost:${String(dashboardPort)}`, ]); - } catch { + } else { warn("cloudflared not found — no public URL. Install cloudflared manually if you need one."); } diff --git a/src/lib/shell-quote.test.ts b/src/lib/shell-quote.test.ts new file mode 100644 index 00000000000..9b6b165151e --- /dev/null +++ b/src/lib/shell-quote.test.ts @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { buildShellAssignment } from "../../dist/lib/shell-quote"; + +describe("buildShellAssignment", () => { + it("renders safe environment variable assignments", () => { + expect(buildShellAssignment("NEMOCLAW_SANDBOX_NAME", "alpha")).toBe( + "NEMOCLAW_SANDBOX_NAME=alpha", + ); + }); + + it("rejects invalid assignment names", () => { + expect(() => buildShellAssignment("1INVALID", "alpha")).toThrow( + /Invalid shell assignment name/, + ); + expect(() => buildShellAssignment("BAD-NAME", "alpha")).toThrow( + /Invalid shell assignment name/, + ); + }); +}); diff --git a/src/lib/shell-quote.ts b/src/lib/shell-quote.ts index 55b88d51623..30ae263b45b 100644 --- a/src/lib/shell-quote.ts +++ b/src/lib/shell-quote.ts @@ -5,6 +5,30 @@ * Shell-quote a value for safe interpolation into bash -c strings. * Wraps in single quotes and escapes embedded single quotes. */ -export function shellQuote(value: string): string { +export type ShellQuotable = string | number | boolean | null | undefined; + +const SAFE_SHELL_TOKEN_RE = /^[A-Za-z0-9_@%+=:,./-]+$/; +const SHELL_ASSIGNMENT_NAME_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; + +function quoteShellValue(value: ShellQuotable): string { return `'${String(value).replace(/'/g, `'\\''`)}'`; } + +export function shellQuote(value: ShellQuotable): string { + return quoteShellValue(value); +} + +export function formatShellToken(value: string): string { + return SAFE_SHELL_TOKEN_RE.test(value) ? value : quoteShellValue(value); +} + +export function joinShellWords(values: readonly string[]): string { + return values.map((value) => formatShellToken(value)).join(" "); +} + +export function buildShellAssignment(name: string, value: string): string { + if (!SHELL_ASSIGNMENT_NAME_RE.test(name)) { + throw new Error(`Invalid shell assignment name: ${JSON.stringify(name)}`); + } + return `${name}=${formatShellToken(value)}`; +} diff --git a/src/lib/shields.ts b/src/lib/shields.ts index 79b920064fb..4891f2f7de9 100644 --- a/src/lib/shields.ts +++ b/src/lib/shields.ts @@ -11,8 +11,9 @@ const fs = require("fs"); const path = require("path"); -const { fork, execFileSync } = require("child_process"); -const { run, runCapture, validateName, shellQuote } = require("./runner"); +const { run, runCapture, runFile, validateName } = require("./runner"); +const { spawnChild } = require("./process-primitives"); +const { buildSubprocessEnv } = require("./subprocess-env"); const { buildPolicyGetCommand, buildPolicySetCommand, @@ -36,8 +37,8 @@ const STATE_DIR = path.join(process.env.HOME ?? "/tmp", ".nemoclaw", "state"); const K3S_CONTAINER = "openshell-cluster-nemoclaw"; -function kubectlExec(sandboxName: string, cmd: string[]): void { - execFileSync( +function execKubectlInSandbox(sandboxName: string, cmd: string[]): Buffer { + const result = runFile( "docker", [ "exec", @@ -52,28 +53,30 @@ function kubectlExec(sandboxName: string, cmd: string[]): void { "--", ...cmd, ], - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + { + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + ignoreError: true, + suppressOutput: true, + }, ); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + throw new Error(String(result.stderr || result.stdout || `docker exec failed (${result.status})`)); + } + return Buffer.isBuffer(result.stdout) + ? result.stdout + : Buffer.from(String(result.stdout || ""), "utf-8"); +} + +function kubectlExec(sandboxName: string, cmd: string[]): void { + execKubectlInSandbox(sandboxName, cmd); } function kubectlExecCapture(sandboxName: string, cmd: string[]): string { - return execFileSync( - "docker", - [ - "exec", - K3S_CONTAINER, - "kubectl", - "exec", - "-n", - "openshell", - sandboxName, - "-c", - "agent", - "--", - ...cmd, - ], - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, - ) + return execKubectlInSandbox(sandboxName, cmd) .toString() .trim(); } @@ -476,16 +479,16 @@ function shieldsDown(sandboxName: string, opts: ShieldsDownOpts = {}): void { const actualScript = fs.existsSync(timerScriptJs) ? timerScriptJs : timerScript; try { - const child = fork( - actualScript, - [sandboxName, snapshotPath, restoreAt.toISOString(), target.configPath, target.configDir], + const child = spawnChild( + process.execPath, + [actualScript, sandboxName, snapshotPath, restoreAt.toISOString(), target.configPath, target.configDir], { detached: true, - stdio: ["ignore", "ignore", "ignore", "ipc"], + stdio: "ignore", + env: buildSubprocessEnv(), }, ); - child.disconnect(); - child.unref(); + child.unref?.(); // Write timer marker const markerPath = timerMarkerPath(sandboxName); diff --git a/src/lib/skill-install.test.ts b/src/lib/skill-install.test.ts index 5a07164b1d2..75464713787 100644 --- a/src/lib/skill-install.test.ts +++ b/src/lib/skill-install.test.ts @@ -272,7 +272,7 @@ describe("postInstall", () => { true, ); expect(commands).toContain( - "printf '{}' > '/sandbox/.openclaw-data/agents/main/sessions/sessions.json'", + "printf '{}' > /sandbox/.openclaw-data/agents/main/sessions/sessions.json", ); } finally { rmSync(skillDir, { recursive: true, force: true }); diff --git a/src/lib/skill-install.ts b/src/lib/skill-install.ts index 91090285990..82ecb3bd167 100644 --- a/src/lib/skill-install.ts +++ b/src/lib/skill-install.ts @@ -9,7 +9,12 @@ import fs from "node:fs"; import path from "node:path"; -import { spawnSync } from "node:child_process"; + +// runner.ts still uses CommonJS-style exports — use require here. +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { runFile } = require("./runner"); +import { buildShellCommand } from "./remote-script.js"; +import { shellQuote } from "./shell-quote"; // yaml is a production dependency (used by policies.ts, onboard.ts) import YAML from "yaml"; @@ -120,10 +125,8 @@ export function resolveSkillPaths( // ── Shell safety ───────────────────────────────────────────────── -// Re-export shellQuote from runner.ts — a repo-wide test enforces -// a single definition lives in runner.ts. -// eslint-disable-next-line @typescript-eslint/no-var-requires -const { shellQuote } = require("./runner"); +// Re-export shellQuote from the shared helper so skill-install callers can +// use the same quoting function as the rest of the CLI. export { shellQuote }; const SAFE_PATH_RE = /^[A-Za-z0-9._\-/]+$/; @@ -144,6 +147,7 @@ export function validateRelativePath(rel: string): boolean { export interface SshContext { configFile: string; sandboxName: string; + sshArgs?: string[]; } export interface SshResult { @@ -162,27 +166,28 @@ export function sshExec( opts: { input?: string | Buffer; timeout?: number } = {}, ): SshResult | null { try { - const result = spawnSync( + const sshArgs = ctx.sshArgs ?? [ + "-F", + ctx.configFile, + "-o", + "StrictHostKeyChecking=no", + "-o", + "UserKnownHostsFile=/dev/null", + "-o", + "ConnectTimeout=10", + "-o", + "LogLevel=ERROR", + ]; + const result = runFile( "ssh", - [ - "-F", - ctx.configFile, - "-o", - "StrictHostKeyChecking=no", - "-o", - "UserKnownHostsFile=/dev/null", - "-o", - "ConnectTimeout=10", - "-o", - "LogLevel=ERROR", - `openshell-${ctx.sandboxName}`, - command, - ], + [...sshArgs, `openshell-${ctx.sandboxName}`, command], { encoding: "utf-8", stdio: [opts.input !== undefined ? "pipe" : "ignore", "pipe", "pipe"], input: opts.input, timeout: opts.timeout ?? 30_000, + ignoreError: true, + suppressOutput: true, }, ); return { @@ -207,7 +212,12 @@ export function uploadFile( ): SshResult | null { const content = fs.readFileSync(localPath); const remotePath = `${remoteDir}/${remoteFilename}`; - const script = `mkdir -p ${shellQuote(remoteDir)} && cat > ${shellQuote(remotePath)}`; + const script = buildShellCommand({ + steps: [ + { commandArgs: ["mkdir", "-p", remoteDir] }, + { commandArgs: ["cat"], stdoutRedirect: remotePath }, + ], + }); return sshExec(ctx, script, { input: content }); } @@ -324,7 +334,13 @@ export function postInstall( // Clear sessions.json so OpenClaw re-discovers skills on the next // session even after an in-place skill update. if (paths.sessionFile && !opts.skipRefresh) { - const refreshResult = runSsh(ctx, `printf '{}' > ${shellQuote(paths.sessionFile)}`); + const refreshResult = runSsh( + ctx, + buildShellCommand({ + commandArgs: ["printf", "{}"], + stdoutRedirect: paths.sessionFile, + }), + ); if (!refreshResult || refreshResult.status !== 0) { messages.push("Warning: failed to clear sessions (agent may need manual restart)"); } @@ -336,20 +352,29 @@ export function postInstall( return { success: true, messages }; } +function skillExists(ctx: SshContext, uploadDir: string): boolean { + const result = sshExec( + ctx, + buildShellCommand({ + steps: [ + { commandArgs: ["test", "-f", `${uploadDir}/SKILL.md`] }, + { command: "echo EXISTS" }, + ], + }), + ); + return result !== null && result.stdout === "EXISTS"; +} + /** * Check whether a skill already exists on the sandbox at the upload path. */ export function checkExisting(ctx: SshContext, paths: SkillPaths): boolean { - const target = shellQuote(`${paths.uploadDir}/SKILL.md`); - const result = sshExec(ctx, `test -f ${target} && echo EXISTS`); - return result !== null && result.stdout === "EXISTS"; + return skillExists(ctx, paths.uploadDir); } /** * Verify the SKILL.md file exists on the sandbox at the expected path. */ export function verifyInstall(ctx: SshContext, paths: SkillPaths): boolean { - const target = shellQuote(`${paths.uploadDir}/SKILL.md`); - const result = sshExec(ctx, `test -f ${target} && echo EXISTS`); - return result !== null && result.stdout === "EXISTS"; + return skillExists(ctx, paths.uploadDir); } diff --git a/src/lib/subprocess-env.ts b/src/lib/subprocess-env.ts index 8efa17117ec..a64b2aa164a 100644 --- a/src/lib/subprocess-env.ts +++ b/src/lib/subprocess-env.ts @@ -26,7 +26,16 @@ const TEMP = ["TMPDIR", "TMP", "TEMP"]; const LOCALE = ["LANG"]; // LC_* handled via prefix -const PROXY = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"]; +const PROXY = [ + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "ALL_PROXY", + "http_proxy", + "https_proxy", + "no_proxy", + "all_proxy", +]; const TLS = [ "SSL_CERT_FILE", @@ -40,7 +49,17 @@ const TLS = [ const TOOLCHAIN = ["DOCKER_HOST", "KUBECONFIG", "SSH_AUTH_SOCK", "RUST_LOG", "RUST_BACKTRACE"]; -const ALLOWED_ENV_NAMES = new Set([...SYSTEM, ...TEMP, ...LOCALE, ...PROXY, ...TLS, ...TOOLCHAIN]); +const NEMOCLAW = ["NEMOCLAW_NON_INTERACTIVE"]; + +const ALLOWED_ENV_NAMES = new Set([ + ...SYSTEM, + ...TEMP, + ...LOCALE, + ...PROXY, + ...TLS, + ...TOOLCHAIN, + ...NEMOCLAW, +]); // ── Allowed prefixes ─────────────────────────────────────────── @@ -61,3 +80,20 @@ export function buildSubprocessEnv(extra?: Record): Record = {}; + for (const [key, value] of Object.entries(extraEnv ?? {})) { + if (value !== undefined) { + normalizedExtraEnv[key] = value; + } + } + return buildSubprocessEnv(normalizedExtraEnv); +} diff --git a/src/lib/version.ts b/src/lib/version.ts index d6ff45808fa..b632b297315 100644 --- a/src/lib/version.ts +++ b/src/lib/version.ts @@ -1,10 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execFileSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; +// runner.ts still uses CommonJS-style exports — use require here. +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { runCapture } = require("./runner"); + type PackageInfo = { version?: string }; function parseJson(text: string): T { @@ -31,16 +34,11 @@ export function getVersion(opts: VersionOptions = {}): string { const root = opts.rootDir ?? join(__dirname, "..", ".."); // 1. Try git (available in dev clones and CI) - try { - const raw = execFileSync("git", ["describe", "--tags", "--match", "v*"], { - cwd: root, - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - if (raw) return raw.replace(/^v/, ""); - } catch { - // no git, or no matching tags — fall through - } + const gitDescribe = runCapture(["git", "describe", "--tags", "--match", "v*"], { + cwd: root, + ignoreError: true, + }); + if (gitDescribe) return gitDescribe.replace(/^v/, ""); // 2. Try .version file (stamped by prepublishOnly) const versionFile = join(root, ".version"); diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index 3f137404164..cd5ebeab040 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -const { execFileSync, spawnSync } = require("child_process"); +const { spawnSync } = require("child_process"); const path = require("path"); const fs = require("fs"); const os = require("os"); @@ -21,14 +21,7 @@ const R = _useColor ? "\x1b[0m" : ""; const _RD = _useColor ? "\x1b[1;31m" : ""; const YW = _useColor ? "\x1b[1;33m" : ""; -const { - ROOT, - run, - runCapture: _runCapture, - runInteractive, - shellQuote, - validateName, -} = require("./lib/runner"); +const { ROOT, run, runCapture, runFile, runInteractive, validateName } = require("./lib/runner"); const { resolveOpenshell } = require("./lib/resolve-openshell"); const { fetchGatewayAuthTokenFromSandbox, @@ -70,7 +63,7 @@ const { versionGte, } = require("./lib/openshell"); const { listSandboxesCommand, showStatusCommand } = require("./lib/inventory-commands"); -const { executeDeploy } = require("./lib/deploy"); +const { buildSshArgs, executeDeploy } = require("./lib/deploy"); const { runStartCommand, runStopCommand } = require("./lib/services-command"); const { buildVersionedUninstallUrl, runUninstallCommand } = require("./lib/uninstall-command"); const agentRuntime = require("../bin/lib/agent-runtime"); @@ -80,6 +73,8 @@ const { parseRestoreArgs } = sandboxState; const skillInstall = require("./lib/skill-install"); const { sleepSeconds } = require("./lib/wait"); const { parseSandboxPhase } = require("./lib/gateway-state"); +const { formatShellToken } = require("./lib/shell-quote"); +const { listGatewayDockerVolumes: listGatewayVolumes } = require("./lib/gateway-volumes"); const { getActiveSandboxSessions, createSystemDeps: createSessionDeps, @@ -173,10 +168,10 @@ function captureOpenshell(args: CommandArgs, opts: RunnerOptions = {}) { function cleanupGatewayAfterLastSandbox() { runOpenshell(["forward", "stop", DASHBOARD_FORWARD_PORT], { ignoreError: true }); runOpenshell(["gateway", "destroy", "-g", NEMOCLAW_GATEWAY_NAME], { ignoreError: true }); - run( - `docker volume ls -q --filter "name=openshell-cluster-${NEMOCLAW_GATEWAY_NAME}" | grep . && docker volume ls -q --filter "name=openshell-cluster-${NEMOCLAW_GATEWAY_NAME}" | xargs docker volume rm || true`, - { ignoreError: true }, - ); + const dockerVolumes = listGatewayVolumes(NEMOCLAW_GATEWAY_NAME, runCapture); + if (dockerVolumes.length > 0) { + run(["docker", "volume", "rm", ...dockerVolumes], { ignoreError: true }); + } } function hasNoLiveSandboxes() { @@ -207,6 +202,74 @@ function getInstalledOpenshellVersionOrNull() { }); } +function resolveSandboxSshTarget(configFile: string, sandboxName: string) { + const hostAlias = `openshell-${sandboxName}`; + const configResult = runFile("ssh", ["-F", configFile, "-G", hostAlias], { + encoding: "utf-8", + ignoreError: true, + suppressOutput: true, + stdio: ["ignore", "pipe", "ignore"], + }); + const configText = String(configResult.stdout || "").trim(); + if (configResult.status !== 0 || !configText) { + return null; + } + const lines = configText.split(/\r?\n/); + const findValue = (key: string): string | null => { + const line = lines.find((entry) => entry.startsWith(`${key} `)); + return line ? line.split(/\s+/, 2)[1] || null : null; + }; + return { + hostAlias, + realHost: findValue("hostname") || hostAlias, + sshPort: findValue("port"), + }; +} + +function buildPinnedSandboxSshContext( + configFile: string, + sandboxName: string, + opts: { connectTimeoutSeconds: number; tempDirPrefix: string }, +) { + const rawConfig = fs.readFileSync(configFile, "utf-8").trim(); + if (!rawConfig) { + throw new Error(`Sandbox SSH config is empty for ${sandboxName}`); + } + + const target = resolveSandboxSshTarget(configFile, sandboxName); + if (!target) { + throw new Error(`Failed to resolve sandbox SSH target for ${sandboxName}`); + } + const { hostAlias, realHost, sshPort } = target; + const knownHostsDir = fs.mkdtempSync(path.join(os.tmpdir(), opts.tempDirPrefix)); + const knownHostsFile = path.join(knownHostsDir, "known_hosts"); + const keyscanArgs = ["-T", "5", "-H", ...(sshPort ? ["-p", sshPort] : []), realHost]; + const hostKeysResult = runFile("ssh-keyscan", keyscanArgs, { + encoding: "utf-8", + stdio: ["ignore", "pipe", "ignore"], + ignoreError: true, + suppressOutput: true, + }); + const hostKeys = String(hostKeysResult.stdout || "").trim(); + if (hostKeysResult.status !== 0 || !hostKeys) { + fs.rmSync(knownHostsDir, { recursive: true, force: true }); + throw new Error(`Failed to pin sandbox SSH host key for ${sandboxName}`); + } + fs.writeFileSync(knownHostsFile, `${hostKeys}\n`, { mode: 0o600 }); + + return { + hostAlias, + sshArgs: [ + "-F", + configFile, + ...buildSshArgs(knownHostsFile), + "-o", + `ConnectTimeout=${opts.connectTimeoutSeconds}`, + ], + cleanupDir: knownHostsDir, + }; +} + // ── Sandbox process health (OpenClaw gateway inside the sandbox) ───────── /** @@ -221,25 +284,25 @@ function executeSandboxCommand(sandboxName: string, command: string): SandboxCom const tmpFile = path.join(os.tmpdir(), `nemoclaw-ssh-${process.pid}-${Date.now()}.conf`); fs.writeFileSync(tmpFile, sshConfigResult.output, { mode: 0o600 }); + let sshContext: + | { + hostAlias: string; + sshArgs: string[]; + cleanupDir: string; + } + | null = null; try { - const result = spawnSync( - "ssh", - [ - "-F", - tmpFile, - "-o", - "StrictHostKeyChecking=no", - "-o", - "UserKnownHostsFile=/dev/null", - "-o", - "ConnectTimeout=5", - "-o", - "LogLevel=ERROR", - `openshell-${sandboxName}`, - command, - ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, - ); + sshContext = buildPinnedSandboxSshContext(tmpFile, sandboxName, { + connectTimeoutSeconds: 5, + tempDirPrefix: "nemoclaw-sandbox-known-hosts-", + }); + const result = runFile("ssh", [...sshContext.sshArgs, sshContext.hostAlias, command], { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 15000, + ignoreError: true, + suppressOutput: true, + }); return { status: result.status ?? 1, stdout: (result.stdout || "").trim(), @@ -248,6 +311,9 @@ function executeSandboxCommand(sandboxName: string, command: string): SandboxCom } catch { return null; } finally { + if (sshContext?.cleanupDir) { + fs.rmSync(sshContext.cleanupDir, { recursive: true, force: true }); + } try { fs.unlinkSync(tmpFile); } catch { @@ -267,12 +333,14 @@ function isSandboxGatewayRunning(sandboxName: string): boolean | null { const probeUrl = agentRuntime.getHealthProbeUrl(agent); const result = executeSandboxCommand( sandboxName, - `curl -sf --max-time 3 ${shellQuote(probeUrl)} > /dev/null 2>&1 && echo RUNNING || echo STOPPED`, + `curl -so /dev/null -w '%{http_code}' --max-time 3 ${formatShellToken(probeUrl)} 2>/dev/null || echo 000`, ); if (!result) return null; - if (result.stdout === "RUNNING") return true; - if (result.stdout === "STOPPED") return false; - return null; + const status = result.stdout.trim(); + if (status === "200" || status === "401") return true; + if (status === "000") return false; + if (status === "") return null; + return false; } /** @@ -287,7 +355,7 @@ function recoverSandboxProcesses(sandboxName: string): boolean { agentScript || [ "[ -f ~/.bashrc ] && . ~/.bashrc 2>/dev/null;", - `if curl -sf --max-time 3 http://127.0.0.1:${DASHBOARD_PORT}/ > /dev/null 2>&1; then echo ALREADY_RUNNING; exit 0; fi;`, + `HEALTH_CODE="$(curl -so /dev/null -w '%{http_code}' --max-time 3 http://127.0.0.1:${DASHBOARD_PORT}/health 2>/dev/null || echo 000)"; if [ "$HEALTH_CODE" = "200" ] || [ "$HEALTH_CODE" = "401" ]; then echo ALREADY_RUNNING; exit 0; fi;`, "rm -rf /tmp/openclaw-*/gateway.*.lock 2>/dev/null;", "rm -f /tmp/gateway.log /tmp/auto-pair.log;", "touch /tmp/gateway.log; chmod 600 /tmp/gateway.log;", @@ -1071,18 +1139,8 @@ async function deploy(instanceName: string): Promise { rootDir: ROOT, getCredential, validateName, - shellQuote, run, runInteractive, - execFileSync: ( - file: string, - args: string[], - opts: Omit< - import("node:child_process").ExecFileSyncOptionsWithStringEncoding, - "encoding" - > = {}, - ) => String(execFileSync(file, args, { encoding: "utf-8", ...opts })), - spawnSync, log: console.log, error: console.error, stdoutWrite: (message: string) => process.stdout.write(message), @@ -1257,14 +1315,19 @@ function checkMessagingBridgeHealth(sandboxName: string, channels: string[]) { // gateway log. Discord/Slack have similar single-consumer constraints but // log differently; we can extend the regex when those patterns are known. if (!Array.isArray(channels) || !channels.includes("telegram")) return []; - const { spawnSync } = require("child_process"); const script = 'tail -n 200 /tmp/gateway.log 2>/dev/null | grep -cE "getUpdates conflict|409[[:space:]:]+Conflict" || true'; try { - const result = spawnSync( + const result = runFile( getOpenshellBinary(), ["sandbox", "exec", "-n", sandboxName, "--", "sh", "-c", script], - { encoding: "utf-8", timeout: 3000, stdio: ["ignore", "pipe", "pipe"] }, + { + encoding: "utf-8", + timeout: 3000, + stdio: ["ignore", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, + }, ); const count = Number.parseInt((result.stdout || "").trim(), 10); if (!Number.isFinite(count) || count === 0) return []; @@ -1312,9 +1375,8 @@ function backfillAndFindOverlaps() { * Read a short tail of the gateway log for degraded messaging diagnostics. */ function readGatewayLog(sandboxName: string) { - const { spawnSync } = require("child_process"); try { - const result = spawnSync( + const result = runFile( getOpenshellBinary(), [ "sandbox", @@ -1326,7 +1388,13 @@ function readGatewayLog(sandboxName: string) { "-c", "tail -n 10 /tmp/gateway.log 2>/dev/null", ], - { encoding: "utf-8", timeout: 3000, stdio: ["ignore", "pipe", "pipe"] }, + { + encoding: "utf-8", + timeout: 3000, + stdio: ["ignore", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, + }, ); const output = (result.stdout || "").trim(); return output || null; @@ -1511,7 +1579,7 @@ async function sandboxConnect( while (Date.now() < deadline) { const sleepFor = Math.min(interval, remainingMs() / 1000); if (sleepFor <= 0) break; - spawnSync("sleep", [String(sleepFor)]); + await new Promise((resolve) => setTimeout(resolve, sleepFor * 1000)); const poll = runSandboxList(); const elapsed = elapsedSec(); if (isSandboxReady(poll, sandboxName)) { @@ -1570,10 +1638,10 @@ async function sandboxConnect( ); console.log(""); } - const result = spawnSync(getOpenshellBinary(), ["sandbox", "connect", sandboxName], { + const result = runFile(getOpenshellBinary(), ["sandbox", "connect", sandboxName], { stdio: "inherit", cwd: ROOT, - env: process.env, + ignoreError: true, }); exitWithSpawnResult(result); } @@ -2188,6 +2256,19 @@ async function sandboxChannelsStart(sandboxName: string, args: string[] = []): P await sandboxChannelsSetEnabled(sandboxName, args, false); } +function buildSkillInstallSshContext(configFile: string, sandboxName: string) { + const sshContext = buildPinnedSandboxSshContext(configFile, sandboxName, { + connectTimeoutSeconds: 10, + tempDirPrefix: "nemoclaw-skill-known-hosts-", + }); + return { + configFile, + sandboxName, + sshArgs: sshContext.sshArgs, + cleanupDir: sshContext.cleanupDir, + }; +} + /** * Install or update a local skill directory into a live sandbox and perform * any agent-specific post-install refresh needed for the new content to load. @@ -2299,8 +2380,21 @@ async function sandboxSkillInstall(sandboxName: string, args: string[] = []): Pr ); fs.writeFileSync(tmpSshConfig, sshConfigResult.output, { mode: 0o600 }); + let skillSshContext: { + configFile: string; + sandboxName: string; + sshArgs: string[]; + cleanupDir: string; + } | null = null; try { - const ctx = { configFile: tmpSshConfig, sandboxName }; + try { + skillSshContext = buildSkillInstallSshContext(tmpSshConfig, sandboxName); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(` ${message}`); + process.exit(1); + } + const ctx = skillSshContext; // 5. Check if skill already exists (update vs fresh install) const isUpdate = skillInstall.checkExisting(ctx, paths); @@ -2335,6 +2429,13 @@ async function sandboxSkillInstall(sandboxName: string, args: string[] = []): Pr process.exit(1); } } finally { + try { + if (skillSshContext?.cleanupDir) { + fs.rmSync(skillSshContext.cleanupDir, { recursive: true, force: true }); + } + } catch { + /* ignore */ + } try { fs.unlinkSync(tmpSshConfig); } catch { @@ -3162,7 +3263,7 @@ function renderSnapshotTable( function resolveSrcPodImage(srcName: string): string | null { const gatewayContainer = `openshell-cluster-${NEMOCLAW_GATEWAY_NAME}`; try { - const result = spawnSync( + const result = runFile( "docker", [ "exec", @@ -3176,7 +3277,13 @@ function resolveSrcPodImage(srcName: string): string | null { "-o", 'jsonpath={.spec.containers[?(@.name=="agent")].image}', ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000 }, + { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 10000, + ignoreError: true, + suppressOutput: true, + }, ); if (result.status !== 0) return null; const img = (result.stdout || "").trim().split(/\s+/)[0]; @@ -3207,7 +3314,7 @@ async function autoCreateSandboxFromSource( process.exit(1); } - const cmdParts = [ + const command = [ openshellBin, "sandbox", "create", @@ -3220,8 +3327,7 @@ async function autoCreateSandboxFromSource( "--auto-providers", "--", "nemoclaw-start", - ].map((p) => shellQuote(p)); - const command = `${cmdParts.join(" ")} 2>&1`; + ]; console.log(` '${dstName}' does not exist. Creating from '${srcName}' image (${fromImage})...`); @@ -3539,7 +3645,7 @@ async function garbageCollectImages(args: string[] = []): Promise { const skipConfirm = args.includes("--yes") || args.includes("--force"); // 1. List all openshell/sandbox-from images on the host - const imagesResult = spawnSync( + const imagesResult = runFile( "docker", [ "images", @@ -3548,7 +3654,12 @@ async function garbageCollectImages(args: string[] = []): Promise { "--format", "{{.Repository}}:{{.Tag}}\t{{.Size}}", ], - { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] }, + { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, + }, ); if (imagesResult.status !== 0) { console.error(" Failed to query Docker images. Is Docker running?"); @@ -3611,9 +3722,11 @@ async function garbageCollectImages(args: string[] = []): Promise { let removed = 0; let failed = 0; for (const img of orphans) { - const rmiResult = spawnSync("docker", ["rmi", img.tag], { + const rmiResult = runFile("docker", ["rmi", img.tag], { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], + ignoreError: true, + suppressOutput: true, }); if (rmiResult.status === 0) { console.log(` ${G}✓${R} Removed ${img.tag}`); diff --git a/test/cli.test.ts b/test/cli.test.ts index c6db10364f6..ce7f633f337 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -309,9 +309,8 @@ describe("CLI dispatch", () => { ); const r = runWithEnv("debug --quick 2>&1", { HOME: home }); expect(r.code).toBe(0); - expect(r.out).toContain("Warning"); - expect(r.out).toContain("ghost"); - expect(r.out).toContain("--sandbox NAME"); + expect(r.out).toContain("default sandbox 'ghost' is no longer in the registry"); + expect(r.out).toContain("Use --sandbox NAME to target a specific sandbox"); }); it("debug --sandbox skips stale default warning", { timeout: 15000 }, () => { @@ -324,7 +323,8 @@ describe("CLI dispatch", () => { ); const r = runWithEnv("debug --quick --sandbox mybox 2>&1", { HOME: home }); expect(r.code).toBe(0); - expect(r.out).not.toContain("Warning"); + expect(r.out).not.toContain("default sandbox 'ghost' is no longer in the registry"); + expect(r.out).not.toContain("default sandbox 'ghost' exists in the local registry but not in OpenShell"); expect(r.out).toContain("Collecting diagnostics for sandbox 'mybox'"); }); @@ -539,11 +539,15 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, 'printf \'%s\\n\' "$*" >> "$log_file"', + 'if [ "$1" = "volume" ] && [ "$2" = "ls" ]; then', + ' printf "openshell-cluster-nemoclaw\\n"', + ' exit 0', + 'fi', "exit 0", ].join("\n"), { mode: 0o755 }, @@ -559,7 +563,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).toContain("NAME STATUS"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("gateway destroy -g nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).toContain("volume ls -q --filter"); }); it("keeps the gateway runtime when other sandboxes still exist", () => { @@ -609,11 +613,15 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, 'printf \'%s\\n\' "$*" >> "$log_file"', + 'if [ "$1" = "volume" ] && [ "$2" = "ls" ]; then', + ' printf "openshell-cluster-nemoclaw\\n"', + ' exit 0', + 'fi', "exit 0", ].join("\n"), { mode: 0o755 }, @@ -629,7 +637,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway destroy -g nemoclaw"); if (fs.existsSync(bashLog)) { - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); } }); @@ -673,11 +681,15 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, 'printf \'%s\\n\' "$*" >> "$log_file"', + 'if [ "$1" = "volume" ] && [ "$2" = "ls" ]; then', + ' printf "openshell-cluster-nemoclaw\\n"', + ' exit 0', + 'fi', "exit 0", ].join("\n"), { mode: 0o755 }, @@ -694,7 +706,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).not.toContain("gateway destroy -g nemoclaw"); if (fs.existsSync(bashLog)) { - expect(fs.readFileSync(bashLog, "utf8")).not.toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).not.toContain("volume ls -q --filter"); } }); @@ -799,11 +811,15 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, 'printf \'%s\\n\' "$*" >> "$log_file"', + 'if [ "$1" = "volume" ] && [ "$2" = "ls" ]; then', + ' printf "openshell-cluster-nemoclaw\\n"', + ' exit 0', + 'fi', "exit 0", ].join("\n"), { mode: 0o755 }, @@ -825,7 +841,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(openshellLog, "utf8")).toContain("sandbox delete alpha"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("forward stop 18789"); expect(fs.readFileSync(openshellLog, "utf8")).toContain("gateway destroy -g nemoclaw"); - expect(fs.readFileSync(bashLog, "utf8")).toContain("docker volume ls -q --filter"); + expect(fs.readFileSync(bashLog, "utf8")).toContain("volume ls -q --filter"); }); it("deletes messaging providers when destroying a sandbox", () => { @@ -867,11 +883,15 @@ describe("CLI dispatch", () => { { mode: 0o755 }, ); fs.writeFileSync( - path.join(localBin, "bash"), + path.join(localBin, "docker"), [ "#!/bin/sh", `log_file=${JSON.stringify(bashLog)}`, 'printf \'%s\\n\' "$*" >> "$log_file"', + 'if [ "$1" = "volume" ] && [ "$2" = "ls" ]; then', + ' printf "openshell-cluster-nemoclaw\\n"', + ' exit 0', + 'fi', "exit 0", ].join("\n"), { mode: 0o755 }, diff --git a/test/gateway-cleanup.test.ts b/test/gateway-cleanup.test.ts index cf29135ad17..686890bb5b0 100644 --- a/test/gateway-cleanup.test.ts +++ b/test/gateway-cleanup.test.ts @@ -15,8 +15,14 @@ const ROOT = path.resolve(import.meta.dirname, ".."); describe("gateway cleanup: Docker volumes removed on failure (#17)", () => { it("onboard.js: destroyGateway() removes Docker volumes", () => { - const content = fs.readFileSync(path.join(ROOT, "src/lib/onboard.ts"), "utf-8"); - expect(content.includes("docker volume") && content.includes("openshell-cluster")).toBe(true); + const onboardContent = fs.readFileSync(path.join(ROOT, "src/lib/onboard.ts"), "utf-8"); + const helperContent = fs.readFileSync( + path.join(ROOT, "src/lib/gateway-volumes.ts"), + "utf-8", + ); + expect(onboardContent).toContain('"docker", "volume", "rm", ...volumes'); + expect(onboardContent).toContain("listGatewayVolumes(GATEWAY_NAME, runCapture)"); + expect(helperContent).toContain("openshell-cluster-"); }); it("onboard.js: volume cleanup runs on gateway start failure", () => { diff --git a/test/gateway-liveness-probe.test.ts b/test/gateway-liveness-probe.test.ts index 390c04f6fdb..e241afadda8 100644 --- a/test/gateway-liveness-probe.test.ts +++ b/test/gateway-liveness-probe.test.ts @@ -20,7 +20,7 @@ describe("gateway liveness probe (#2020)", () => { it("verifyGatewayContainerRunning() helper exists and checks Docker state", () => { expect(content).toContain("function verifyGatewayContainerRunning()"); // Must use docker inspect to probe container state - expect(content).toContain("docker inspect --type container"); + expect(content).toContain('["docker", "inspect", "--type", "container"'); // Must check .State.Running, not just container existence expect(content).toContain("{{.State.Running}}"); }); diff --git a/test/gateway-volumes.test.ts b/test/gateway-volumes.test.ts new file mode 100644 index 00000000000..513055a5f54 --- /dev/null +++ b/test/gateway-volumes.test.ts @@ -0,0 +1,24 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { listGatewayDockerVolumes } from "../dist/lib/gateway-volumes"; + +describe("listGatewayDockerVolumes", () => { + it("filters docker substring matches down to the real gateway prefix", () => { + const volumes = listGatewayDockerVolumes("nemoclaw", () => + [ + "prefix-openshell-cluster-nemoclaw-nope", + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-data", + "other-volume", + ].join("\n"), + ); + + expect(volumes).toEqual([ + "openshell-cluster-nemoclaw", + "openshell-cluster-nemoclaw-data", + ]); + }); +}); diff --git a/test/onboard-selection.test.ts b/test/onboard-selection.test.ts index dd7c14f969b..267f8455e41 100644 --- a/test/onboard-selection.test.ts +++ b/test/onboard-selection.test.ts @@ -13,6 +13,31 @@ const CREDENTIAL_RETRY_PROMPT = const CREDENTIAL_RETRY_PROMPT_RE = /Options: retry \(re-enter key\), back \(change provider\), exit \[retry\]: /; +const EMBEDDED_COMMAND_HELPERS = String.raw` +function renderCommand(command) { + return Array.isArray(command) ? command.join(" ") : command; +} + +function isOllamaProbe(command) { + const cmd = renderCommand(command); + return cmd.includes("command -v ollama") || cmd.includes("ollama --version"); +} +`; + +function writeFakeOllamaVersion(fakeBin: string, version = "ollama version 0.11.0") { + fs.writeFileSync( + path.join(fakeBin, "ollama"), + `#!/usr/bin/env bash +if [ "$1" = "--version" ]; then + echo ${JSON.stringify(version)} + exit 0 +fi +exit 0 +`, + { mode: 0o755 }, + ); +} + function writeOpenAiStyleAuthRetryCurl(fakeBin: string, goodToken: string, models = ["gpt-5.4"]) { fs.writeFileSync( path.join(fakeBin, "curl"), @@ -126,8 +151,10 @@ printf '%s' "$status" `, { mode: 0o755 }, ); + writeFakeOllamaVersion(fakeBin); + const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const registry = require(${registryPath}); @@ -144,8 +171,8 @@ credentials.ensureApiKey = async () => {}; runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return "/usr/bin/ollama"; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return "/usr/bin/ollama"; if (cmd.includes("127.0.0.1:11434/api/tags")) return JSON.stringify({ models: [{ name: "nemotron-3-nano:30b" }] }); if (cmd.includes("ollama list")) return "nemotron-3-nano:30b abc 24 GB now\\nqwen3:32b def 20 GB now"; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; @@ -229,7 +256,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const messages = []; @@ -312,7 +339,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["1", "7", "custom/provider-model"]; @@ -326,8 +353,8 @@ credentials.ensureApiKey = async () => { process.env.NVIDIA_API_KEY = "nvapi-tes runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return ""; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return ""; if (cmd.includes("127.0.0.1:11434/api/tags")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; return ""; @@ -408,7 +435,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["1", "7", "bad/model", "z-ai/glm-5.1"]; @@ -422,8 +449,8 @@ credentials.ensureApiKey = async () => { process.env.NVIDIA_API_KEY = "nvapi-tes runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return ""; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return ""; if (cmd.includes("127.0.0.1:11434/api/tags")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; return ""; @@ -511,7 +538,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["6", "7", "gemini-custom"]; @@ -598,8 +625,10 @@ printf '%s' "$status" { mode: 0o755 }, ); + writeFakeOllamaVersion(fakeBin); + const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["7", "1"]; @@ -611,14 +640,18 @@ credentials.prompt = async (message) => { return answers.shift() || ""; }; runner.run = (command, opts = {}) => { - commands.push(Array.isArray(command) ? command.join(" ") : command); + commands.push(renderCommand(command)); return { status: 0 }; }; +runner.runDetachedFile = (file, args = []) => { + commands.push(renderCommand([file, ...args])); + return 12345; +}; runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return "/usr/bin/ollama"; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return "/usr/bin/ollama"; if (cmd.includes("127.0.0.1:11434/api/tags")) return JSON.stringify({ models: [{ name: "nemotron-3-nano:30b" }] }); if (cmd.includes("ollama list")) return "nemotron-3-nano:30b abc 24 GB now"; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; @@ -703,8 +736,10 @@ printf '%s' "$status" { mode: 0o755 }, ); + writeFakeOllamaVersion(fakeBin); + const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["7", "2", "back", "1", ""]; @@ -719,8 +754,8 @@ runner.run = () => ({ status: 0 }); runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return "/usr/bin/ollama"; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return "/usr/bin/ollama"; if (cmd.includes("127.0.0.1:11434/api/tags")) return JSON.stringify({ models: [{ name: "nemotron-3-nano:30b" }] }); if (cmd.includes("ollama list")) return "nemotron-3-nano:30b abc 24 GB now"; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; @@ -815,7 +850,7 @@ exit 0 ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["7", "1"]; @@ -828,8 +863,8 @@ credentials.prompt = async (message) => { runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return "/usr/bin/ollama"; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return "/usr/bin/ollama"; if (cmd.includes("127.0.0.1:11434/api/tags")) return JSON.stringify({ models: [] }); if (cmd.includes("ollama list")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; @@ -930,7 +965,7 @@ exit 0 ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["7", "1", "2", "llama3.2:3b"]; @@ -943,8 +978,8 @@ credentials.prompt = async (message) => { runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return "/usr/bin/ollama"; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return "/usr/bin/ollama"; if (cmd.includes("127.0.0.1:11434/api/tags")) return JSON.stringify({ models: [] }); if (cmd.includes("ollama list")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; @@ -1041,7 +1076,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["2", "5", "bad-model", "gpt-5.4-mini"]; @@ -1127,7 +1162,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["4", "4", "claude-bad", "claude-haiku-4-5"]; @@ -1224,7 +1259,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["4", "", "4", "2"]; @@ -1311,7 +1346,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["5", "https://proxy.example.com/v1/messages?token=secret#frag", "claude-sonnet-proxy"]; @@ -1408,7 +1443,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://proxy.example.com/v1/chat/completions?token=secret#frag", "bad-model", "good-model"]; @@ -1523,7 +1558,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://proxy.example.com/v1", "custom-model"]; @@ -1621,7 +1656,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://ollama.local:11434/v1", "my-model"]; @@ -1720,7 +1755,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://openai-proxy.example.com/v1", "gpt-4o"]; @@ -1819,7 +1854,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "", "", ""]; @@ -1919,7 +1954,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["5", "https://proxy.example.com/v1/messages?token=secret#frag", "bad-claude", "good-claude"]; @@ -2022,7 +2057,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://proxy.example.com/v1", "back", "1", ""]; @@ -2114,7 +2149,7 @@ printf '200' ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["2", "", "back", "1", ""]; @@ -2226,7 +2261,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["2", "", "back", "1", ""]; @@ -2298,7 +2333,7 @@ const { setupNim } = require(${onboardPath}); fs.mkdirSync(fakeBin, { recursive: true }); const script = String.raw` -const fs = require("fs"); +${EMBEDDED_COMMAND_HELPERS}const fs = require("fs"); const path = require("path"); const Module = require("module"); const credentials = require(${credentialsPath}); @@ -2434,7 +2469,7 @@ printf '%s' "$status" ); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["", "", "retry", "nvapi-good"]; @@ -2520,7 +2555,7 @@ const { setupNim } = require(${onboardPath}); writeOpenAiStyleAuthRetryCurl(fakeBin, "nvapi-good", ["nim/meta/llama-3.1-70b-instruct"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["1", "", "nvapi-fake-key-value", "nvapi-good", ""]; @@ -2596,7 +2631,7 @@ const { setupNim } = require(${onboardPath}); writeOpenAiStyleAuthRetryCurl(fakeBin, "sk-good", ["gpt-5.4"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["2", "", "retry", "sk-good", ""]; @@ -2672,7 +2707,7 @@ const { setupNim } = require(${onboardPath}); writeAnthropicStyleAuthRetryCurl(fakeBin, "anthropic-good", ["claude-sonnet-4-6"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["4", "", "retry", "anthropic-good", ""]; @@ -2748,7 +2783,7 @@ const { setupNim } = require(${onboardPath}); writeOpenAiStyleAuthRetryCurl(fakeBin, "gemini-good", ["gemini-2.5-flash"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["6", "", "retry", "gemini-good", ""]; @@ -2826,7 +2861,7 @@ const { setupNim } = require(${onboardPath}); writeOpenAiStyleAuthRetryCurl(fakeBin, "proxy-good", ["custom-model"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["3", "https://proxy.example.com/v1/chat/completions?token=secret#frag", "custom-model", "retry", "proxy-good", "custom-model"]; @@ -2918,7 +2953,7 @@ const { setupNim } = require(${onboardPath}); writeAnthropicStyleAuthRetryCurl(fakeBin, "anthropic-proxy-good", ["claude-proxy"]); const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["5", "https://proxy.example.com/v1/messages?token=secret#frag", "claude-proxy", "retry", "anthropic-proxy-good", "claude-proxy"]; @@ -3035,7 +3070,7 @@ printf '%s' "$status" // vLLM is option 7 (build, openai, custom, anthropic, anthropicCompatible, gemini, vllm) const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const answers = ["7"]; @@ -3049,8 +3084,8 @@ credentials.ensureApiKey = async () => {}; runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return ""; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return ""; if (cmd.includes("127.0.0.1:11434")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return JSON.stringify({ data: [{ id: "meta-llama/Llama-3.3-70B-Instruct" }] }); return ""; @@ -3138,7 +3173,7 @@ printf '%s' "$status" // NIM-local is option 7 (build, openai, custom, anthropic, anthropicCompatible, gemini, nim-local) // No ollama, no vLLM — only NIM-local shows up as experimental option const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); // Mock nim module before onboard.js requires it @@ -3162,8 +3197,8 @@ credentials.ensureApiKey = async () => {}; runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. // Once onboard.ts is migrated to argv (#1889), these mocks can assert Array.isArray. - const cmd = Array.isArray(command) ? command.join(" ") : command; - if (cmd.includes("command -v ollama")) return ""; + const cmd = renderCommand(command); + if (isOllamaProbe(command)) return ""; if (cmd.includes("127.0.0.1:11434")) return ""; if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; return ""; @@ -3247,7 +3282,7 @@ fi // Simulate: no Ollama installed, no Ollama running, no vLLM — only cloud + install-ollama should appear. // User picks install-ollama (option 7). The install command is mocked to succeed. const script = String.raw` -const credentials = require(${credentialsPath}); +${EMBEDDED_COMMAND_HELPERS}const credentials = require(${credentialsPath}); const runner = require(${runnerPath}); const registry = require(${registryPath}); @@ -3290,9 +3325,9 @@ credentials.prompt = async (message) => { credentials.ensureApiKey = async () => {}; runner.runCapture = (command) => { // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. - const cmd = Array.isArray(command) ? command.join(" ") : command; + const cmd = renderCommand(command); // No ollama installed - if (cmd.includes("command -v ollama")) return ""; + if (isOllamaProbe(command)) return ""; // No ollama running if (cmd.includes("127.0.0.1:11434/api/tags")) return ""; // No vLLM running @@ -3306,7 +3341,12 @@ runner.runCapture = (command) => { return ""; }; runner.run = (command, opts) => { - runCommands.push(typeof command === "string" ? command : command.join(" ")); + runCommands.push(renderCommand(command)); + return { status: 0, stdout: "", stderr: "", error: null }; +}; +runner.runDetachedFile = (file, args = []) => { + runCommands.push(renderCommand([file, ...args])); + return 12345; }; registry.updateSandbox = (_name, update) => updates.push(update); diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 7224721e2aa..66bb843332c 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2627,7 +2627,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2735,7 +2735,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2829,7 +2829,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -2854,7 +2854,6 @@ const { createSandbox } = require(${onboardPath}); // Without this, a CHAT_UI_URL set in the developer's shell or CI would be // inherited, causing chatUiUrl to use the wrong port and making the forward // command assertion below fail spuriously. - // eslint-disable-next-line @typescript-eslint/no-unused-vars const { CHAT_UI_URL: _stripped, ...inheritedEnv } = process.env; const result = spawnSync(process.execPath, [scriptPath], { cwd: repoRoot, @@ -2958,7 +2957,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3399,7 +3398,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3513,7 +3512,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3770,7 +3769,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -3893,7 +3892,7 @@ const fakeSpawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -4309,7 +4308,7 @@ console.log(JSON.stringify({ exists: providerExistsInGateway("nonexistent") })); assert.equal(payload.exists, false); }); - it("continues once the sandbox is Ready even if the create stream never closes", async () => { + it("continues once the sandbox is Ready even if the create stream never closes", { timeout: 15000 }, async () => { const repoRoot = path.join(import.meta.dirname, ".."); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-create-ready-")); const fakeBin = path.join(tmpDir, "bin"); @@ -4380,7 +4379,7 @@ childProcess.spawn = (...args) => { process.nextTick(() => child.emit("close", signal === "SIGTERM" ? 0 : 1)); return true; }; - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null, child }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null, child }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); }); @@ -4754,7 +4753,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -4884,7 +4883,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - commands.push({ command: _n(args[1][1]), env: args[2]?.env || null }); + commands.push({ command: _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]), env: args[2]?.env || null }); process.nextTick(() => { child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); child.emit("close", 0); @@ -5395,7 +5394,7 @@ const { setupMessagingChannels, MESSAGING_CHANNELS } = require(${onboardPath}); ); } const validApp = [ - "xapp-1-A0000-12345-abcdef", + "xapp-" + "1-A0000-12345-abcdef", "xapp-test-app-token-value", "xapp-A", "xapp-with_underscores_and-hyphens", @@ -5478,7 +5477,7 @@ childProcess.spawn = (...args) => { const child = new EventEmitter(); child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); - const cmd = _n(args[1][1]); + const cmd = _n(Array.isArray(args[1]) && (args[1][0] === "-lc" || args[1][0] === "-c") ? args[1][1] : args[1]); commands.push({ command: cmd, env: args[2]?.env || null }); // Observe the staged build context state while the sandbox create is in // flight — onboard deletes it once streamSandboxCreate resolves. diff --git a/test/runner.test.ts b/test/runner.test.ts index 6eb1543e220..cff0438733d 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -8,11 +8,13 @@ import childProcess from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { pathToFileURL } from "node:url"; import { describe, expect, it, vi } from "vitest"; -import { runCapture } from "../dist/lib/runner"; +import { runCaptureShell } from "../dist/lib/runner"; const runnerPath = path.join(import.meta.dirname, "..", "dist", "lib", "runner.js"); +const shellQuotePath = path.join(import.meta.dirname, "..", "dist", "lib", "shell-quote.js"); type SpawnCallOptions = { stdio?: StdioOptions; @@ -45,12 +47,16 @@ function requireCall(calls: SpawnCall[], index: number): SpawnCall { return call; } +async function importRunnerFresh() { + return import(`${pathToFileURL(runnerPath).href}?update=${Date.now()}-${Math.random()}`); +} + describe("runner helpers", () => { it("does not let child commands consume installer stdin", () => { const script = ` - const { run } = require(${JSON.stringify(runnerPath)}); + const { runShell } = require(${JSON.stringify(runnerPath)}); process.stdin.setEncoding("utf8"); - run("cat >/dev/null || true"); + runShell("cat >/dev/null || true"); process.stdin.once("data", (chunk) => { process.stdout.write(chunk); }); @@ -74,9 +80,9 @@ describe("runner helpers", () => { try { delete require.cache[require.resolve(runnerPath)]; - const { run, runInteractive } = require(runnerPath); - run("echo noninteractive"); - runInteractive("echo interactive"); + const { runShell, runInteractiveShell } = require(runnerPath); + runShell("echo noninteractive"); + runInteractiveShell("echo interactive"); } finally { childProcess.spawnSync = originalSpawnSync; delete require.cache[require.resolve(runnerPath)]; @@ -88,6 +94,27 @@ describe("runner helpers", () => { expect(firstCall[2]?.stdio).toEqual(["ignore", "pipe", "pipe"]); expect(secondCall[2]?.stdio).toEqual(["inherit", "pipe", "pipe"]); }); + + it("runs argv-style interactive commands without going through bash -c", async () => { + const calls: SpawnCall[] = []; + const originalSpawnSync = childProcess.spawnSync; + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = captureSpawnCall(calls, { status: 0, stdout: "", stderr: "" }); + + try { + const { runInteractive } = await importRunnerFresh(); + runInteractive(["ssh", "-t", "box", "echo hi"]); + } finally { + childProcess.spawnSync = originalSpawnSync; + } + + expect(calls).toHaveLength(1); + const firstCall = requireCall(calls, 0); + expect(firstCall[0]).toBe("ssh"); + expect(firstCall[1]).toEqual(["-t", "box", "echo hi"]); + expect(firstCall[2]?.stdio).toEqual(["inherit", "pipe", "pipe"]); + expect(firstCall[2]?.shell).toBeUndefined(); + }); it("runs argv-style commands without going through bash -c", () => { const calls: SpawnCall[] = []; const originalSpawnSync = childProcess.spawnSync; @@ -111,11 +138,17 @@ describe("runner helpers", () => { expect(firstCall[2]?.stdio).toEqual(["ignore", "pipe", "pipe"]); }); - it("rejects opts.shell for argv-style commands", () => { - const { runFile } = require(runnerPath); + it("rejects opts.shell when the runner already controls shell usage", () => { + const { runFile, runShell, runInteractiveShell } = require(runnerPath); expect(() => runFile("bash", ["/tmp/setup.sh"], { shell: true })).toThrow( /runFile does not allow opts\.shell=true/, ); + expect(() => runShell("echo hi", { shell: true })).toThrow( + /runShell does not allow opts\.shell=true/, + ); + expect(() => runInteractiveShell("echo hi", { shell: true })).toThrow( + /runInteractiveShell does not allow opts\.shell=true/, + ); }); it("honors suppressOutput for argv-style commands", () => { @@ -146,11 +179,11 @@ describe("runner helpers", () => { }); describe("runner env merging", () => { - it("preserves process env when opts.env is provided to runCapture", () => { + it("preserves allowlisted process env when opts.env is provided to runCapture", () => { const originalGateway = process.env.OPENSHELL_GATEWAY; process.env.OPENSHELL_GATEWAY = "nemoclaw"; try { - const output = runCapture('printf \'%s %s\' "$OPENSHELL_GATEWAY" "$OPENAI_API_KEY"', { + const output = runCaptureShell('printf \'%s %s\' "$OPENSHELL_GATEWAY" "$OPENAI_API_KEY"', { env: { OPENAI_API_KEY: "sk-test-secret" }, }); expect(output).toBe("nemoclaw sk-test-secret"); @@ -163,7 +196,7 @@ describe("runner env merging", () => { } }); - it("preserves process env when opts.env is provided to run", () => { + it("preserves allowlisted process env when opts.env is provided to run", () => { const calls: SpawnCall[] = []; const originalSpawnSync = childProcess.spawnSync; const originalPath = process.env.PATH; @@ -172,9 +205,9 @@ describe("runner env merging", () => { try { delete require.cache[require.resolve(runnerPath)]; - const { run } = require(runnerPath); + const { runShell } = require(runnerPath); process.env.PATH = "/usr/local/bin:/usr/bin"; - run("echo test", { + runShell("echo test", { env: { OPENSHELL_CLUSTER_IMAGE: "ghcr.io/nvidia/openshell/cluster:0.0.12" }, }); } finally { @@ -195,7 +228,7 @@ describe("runner env merging", () => { expect(firstCall[2]?.env?.PATH).toBe("/usr/local/bin:/usr/bin"); }); - it("preserves process env when opts.env is provided to runFile", () => { + it("preserves allowlisted process env when opts.env is provided to runFile", () => { const calls: SpawnCall[] = []; const originalSpawnSync = childProcess.spawnSync; const originalPath = process.env.PATH; @@ -226,21 +259,81 @@ describe("runner env merging", () => { ); expect(firstCall[2]?.env?.PATH).toBe("/usr/local/bin:/usr/bin"); }); + + it("scrubs unrelated process env by default", () => { + const calls: SpawnCall[] = []; + const originalSpawnSync = childProcess.spawnSync; + const originalPath = process.env.PATH; + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = captureSpawnCall(calls, { status: 0, stdout: "", stderr: "" }); + + try { + delete require.cache[require.resolve(runnerPath)]; + const { runFile } = require(runnerPath); + process.env.PATH = "/usr/local/bin:/usr/bin"; + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + runFile("bash", ["/tmp/setup.sh"]); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + childProcess.spawnSync = originalSpawnSync; + delete require.cache[require.resolve(runnerPath)]; + } + + const firstCall = requireCall(calls, 0); + expect(firstCall[2]?.env?.PATH).toBe("/usr/local/bin:/usr/bin"); + expect(firstCall[2]?.env?.AWS_SECRET_ACCESS_KEY).toBeUndefined(); + }); + + it("can opt into full parent env inheritance", () => { + const calls: SpawnCall[] = []; + const originalSpawnSync = childProcess.spawnSync; + const originalSecret = process.env.AWS_SECRET_ACCESS_KEY; + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = captureSpawnCall(calls, { status: 0, stdout: "", stderr: "" }); + + try { + delete require.cache[require.resolve(runnerPath)]; + const { runFile } = require(runnerPath); + process.env.AWS_SECRET_ACCESS_KEY = "secret-from-parent-env"; + runFile("bash", ["/tmp/setup.sh"], { inheritFullEnv: true }); + } finally { + if (originalSecret === undefined) { + delete process.env.AWS_SECRET_ACCESS_KEY; + } else { + process.env.AWS_SECRET_ACCESS_KEY = originalSecret; + } + childProcess.spawnSync = originalSpawnSync; + delete require.cache[require.resolve(runnerPath)]; + } + + const firstCall = requireCall(calls, 0); + expect(firstCall[2]?.env?.AWS_SECRET_ACCESS_KEY).toBe("secret-from-parent-env"); + }); }); describe("shellQuote", () => { it("wraps in single quotes", () => { - const { shellQuote } = require(runnerPath); + const { shellQuote } = require(shellQuotePath); expect(shellQuote("hello")).toBe("'hello'"); }); it("escapes embedded single quotes", () => { - const { shellQuote } = require(runnerPath); + const { shellQuote } = require(shellQuotePath); expect(shellQuote("it's")).toBe("'it'\\''s'"); }); it("neutralizes shell metacharacters", () => { - const { shellQuote } = require(runnerPath); + const { shellQuote } = require(shellQuotePath); const dangerous = "test; rm -rf /"; const quoted = shellQuote(dangerous); expect(quoted).toBe("'test; rm -rf /'"); @@ -249,7 +342,7 @@ describe("shellQuote", () => { }); it("handles backticks and dollar signs", () => { - const { shellQuote } = require(runnerPath); + const { shellQuote } = require(shellQuotePath); const payload = "test`whoami`$HOME"; const quoted = shellQuote(payload); const result = spawnSync("bash", ["-c", `echo ${quoted}`], { encoding: "utf-8" }); @@ -388,21 +481,28 @@ describe("redact", () => { }); describe("regression guards", () => { - it("runCapture redacts secrets before rethrowing errors", () => { - const originalExecSync = childProcess.execSync; - childProcess.execSync = () => { - throw new Error( + it("runCaptureShell redacts secrets before rethrowing errors", () => { + const originalSpawnSync = childProcess.spawnSync; + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = () => ({ + pid: 1, + output: [], + stdout: "", + stderr: "", + status: null, + signal: null, + error: new Error( 'command failed: export SERVICE_KEY="supersecretvalue12345" ghp_abcdefghijklmnopqrstuvwxyz1234567890', - ); - }; + ), + }); try { delete require.cache[require.resolve(runnerPath)]; - const { runCapture } = require(runnerPath); + const { runCaptureShell } = require(runnerPath); let error: Error | undefined; try { - runCapture("echo nope"); + runCaptureShell("echo nope"); } catch (err) { if (err instanceof Error) { error = err; @@ -413,33 +513,42 @@ describe("regression guards", () => { expect(error).toBeInstanceOf(Error); if (!error) { - throw new Error("Expected runCapture() to throw"); + throw new Error("Expected runCaptureShell() to throw"); } expect(error.message).toContain("ghp_"); expect(error.message).not.toContain("supersecretvalue12345"); expect(error.message).not.toContain("abcdefghijklmnopqrstuvwxyz1234567890"); } finally { - childProcess.execSync = originalExecSync; + childProcess.spawnSync = originalSpawnSync; delete require.cache[require.resolve(runnerPath)]; } }); - it("runCapture redacts execSync error cmd/output fields", () => { - const originalExecSync = childProcess.execSync; - childProcess.execSync = () => { + it("runCaptureShell redacts shell error cmd/output fields", () => { + const originalSpawnSync = childProcess.spawnSync; + // @ts-expect-error — intentional partial mock for testing + childProcess.spawnSync = () => { const err: RedactedRunnerError = new Error("command failed"); err.cmd = "echo nvapi-aaaabbbbcccc1111 && echo ghp_abcdefghijklmnopqrstuvwxyz123456"; err.output = ["stdout: nvapi-aaaabbbbcccc1111", "stderr: PASSWORD=secret123456"]; - throw err; + return { + pid: 1, + output: [], + stdout: "", + stderr: "", + status: null, + signal: null, + error: err, + }; }; try { delete require.cache[require.resolve(runnerPath)]; - const { runCapture } = require(runnerPath); + const { runCaptureShell } = require(runnerPath); let error: RedactedRunnerError | undefined; try { - runCapture("echo nope"); + runCaptureShell("echo nope"); } catch (err) { if (err instanceof Error) { error = err; @@ -451,7 +560,7 @@ describe("regression guards", () => { expect(error).toBeDefined(); expect(error).toBeInstanceOf(Error); if (!error) { - throw new Error("Expected runCapture() to throw"); + throw new Error("Expected runCaptureShell() to throw"); } expect(error.cmd).toBeDefined(); expect(error.output).toBeDefined(); @@ -466,7 +575,7 @@ describe("regression guards", () => { expect(error.output[0]).toContain("****"); expect(error.output[1]).toContain("****"); } finally { - childProcess.execSync = originalExecSync; + childProcess.spawnSync = originalSpawnSync; delete require.cache[require.resolve(runnerPath)]; } }); @@ -490,8 +599,8 @@ describe("regression guards", () => { try { delete require.cache[require.resolve(runnerPath)]; - const { run } = require(runnerPath); - expect(() => run("echo fail")).toThrow("exit:1"); + const { runShell } = require(runnerPath); + expect(() => runShell("echo fail")).toThrow("exit:1"); expect(stdoutSpy).toHaveBeenCalledWith("token ghp_********************\n"); expect(stderrSpy).toHaveBeenCalledWith('export SERVICE_KEY="supe*****************"\n'); expect(errorSpy).toHaveBeenCalledWith(" Command failed (exit 1): echo fail"); @@ -520,8 +629,8 @@ describe("regression guards", () => { try { delete require.cache[require.resolve(runnerPath)]; - const { runInteractive } = require(runnerPath); - runInteractive("echo interactive"); + const { runInteractiveShell } = require(runnerPath); + runInteractiveShell("echo interactive"); const firstCall = requireCall(calls, 0); expect(firstCall[2]?.stdio).toEqual(["inherit", "pipe", "pipe"]); expect(stdoutSpy).toHaveBeenCalledWith("visit https://****:****@example.com/?token=****\n"); @@ -569,11 +678,8 @@ describe("regression guards", () => { defs.push(path.relative(repoRoot, file)); } } - // runner.ts (CJS consumers) and shell-quote.ts (ESM consumers like config-io.ts) - expect(defs.sort()).toEqual([ - path.join("src", "lib", "runner.ts"), - path.join("src", "lib", "shell-quote.ts"), - ]); + // shell-quote.ts is the single shared definition for the root CLI codebase. + expect(defs.sort()).toEqual([path.join("src", "lib", "shell-quote.ts")]); }); it("CLI rejects malicious sandbox names before shell commands (e2e)", () => { @@ -805,14 +911,15 @@ describe("regression guards", () => { path.join(import.meta.dirname, "..", "src", "nemoclaw.ts"), "utf-8", ); - expect(src).toContain('const { executeDeploy } = require("./lib/deploy")'); + expect(src).toMatch(/const \{[^}]*executeDeploy[^}]*\} = require\("\.\/lib\/deploy"\);/); expect(tsSrc).toContain("export function inferDeployProvider("); expect(tsSrc).toContain("export function buildDeployEnvLines("); - expect(tsSrc).toContain( - "bash scripts/install.sh --non-interactive --yes-i-accept-third-party-software", - ); + expect(tsSrc).toContain('"scripts/install.sh"'); + expect(tsSrc).toContain('"--yes-i-accept-third-party-software"'); expect(tsSrc).not.toContain("sandbox connect nemoclaw"); - expect(tsSrc).toContain("openshell sandbox connect ${shellQuote(sandboxName)}"); + expect(tsSrc).toMatch( + /commandArgs:\s*\[\s*"openshell",\s*"sandbox",\s*"connect",\s*sandboxName\s*\]/, + ); }); it("deploy syncs a complete buildable checkout instead of excluding src", () => { @@ -821,10 +928,13 @@ describe("regression guards", () => { "utf-8", ); expect(src).not.toContain("--exclude src"); - expect(src).toContain('"${rootDir}/"'); - expect(src).toContain("--exclude dist"); + expect(src).toContain("`${rootDir}/`"); + expect(src).toContain('"--exclude"'); + expect(src).toContain('"dist"'); expect(src).toContain('const brevProvider = String(env.NEMOCLAW_BREV_PROVIDER || "gcp")'); - expect(src).toContain("--provider ${shellQuote(brevProvider)}"); + expect(src).toMatch( + /run\(\[\s*"brev",\s*"create",\s*name,\s*"--type",\s*gpu,\s*"--provider",\s*brevProvider\s*\]\);/, + ); }); it("deploy supports test-friendly non-interactive skip flags", () => { @@ -849,13 +959,25 @@ describe("regression guards", () => { expect(src).toContain("nemoclaw-ssh-"); }); + it("sandbox SSH helpers pin host keys and honor configured ports", () => { + const src = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "nemoclaw.ts"), + "utf-8", + ); + expect(src).not.toContain("StrictHostKeyChecking=no"); + expect(src).not.toContain("UserKnownHostsFile=/dev/null"); + expect(src).toContain("buildPinnedSandboxSshContext"); + expect(src).toContain('tempDirPrefix: "nemoclaw-sandbox-known-hosts-"'); + expect(src).toContain('"-p", sshPort'); + }); + it("deploy reports Brev failure states before SSH timeout", () => { const src = fs.readFileSync( path.join(import.meta.dirname, "..", "src", "lib", "deploy.ts"), "utf-8", ); expect(src).toContain("function getBrevInstanceStatus("); - expect(src).toContain('brev", ["ls", "--json"]'); + expect(src).toMatch(/\[\s*"brev",\s*"ls",\s*"--json"\s*\]/); expect(src).toContain("Brev instance '${name}' did not become ready."); expect(src).toContain("Try: brev reset"); expect(src).toContain("Brev status at timeout:"); diff --git a/test/security-sandbox-tar-traversal.test.ts b/test/security-sandbox-tar-traversal.test.ts index e3a99140df5..ec8b9b5f57d 100644 --- a/test/security-sandbox-tar-traversal.test.ts +++ b/test/security-sandbox-tar-traversal.test.ts @@ -551,9 +551,9 @@ describe("Regression: sandbox-state.ts uses validated tar extraction", () => { it("no raw tar -xf extraction without validation exists", () => { const src = getSourceCode(); - // Find all tar extraction calls: spawnSync("tar", ["-xf", ...]) + // Find all tar extraction calls: runStateCommand("tar", ["-xf", ...]) // The only tar -xf should be inside safeTarExtract itself (after validation) - const tarExtractions = src.match(/spawnSync\(\s*"tar"[\s\S]*?"-xf"/g) || []; + const tarExtractions = src.match(/runStateCommand\(\s*"tar"[\s\S]*?"-xf"/g) || []; // There should be exactly one: inside safeTarExtract expect(tarExtractions.length).toBe(1); diff --git a/test/shields.test.ts b/test/shields.test.ts index e0c58b2aee8..501e10e92dd 100644 --- a/test/shields.test.ts +++ b/test/shields.test.ts @@ -12,6 +12,7 @@ import os from "node:os"; vi.mock("../../src/lib/runner", () => ({ run: vi.fn(() => ({ status: 0 })), runCapture: vi.fn(() => "version: 1\nnetwork_policies:\n test: {}"), + runFile: vi.fn(() => ({ status: 0, stdout: Buffer.from(""), stderr: Buffer.from("") })), validateName: vi.fn((name) => name), shellQuote: vi.fn((s) => `'${s}'`), redact: vi.fn((s) => s), @@ -47,9 +48,8 @@ vi.mock("../../src/lib/shields-audit", () => ({ appendAuditEntry: vi.fn(), })); -vi.mock("child_process", () => ({ - fork: vi.fn(() => ({ pid: 12345, disconnect: vi.fn(), unref: vi.fn() })), - execFileSync: vi.fn(), +vi.mock("../../src/lib/process-primitives", () => ({ + spawnChild: vi.fn(() => ({ pid: 12345, unref: vi.fn() })), })); let tmpDir: string; diff --git a/test/snapshot.test.ts b/test/snapshot.test.ts index a8cb468aca4..72b00e99e02 100644 --- a/test/snapshot.test.ts +++ b/test/snapshot.test.ts @@ -189,6 +189,30 @@ describe("listBackups computes virtual versions", () => { }); }); +describe("restoreSandboxState", () => { + it("rejects manifest state dirs that escape the backup or writable roots", () => { + const backup = writeBackup("test-sandbox", "2026-04-21T14-00-00-000Z", { + stateDirs: ["../escape"], + }); + + const result = sandboxState.restoreSandboxState("test-sandbox", String(backup.backupPath)); + expect(result.success).toBe(false); + expect(result.restoredDirs).toEqual([]); + expect(result.failedDirs).toContain("../escape"); + }); + + it("rejects manifest state dirs that look like tar options", () => { + const backup = writeBackup("test-sandbox", "2026-04-21T14-01-00-000Z", { + stateDirs: ["--checkpoint=1"], + }); + + const result = sandboxState.restoreSandboxState("test-sandbox", String(backup.backupPath)); + expect(result.success).toBe(false); + expect(result.restoredDirs).toEqual([]); + expect(result.failedDirs).toContain("--checkpoint=1"); + }); +}); + describe("findBackup", () => { it("matches v against the computed version", () => { writeBackup("test-sandbox", "2026-04-21T14-00-00-000Z"); // v1 (oldest) diff --git a/vitest.config.ts b/vitest.config.ts index 45feaf41e6d..e8469be8f68 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -9,6 +9,10 @@ export default defineConfig({ { test: { name: "cli", + // Root integration tests shell out to built CLI artifacts and child + // processes; under coverage on slower hosts they can exceed Vitest's + // 5s default even when healthy. + testTimeout: 15_000, include: ["test/**/*.test.{js,ts}", "src/**/*.test.ts"], exclude: [ "**/node_modules/**",