diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index a462b1df317..08812c55a48 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2595,17 +2595,33 @@ function destroyGateway() { } // 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`, + const volumeIds = runCapture( + ["docker", "volume", "ls", "-q", "--filter", `name=openshell-cluster-${GATEWAY_NAME}`], { ignoreError: true }, - ); + ) + .split("\n") + .map((line) => line.trim()) + .filter(Boolean); + if (volumeIds.length > 0) { + run(["docker", "volume", "rm", ...volumeIds], { + ignoreError: true, + suppressOutput: true, + }); + } } 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() @@ -2700,12 +2716,12 @@ fi function runGatewayClusterCapture(script: string, opts: RunnerOptions = {}) { const containerName = getGatewayClusterContainerName(); - return runCapture(`docker exec ${shellQuote(containerName)} sh -lc ${shellQuote(script)}`, opts); + return runCapture(["docker", "exec", containerName, "sh", "-lc", script], opts); } function runGatewayCluster(script: string, opts: RunnerOptions = {}) { const containerName = getGatewayClusterContainerName(); - return run(`docker exec ${shellQuote(containerName)} sh -lc ${shellQuote(script)}`, opts); + return run(["docker", "exec", containerName, "sh", "-lc", script], opts); } function listMissingGatewayBootstrapSecrets() { @@ -3289,14 +3305,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) { @@ -4598,8 +4614,7 @@ async function setupNim(gpu: ReturnType): Promise<{ 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 }); + const hasOllama = !!runCapture(["which", "ollama"], { ignoreError: true }); const ollamaRunning = !!runCapture(["curl", "-sf", `http://127.0.0.1:${OLLAMA_PORT}/api/tags`], { ignoreError: true, }); @@ -7090,7 +7105,11 @@ 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 = isWsl() + ? (String(runCapture(["hostname", "-I"], { ignoreError: true }) || "") + .trim() + .split(/\s+/)[0] || null) + : null; 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/preflight.ts b/src/lib/preflight.ts index 1b232e6bc38..b0849f451c3 100644 --- a/src/lib/preflight.ts +++ b/src/lib/preflight.ts @@ -18,7 +18,9 @@ import { DASHBOARD_PORT } from "./ports"; // 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 } = require("./runner"); + +type CaptureCommand = string | readonly string[]; // ── Types ──────────────────────────────────────────────────────── @@ -120,17 +122,17 @@ export interface AssessHostOpts { dockerInfoOutput?: string; dockerInfoError?: string; readFileImpl?: (filePath: string, encoding: BufferEncoding) => string; - runCaptureImpl?: (command: string, options?: { ignoreError?: boolean }) => string; + runCaptureImpl?: (command: CaptureCommand, options?: { ignoreError?: boolean }) => string; commandExistsImpl?: (commandName: string) => boolean; gpuProbeImpl?: () => boolean; } function commandExists( commandName: string, - runCaptureImpl: (command: string, options?: { ignoreError?: boolean }) => string, + runCaptureImpl: (command: CaptureCommand, options?: { ignoreError?: boolean }) => string, ): boolean { try { - const output = runCaptureImpl(`command -v ${commandName}`, { ignoreError: true }); + const output = runCaptureImpl(["which", commandName], { ignoreError: true }); return Boolean(String(output || "").trim()); } catch { return false; @@ -204,16 +206,16 @@ function isHeadlessLikely(env: NodeJS.ProcessEnv): boolean { } function detectNvidiaGpu( - runCaptureImpl: (command: string, options?: { ignoreError?: boolean }) => string, + runCaptureImpl: (command: CaptureCommand, options?: { ignoreError?: boolean }) => string, ): boolean { if (!commandExists("nvidia-smi", runCaptureImpl)) { 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, + runCaptureImpl: (command: CaptureCommand, options?: { ignoreError?: boolean }) => string, ): PackageManager { if (commandExists("apt-get", runCaptureImpl)) return "apt"; if (commandExists("dnf", runCaptureImpl)) return "dnf"; @@ -245,7 +247,7 @@ export function assessHost(opts: AssessHostOpts = {}): HostAssessment { const env = opts.env ?? process.env; const runCaptureImpl = opts.runCaptureImpl ?? - ((command: string, options?: { ignoreError?: boolean }) => + ((command: CaptureCommand, options?: { ignoreError?: boolean }) => runCapture(command, { ignoreError: options?.ignoreError ?? false })); const readFileImpl = opts.readFileImpl ?? fs.readFileSync; const dockerInstalled = @@ -261,7 +263,7 @@ export function assessHost(opts: AssessHostOpts = {}): HostAssessment { let dockerReachable = false; let dockerRunning = false; if (dockerInstalled && dockerInfoOutput === undefined) { - dockerInfoOutput = runCaptureImpl("docker info --format '{{json .}}' 2>/dev/null", { + dockerInfoOutput = runCaptureImpl(["docker", "info", "--format", "{{json .}}"], { ignoreError: true, }); } @@ -290,11 +292,11 @@ 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, @@ -521,8 +523,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 = runCapture(["which", "lsof"], { ignoreError: true }); if (hasLsof) { lsofOut = runCapture(["lsof", "-i", `:${p}`, "-sTCP:LISTEN", "-P", "-n"], { ignoreError: true, @@ -661,11 +662,10 @@ 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 freeKB = parseInt((dfOut || "").trim().split(/\r?\n/).pop() || "", 10); if (!isNaN(freeKB) && freeKB < 5000000) { return { ok: false, @@ -712,11 +712,17 @@ 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 existingFstabEntry = + run(["grep", "-q", "/swapfile", "/etc/fstab"], { + ignoreError: true, + suppressOutput: true, + }).status === 0; + if (!existingFstabEntry) { + runCapture(["sudo", "tee", "-a", "/etc/fstab"], { + ignoreError: false, + input: "/swapfile none swap sw 0 0\n", + }); + } writeManagedSwapMarker(); return { ok: true, totalMB: mem.totalMB + 4096, swapCreated: true }; @@ -818,12 +824,12 @@ export interface DnsProbeResult { export interface ProbeContainerDnsOpts { /** Override the docker run command. */ - command?: string; + command?: CaptureCommand; /** Inject captured output (bypasses shell). */ outputOverride?: string | null; /** Override runCapture. */ runCaptureImpl?: ( - command: string, + command: CaptureCommand, opts?: { ignoreError?: boolean; timeout?: number }, ) => string | null; } @@ -844,7 +850,7 @@ const PROBE_TIMEOUT_MS = 20_000; * `172.17.0.1`. */ export function getDockerBridgeGatewayIp( - runCaptureImpl: (command: string, opts?: { ignoreError?: boolean }) => string | null = ( + runCaptureImpl: (command: CaptureCommand, opts?: { ignoreError?: boolean }) => string | null = ( cmd, o, ) => runCapture(cmd, { ignoreError: o?.ignoreError ?? false }), @@ -852,7 +858,7 @@ export function getDockerBridgeGatewayIp( let raw: string | null; try { raw = runCaptureImpl( - "docker network inspect bridge --format '{{range .IPAM.Config}}{{.Gateway}}{{end}}' 2>/dev/null", + ["docker", "network", "inspect", "bridge", "--format", "{{range .IPAM.Config}}{{.Gateway}}{{end}}"], { ignoreError: true }, ); } catch { @@ -896,15 +902,14 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes // ignoreError, and we fall through to the `no_output` branch. const command = opts.command ?? - "docker run --rm --pull=missing busybox:latest " + - "nslookup registry.npmjs.org 2>&1"; + ["docker", "run", "--rm", "--pull=missing", "busybox:latest", "nslookup", "registry.npmjs.org"]; let output: string | null | undefined = opts.outputOverride; if (output === undefined) { try { const runCaptureImpl = opts.runCaptureImpl ?? - ((cmd: string, o?: { ignoreError?: boolean; timeout?: number }) => + ((cmd: CaptureCommand, o?: { ignoreError?: boolean; timeout?: number }) => runCapture(cmd, { ignoreError: o?.ignoreError ?? false, timeout: o?.timeout, diff --git a/test/argv-callers.test.ts b/test/argv-callers.test.ts new file mode 100644 index 00000000000..e809e298a82 --- /dev/null +++ b/test/argv-callers.test.ts @@ -0,0 +1,130 @@ +import fs from "node:fs"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +type PreflightModule = typeof import("../dist/lib/preflight.js"); + +function resolveCjsModule(module: T & { default?: unknown }): T { + const defaultExport = module.default; + return (defaultExport && typeof defaultExport === "object" ? defaultExport : module) as T; +} + +async function importPreflightModule(): Promise { + vi.resetModules(); + const module = await import("../dist/lib/preflight.js"); + return resolveCjsModule(module as PreflightModule & { default?: unknown }); +} + +afterEach(() => { + vi.restoreAllMocks(); + vi.resetModules(); + vi.doUnmock("../dist/lib/runner.js"); +}); + +describe("argv callsites", () => { + it("assessHost uses argv commands for host probes", async () => { + const seen: Array = []; + const preflight = await importPreflightModule(); + + const assessment = preflight.assessHost({ + platform: "linux", + env: {}, + release: "6.6.0", + procVersion: "Linux version 6.6.0", + readFileImpl: () => { + throw new Error("no daemon config"); + }, + runCaptureImpl: ( + command: string | readonly string[], + options?: { ignoreError?: boolean }, + ) => { + seen.push(command); + const key = Array.isArray(command) ? command.join(" ") : command; + if (key === "which docker") return "/usr/bin/docker\n"; + if (key === "which node") return "/usr/bin/node\n"; + if (key === "which openshell") return "/usr/bin/openshell\n"; + if (key === "which nvidia-smi") return ""; + if (key === "which apt-get") return "/usr/bin/apt-get\n"; + if (key === "which systemctl") return "/usr/bin/systemctl\n"; + if (key === "docker info --format {{json .}}") + return '{"ServerVersion":"27.0.0","OperatingSystem":"Docker Engine"}'; + if (key === "systemctl is-active docker") return "active\n"; + if (key === "systemctl is-enabled docker") return "enabled\n"; + if (options?.ignoreError) return ""; + throw new Error(`unexpected command in test stub: ${key}`); + }, + }); + + expect(assessment.dockerInstalled).toBe(true); + expect(assessment.nodeInstalled).toBe(true); + expect(assessment.openshellInstalled).toBe(true); + expect(assessment.packageManager).toBe("apt"); + expect(assessment.dockerReachable).toBe(true); + expect(seen).toContainEqual(["which", "docker"]); + expect(seen).toContainEqual(["docker", "info", "--format", "{{json .}}"]); + expect(seen).toContainEqual(["systemctl", "is-active", "docker"]); + expect(seen).toContainEqual(["systemctl", "is-enabled", "docker"]); + expect( + seen.some((command) => typeof command === "string" && command.includes("command -v")), + ).toBe(false); + }); + + it("probeContainerDns defaults to argv docker run", async () => { + const seen: Array = []; + const { probeContainerDns } = await importPreflightModule(); + + const result = probeContainerDns({ + runCaptureImpl: ( + command: string | readonly string[], + opts?: { ignoreError?: boolean; timeout?: number }, + ) => { + seen.push(command); + expect(opts?.ignoreError).toBe(true); + expect(opts?.timeout).toBe(20_000); + return "Server:\t1.1.1.1\nName:\tregistry.npmjs.org\nAddress: 104.16.25.35\n"; + }, + }); + + expect(result).toEqual({ ok: true }); + expect(seen).toEqual([ + [ + "docker", + "run", + "--rm", + "--pull=missing", + "busybox:latest", + "nslookup", + "registry.npmjs.org", + ], + ]); + }); + + it("getDockerBridgeGatewayIp uses argv docker inspect", async () => { + const { getDockerBridgeGatewayIp } = await importPreflightModule(); + const seen: Array = []; + + const gateway = getDockerBridgeGatewayIp((command: string | readonly string[]) => { + seen.push(command); + return "172.17.0.1fd00:abcd::1\n"; + }); + + expect(gateway).toBe("172.17.0.1"); + expect(seen).toEqual([ + [ + "docker", + "network", + "inspect", + "bridge", + "--format", + "{{range .IPAM.Config}}{{.Gateway}}{{end}}", + ], + ]); + }); + + it("getGatewayClusterContainerState uses argv docker inspect", () => { + const onboardSrc = fs.readFileSync(new URL("../dist/lib/onboard.js", import.meta.url), "utf-8"); + + expect(onboardSrc).toMatch( + /runCapture\(\[\s*"docker",\s*"inspect",\s*"--type",\s*"container",\s*"--format",\s*"\{\{\.State\.Status\}\}\{\{if \.State\.Health\}\} \{\{\.State\.Health\.Status\}\}\{\{end\}\}",\s*containerName,\s*\], \{ ignoreError: true \}\)/, + ); + }); +});