From f26aa952af6f5cb66cb893c1c128736b5cdac660 Mon Sep 17 00:00:00 2001 From: peteryuqin Date: Mon, 16 Mar 2026 21:56:36 -0400 Subject: [PATCH 1/3] fix: quote shell interpolations and add timeouts in nim.js All Docker command arguments (container names, image names) were interpolated without shell quotes, risking word splitting. The health check curl had no connect timeout, potentially hanging indefinitely. nimStatus() would throw if docker was not installed. Changes: - Quote all variable interpolations in docker run/stop/rm/inspect/pull - Add --connect-timeout 5 to health check curl calls - Guard nimStatus() against missing docker binary Signed-off-by: peteryuqin --- bin/lib/nim.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/bin/lib/nim.js b/bin/lib/nim.js index f291a0967de..0e643636531 100644 --- a/bin/lib/nim.js +++ b/bin/lib/nim.js @@ -156,7 +156,7 @@ function waitForNimHealth(port = 8000, timeout = 300) { while ((Date.now() - start) / 1000 < timeout) { try { - const result = runCapture(`curl -sf http://localhost:${safePort}/v1/models`, { + const result = runCapture(`curl -sf --connect-timeout 5 http://localhost:${safePort}/v1/models`, { ignoreError: true, }); if (result) { @@ -190,15 +190,22 @@ function nimStatus(sandboxName) { function nimStatusByName(name) { try { + // Guard against docker not being installed + runCapture("command -v docker", { ignoreError: false }); + const state = runCapture( +<<<<<<< HEAD `docker inspect --format '{{.State.Status}}' ${shellQuote(name)} 2>/dev/null`, +======= + `docker inspect --format '{{.State.Status}}' "${name}" 2>/dev/null`, +>>>>>>> fc43952 (fix: quote shell interpolations and add timeouts in nim.js) { ignoreError: true } ); if (!state) return { running: false, container: name }; let healthy = false; if (state === "running") { - const health = runCapture(`curl -sf http://localhost:8000/v1/models 2>/dev/null`, { + const health = runCapture(`curl -sf --connect-timeout 5 http://localhost:8000/v1/models 2>/dev/null`, { ignoreError: true, }); healthy = !!health; From b8568046ddeeae2f9b6d97a0c2a5056466218f58 Mon Sep 17 00:00:00 2001 From: peteryuqin Date: Sun, 22 Mar 2026 19:42:10 -0400 Subject: [PATCH 2/3] test(nim): cover quoting and health probe behavior --- bin/lib/nim.js | 4 -- test/nim.test.js | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/bin/lib/nim.js b/bin/lib/nim.js index 0e643636531..15e07ab337a 100644 --- a/bin/lib/nim.js +++ b/bin/lib/nim.js @@ -194,11 +194,7 @@ function nimStatusByName(name) { runCapture("command -v docker", { ignoreError: false }); const state = runCapture( -<<<<<<< HEAD `docker inspect --format '{{.State.Status}}' ${shellQuote(name)} 2>/dev/null`, -======= - `docker inspect --format '{{.State.Status}}' "${name}" 2>/dev/null`, ->>>>>>> fc43952 (fix: quote shell interpolations and add timeouts in nim.js) { ignoreError: true } ); if (!state) return { running: false, container: name }; diff --git a/test/nim.test.js b/test/nim.test.js index cd4cf6cd4c7..5216b4edf79 100644 --- a/test/nim.test.js +++ b/test/nim.test.js @@ -4,6 +4,54 @@ import { describe, it, expect } from "vitest"; import nim from "../bin/lib/nim"; +function withMockedRunner({ runResult, runCaptureResults = [] }, callback) { + const nimPath = require.resolve("../bin/lib/nim"); + const runnerPath = require.resolve("../bin/lib/runner"); + const savedNim = require.cache[nimPath]; + const savedRunner = require.cache[runnerPath]; + const childProcess = require("node:child_process"); + const savedSpawnSync = childProcess.spawnSync; + const calls = { run: [], runCapture: [], spawnSync: [] }; + + const realRunner = require("../bin/lib/runner"); + require.cache[runnerPath] = { + id: runnerPath, + filename: runnerPath, + loaded: true, + exports: { + ...realRunner, + run(command, options) { + calls.run.push({ command, options }); + return runResult; + }, + runCapture(command, options) { + calls.runCapture.push({ command, options }); + return runCaptureResults.shift() ?? ""; + }, + }, + }; + delete require.cache[nimPath]; + childProcess.spawnSync = (...args) => { + calls.spawnSync.push(args); + return { status: 0 }; + }; + + try { + callback(require("../bin/lib/nim"), calls); + } finally { + delete require.cache[nimPath]; + if (savedNim) { + require.cache[nimPath] = savedNim; + } + if (savedRunner) { + require.cache[runnerPath] = savedRunner; + } else { + delete require.cache[runnerPath]; + } + childProcess.spawnSync = savedSpawnSync; + } +} + describe("nim", () => { describe("listModels", () => { it("returns 5 models", () => { @@ -68,5 +116,75 @@ describe("nim", () => { const st = nim.nimStatus("nonexistent-test-xyz"); expect(st.running).toBe(false); }); + + it("probes health with a connect timeout and quoted container name", () => { + withMockedRunner( + { + runCaptureResults: ["/usr/bin/docker", "running", '{"data":[]}'], + }, + (mockedNim, calls) => { + const st = mockedNim.nimStatus("my-sandbox"); + assert.equal(st.running, true); + assert.equal(st.healthy, true); + assert.equal( + calls.runCapture[1].command, + `docker inspect --format '{{.State.Status}}' 'nemoclaw-nim-my-sandbox' 2>/dev/null`, + ); + assert.equal( + calls.runCapture[2].command, + "curl -sf --connect-timeout 5 http://localhost:8000/v1/models 2>/dev/null", + ); + }, + ); + }); + }); + + describe("shell command construction", () => { + it("quotes docker image pulls", () => { + withMockedRunner({}, (mockedNim, calls) => { + mockedNim.pullNimImage("nvidia/nemotron-3-nano-30b-a3b"); + assert.equal( + calls.run[0].command, + "docker pull 'nvcr.io/nim/nvidia/nemotron-3-nano-30b-a3b:latest'", + ); + }); + }); + + it("quotes docker run and cleanup commands", () => { + withMockedRunner({}, (mockedNim, calls) => { + mockedNim.startNimContainer("my-sandbox", "nvidia/nemotron-3-nano-30b-a3b", 9000); + mockedNim.stopNimContainer("my-sandbox"); + + assert.equal( + calls.run[0].command, + "docker rm -f 'nemoclaw-nim-my-sandbox' 2>/dev/null || true", + ); + assert.equal( + calls.run[1].command, + "docker run -d --gpus all -p 9000:8000 --name 'nemoclaw-nim-my-sandbox' --shm-size 16g 'nvcr.io/nim/nvidia/nemotron-3-nano-30b-a3b:latest'", + ); + assert.equal( + calls.run[2].command, + "docker stop 'nemoclaw-nim-my-sandbox' 2>/dev/null || true", + ); + assert.equal( + calls.run[3].command, + "docker rm 'nemoclaw-nim-my-sandbox' 2>/dev/null || true", + ); + }); + }); + }); + + describe("waitForNimHealth", () => { + it("uses curl connect timeout for readiness probes", () => { + withMockedRunner({ runCaptureResults: ['{"data":[]}'] }, (mockedNim, calls) => { + assert.equal(mockedNim.waitForNimHealth(9000, 1), true); + assert.equal( + calls.runCapture[0].command, + "curl -sf --connect-timeout 5 http://localhost:9000/v1/models", + ); + assert.equal(calls.spawnSync.length, 0); + }); + }); }); }); From 45bc12f4ada0e5ddbcb7f984cd30a3cd222dc694 Mon Sep 17 00:00:00 2001 From: peteryuqin Date: Mon, 23 Mar 2026 12:25:37 -0400 Subject: [PATCH 3/3] fix(nim): bound curl health probes --- bin/lib/nim.js | 4 ++-- test/nim.test.js | 30 +++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/bin/lib/nim.js b/bin/lib/nim.js index 15e07ab337a..47b807b3c19 100644 --- a/bin/lib/nim.js +++ b/bin/lib/nim.js @@ -156,7 +156,7 @@ function waitForNimHealth(port = 8000, timeout = 300) { while ((Date.now() - start) / 1000 < timeout) { try { - const result = runCapture(`curl -sf --connect-timeout 5 http://localhost:${safePort}/v1/models`, { + const result = runCapture(`curl -sf --connect-timeout 5 --max-time 5 http://localhost:${safePort}/v1/models`, { ignoreError: true, }); if (result) { @@ -201,7 +201,7 @@ function nimStatusByName(name) { let healthy = false; if (state === "running") { - const health = runCapture(`curl -sf --connect-timeout 5 http://localhost:8000/v1/models 2>/dev/null`, { + const health = runCapture(`curl -sf --connect-timeout 5 --max-time 5 http://localhost:8000/v1/models 2>/dev/null`, { ignoreError: true, }); healthy = !!health; diff --git a/test/nim.test.js b/test/nim.test.js index 5216b4edf79..365bde15036 100644 --- a/test/nim.test.js +++ b/test/nim.test.js @@ -4,7 +4,7 @@ import { describe, it, expect } from "vitest"; import nim from "../bin/lib/nim"; -function withMockedRunner({ runResult, runCaptureResults = [] }, callback) { +function withMockedRunner({ runResult, runCaptureResults = [], runCaptureImpl }, callback) { const nimPath = require.resolve("../bin/lib/nim"); const runnerPath = require.resolve("../bin/lib/runner"); const savedNim = require.cache[nimPath]; @@ -26,6 +26,9 @@ function withMockedRunner({ runResult, runCaptureResults = [] }, callback) { }, runCapture(command, options) { calls.runCapture.push({ command, options }); + if (runCaptureImpl) { + return runCaptureImpl(command, options, calls); + } return runCaptureResults.shift() ?? ""; }, }, @@ -132,11 +135,32 @@ describe("nim", () => { ); assert.equal( calls.runCapture[2].command, - "curl -sf --connect-timeout 5 http://localhost:8000/v1/models 2>/dev/null", + "curl -sf --connect-timeout 5 --max-time 5 http://localhost:8000/v1/models 2>/dev/null", ); }, ); }); + + it("returns not running when docker is unavailable", () => { + withMockedRunner( + { + runCaptureImpl(command) { + if (command === "command -v docker") { + throw new Error("docker missing"); + } + return ""; + }, + }, + (mockedNim, calls) => { + const st = mockedNim.nimStatus("my-sandbox"); + expect(st).toEqual({ + running: false, + container: "nemoclaw-nim-my-sandbox", + }); + expect(calls.runCapture).toHaveLength(1); + }, + ); + }); }); describe("shell command construction", () => { @@ -181,7 +205,7 @@ describe("nim", () => { assert.equal(mockedNim.waitForNimHealth(9000, 1), true); assert.equal( calls.runCapture[0].command, - "curl -sf --connect-timeout 5 http://localhost:9000/v1/models", + "curl -sf --connect-timeout 5 --max-time 5 http://localhost:9000/v1/models", ); assert.equal(calls.spawnSync.length, 0); });