From 5157b01d25bbf4add7874d54d086acac924e4366 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Tue, 31 Mar 2026 20:04:43 +0000 Subject: [PATCH 1/6] fix: replace predictable temp filenames with mkdtempSync Add secureTempFile(prefix, ext) helper using fs.mkdtempSync() to create temp files inside OS-level unique directories, preventing symlink attacks on predictable /tmp paths. Add cleanupTempDir(filePath, expectedPrefix) guard that verifies the parent directory matches the expected mkdtemp prefix before calling fs.rmSync recursive, preventing accidental deletion of the system temp root on regression. Changes: - runCurlProbe: replace Date.now()+Math.random() with secureTempFile - writeSandboxConfigSyncFile: use secureTempFile, drop tmpDir param - All cleanup sites use cleanupTempDir guard - Test updated for new function signature and mkdtemp assertions Closes #1093 --- bin/lib/onboard.js | 37 ++++++++++++++++++++++++++++--------- test/onboard.test.js | 14 ++++++++++---- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index cf96d03f86c..d7a044ab486 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -46,6 +46,29 @@ const nim = require("./nim"); const onboardSession = require("./onboard-session"); const policies = require("./policies"); const { checkPortAvailable, ensureSwap, getMemoryInfo } = require("./preflight"); + +/** + * Create a temp file inside a directory with a cryptographically random name. + * Uses fs.mkdtempSync (OS-level mkdtemp) to avoid predictable filenames that + * could be exploited via symlink attacks on shared /tmp. + * Ref: https://github.com/NVIDIA/NemoClaw/issues/1093 + */ +function secureTempFile(prefix, ext = "") { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-`)); + return path.join(dir, `${prefix}${ext}`); +} + +/** + * Safely remove a mkdtemp-created directory. Guards against accidentally + * deleting the system temp root if a caller passes os.tmpdir() itself. + */ +function cleanupTempDir(filePath, expectedPrefix) { + const parentDir = path.dirname(filePath); + if (parentDir !== os.tmpdir() && path.basename(parentDir).startsWith(`${expectedPrefix}-`)) { + fs.rmSync(parentDir, { recursive: true, force: true }); + } +} + const EXPERIMENTAL = process.env.NEMOCLAW_EXPERIMENTAL === "1"; const USE_COLOR = !process.env.NO_COLOR && !!process.stdout.isTTY; const DIM = USE_COLOR ? "\x1b[2m" : ""; @@ -683,10 +706,7 @@ function getProbeRecovery(probe, options = {}) { // eslint-disable-next-line complexity function runCurlProbe(argv) { - const bodyFile = path.join( - os.tmpdir(), - `nemoclaw-curl-probe-${Date.now()}-${Math.random().toString(36).slice(2)}.json`, - ); + const bodyFile = secureTempFile("nemoclaw-curl-probe", ".json"); try { const args = [...argv]; const url = args.pop(); @@ -739,7 +759,7 @@ function runCurlProbe(argv) { message: summarizeCurlFailure(error?.status || 1, error?.message || String(error)), }; } finally { - fs.rmSync(bodyFile, { force: true }); + cleanupTempDir(bodyFile, "nemoclaw-curl-probe"); } } @@ -930,9 +950,8 @@ function isOpenclawReady(sandboxName) { return Boolean(fetchGatewayAuthTokenFromSandbox(sandboxName)); } -function writeSandboxConfigSyncFile(script, tmpDir = os.tmpdir()) { - const dir = fs.mkdtempSync(path.join(tmpDir, "nemoclaw-sync-")); - const scriptFile = path.join(dir, "sync.sh"); +function writeSandboxConfigSyncFile(script) { + const scriptFile = secureTempFile("nemoclaw-sync", ".sh"); fs.writeFileSync(scriptFile, `${script}\n`, { mode: 0o600 }); return scriptFile; } @@ -3189,7 +3208,7 @@ async function setupOpenclaw(sandboxName, model, provider) { { stdio: ["ignore", "ignore", "inherit"] }, ); } finally { - fs.rmSync(path.dirname(scriptFile), { recursive: true, force: true }); + cleanupTempDir(scriptFile, "nemoclaw-sync"); } } diff --git a/test/onboard.test.js b/test/onboard.test.js index 820a0fae124..c10f95836db 100644 --- a/test/onboard.test.js +++ b/test/onboard.test.js @@ -486,17 +486,23 @@ describe("onboard helpers", () => { }); it("writes sandbox sync scripts to a temp file for stdin redirection", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-test-")); + const scriptFile = writeSandboxConfigSyncFile("echo test"); try { - const scriptFile = writeSandboxConfigSyncFile("echo test", tmpDir); - expect(scriptFile).toMatch(/nemoclaw-sync-.*[/\\]sync\.sh$/); + expect(scriptFile).toMatch(/nemoclaw-sync.*\.sh$/); expect(fs.readFileSync(scriptFile, "utf8")).toBe("echo test\n"); + // Verify the file lives inside a mkdtemp-created directory (not directly in /tmp) + const parentDir = path.dirname(scriptFile); + expect(parentDir).not.toBe(os.tmpdir()); + expect(parentDir).toContain("nemoclaw-sync"); if (process.platform !== "win32") { const stat = fs.statSync(scriptFile); expect(stat.mode & 0o777).toBe(0o600); } } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + const parentDir = path.dirname(scriptFile); + if (parentDir !== os.tmpdir() && path.basename(parentDir).startsWith("nemoclaw-sync-")) { + fs.rmSync(parentDir, { recursive: true, force: true }); + } } }); From 7d476dfa04c174b2151e0cb7cd92108e04290f16 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Tue, 31 Mar 2026 20:04:43 +0000 Subject: [PATCH 2/6] fix: re-prompt on invalid sandbox name instead of exiting When a user enters an invalid sandbox name (e.g. starting with a digit), promptValidatedSandboxName() now shows the error and re-prompts instead of calling process.exit(1). This prevents users from getting stuck in a loop where rerunning the onboard script reuses the invalid name from the session state, requiring manual deletion of ~/.nemoclaw. In non-interactive mode (CI/CD), the function still exits with status 1 since re-prompting is not possible. Closes #1120 --- bin/lib/onboard.js | 68 ++++++++++++++++++-------------------------- test/onboard.test.js | 13 +++++++++ 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index d7a044ab486..06954f80ea0 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -46,29 +46,6 @@ const nim = require("./nim"); const onboardSession = require("./onboard-session"); const policies = require("./policies"); const { checkPortAvailable, ensureSwap, getMemoryInfo } = require("./preflight"); - -/** - * Create a temp file inside a directory with a cryptographically random name. - * Uses fs.mkdtempSync (OS-level mkdtemp) to avoid predictable filenames that - * could be exploited via symlink attacks on shared /tmp. - * Ref: https://github.com/NVIDIA/NemoClaw/issues/1093 - */ -function secureTempFile(prefix, ext = "") { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-`)); - return path.join(dir, `${prefix}${ext}`); -} - -/** - * Safely remove a mkdtemp-created directory. Guards against accidentally - * deleting the system temp root if a caller passes os.tmpdir() itself. - */ -function cleanupTempDir(filePath, expectedPrefix) { - const parentDir = path.dirname(filePath); - if (parentDir !== os.tmpdir() && path.basename(parentDir).startsWith(`${expectedPrefix}-`)) { - fs.rmSync(parentDir, { recursive: true, force: true }); - } -} - const EXPERIMENTAL = process.env.NEMOCLAW_EXPERIMENTAL === "1"; const USE_COLOR = !process.env.NO_COLOR && !!process.stdout.isTTY; const DIM = USE_COLOR ? "\x1b[2m" : ""; @@ -706,7 +683,7 @@ function getProbeRecovery(probe, options = {}) { // eslint-disable-next-line complexity function runCurlProbe(argv) { - const bodyFile = secureTempFile("nemoclaw-curl-probe", ".json"); + const bodyFile = path.join(os.tmpdir(), `nemoclaw-curl-probe-${Date.now()}-${Math.random().toString(36).slice(2)}.json`); try { const args = [...argv]; const url = args.pop(); @@ -759,7 +736,7 @@ function runCurlProbe(argv) { message: summarizeCurlFailure(error?.status || 1, error?.message || String(error)), }; } finally { - cleanupTempDir(bodyFile, "nemoclaw-curl-probe"); + fs.rmSync(bodyFile, { force: true }); } } @@ -950,8 +927,9 @@ function isOpenclawReady(sandboxName) { return Boolean(fetchGatewayAuthTokenFromSandbox(sandboxName)); } -function writeSandboxConfigSyncFile(script) { - const scriptFile = secureTempFile("nemoclaw-sync", ".sh"); +function writeSandboxConfigSyncFile(script, tmpDir = os.tmpdir()) { + const dir = fs.mkdtempSync(path.join(tmpDir, "nemoclaw-sync-")); + const scriptFile = path.join(dir, "sync.sh"); fs.writeFileSync(scriptFile, `${script}\n`, { mode: 0o600 }); return scriptFile; } @@ -2263,23 +2241,33 @@ async function recoverGatewayRuntime() { // ── Step 3: Sandbox ────────────────────────────────────────────── async function promptValidatedSandboxName() { - const nameAnswer = await promptOrDefault( - " Sandbox name (lowercase, numbers, hyphens) [my-assistant]: ", - "NEMOCLAW_SANDBOX_NAME", - "my-assistant", - ); - const sandboxName = (nameAnswer || "my-assistant").trim().toLowerCase(); + // eslint-disable-next-line no-constant-condition + while (true) { + const nameAnswer = await promptOrDefault( + " Sandbox name (lowercase, numbers, hyphens) [my-assistant]: ", + "NEMOCLAW_SANDBOX_NAME", + "my-assistant", + ); + const sandboxName = (nameAnswer || "my-assistant").trim().toLowerCase(); + + // Validate: RFC 1123 subdomain — lowercase alphanumeric and hyphens, + // must start and end with alphanumeric (required by Kubernetes/OpenShell) + if (/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(sandboxName)) { + return sandboxName; + } - // Validate: RFC 1123 subdomain — lowercase alphanumeric and hyphens, - // must start and end with alphanumeric (required by Kubernetes/OpenShell) - if (!/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(sandboxName)) { console.error(` Invalid sandbox name: '${sandboxName}'`); console.error(" Names must be lowercase, contain only letters, numbers, and hyphens,"); console.error(" and must start and end with a letter or number."); - process.exit(1); - } - return sandboxName; + // Non-interactive runs cannot re-prompt — abort so the caller can fix the + // NEMOCLAW_SANDBOX_NAME env var and retry. + if (isNonInteractive()) { + process.exit(1); + } + + console.error(" Please try again.\n"); + } } // ── Step 5: Sandbox ────────────────────────────────────────────── @@ -3208,7 +3196,7 @@ async function setupOpenclaw(sandboxName, model, provider) { { stdio: ["ignore", "ignore", "inherit"] }, ); } finally { - cleanupTempDir(scriptFile, "nemoclaw-sync"); + fs.rmSync(path.dirname(scriptFile), { recursive: true, force: true }); } } diff --git a/test/onboard.test.js b/test/onboard.test.js index c10f95836db..e1691de1d8a 100644 --- a/test/onboard.test.js +++ b/test/onboard.test.js @@ -1804,4 +1804,17 @@ const { setupInference } = require(${onboardPath}); const commands = JSON.parse(result.stdout.trim().split("\n").pop()); assert.equal(commands.length, 3); }); + + it("re-prompts on invalid sandbox names instead of exiting in interactive mode", () => { + const source = fs.readFileSync( + path.join(import.meta.dirname, "..", "bin", "lib", "onboard.js"), + "utf-8", + ); + // Verify the retry loop exists (while + re-prompt pattern) + assert.match(source, /while\s*\(true\)/); + assert.match(source, /Please try again/); + // Non-interactive still exits + assert.match(source, /isNonInteractive\(\)[\s\S]*?process\.exit\(1\)/); + }); + }); From d9a54c3d5428320cd5e4739c30244da889083ad0 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Tue, 31 Mar 2026 20:04:43 +0000 Subject: [PATCH 3/6] test: scope retry-loop assertions to promptValidatedSandboxName body Address Copilot + CodeRabbit feedback: extract the function body first, then assert within it. Prevents false positives from unrelated while(true) or process.exit(1) elsewhere in the file. --- test/onboard.test.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/onboard.test.js b/test/onboard.test.js index e1691de1d8a..0f47db0f56a 100644 --- a/test/onboard.test.js +++ b/test/onboard.test.js @@ -1810,11 +1810,18 @@ const { setupInference } = require(${onboardPath}); path.join(import.meta.dirname, "..", "bin", "lib", "onboard.js"), "utf-8", ); - // Verify the retry loop exists (while + re-prompt pattern) - assert.match(source, /while\s*\(true\)/); - assert.match(source, /Please try again/); - // Non-interactive still exits - assert.match(source, /isNonInteractive\(\)[\s\S]*?process\.exit\(1\)/); + // Extract the promptValidatedSandboxName function body + const fnMatch = source.match( + /async function promptValidatedSandboxName\(\)\s*\{([\s\S]*?)\n\}/, + ); + assert.ok(fnMatch, "promptValidatedSandboxName function not found"); + const fnBody = fnMatch[1]; + // Verify the retry loop exists within this function + assert.match(fnBody, /while\s*\(true\)/); + assert.match(fnBody, /Please try again/); + // Non-interactive still exits within this function + assert.match(fnBody, /isNonInteractive\(\)/); + assert.match(fnBody, /process\.exit\(1\)/); }); }); From 7c4805c0e345bba79f9c11b2714eec0e60830494 Mon Sep 17 00:00:00 2001 From: Benedikt Schackenberg <6381261+BenediktSchackenberg@users.noreply.github.com> Date: Tue, 31 Mar 2026 23:10:01 +0000 Subject: [PATCH 4/6] style: format with prettier, restore secureTempFile helper Signed-off-by: Benedikt Schackenberg --- bin/lib/onboard.js | 22 +++++++++++++++++++--- test/onboard.test.js | 1 - 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 06954f80ea0..cf670c6ddcb 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -46,6 +46,22 @@ const nim = require("./nim"); const onboardSession = require("./onboard-session"); const policies = require("./policies"); const { checkPortAvailable, ensureSwap, getMemoryInfo } = require("./preflight"); +function secureTempFile(prefix, ext = "") { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-`)); + return path.join(dir, `${prefix}${ext}`); +} + +/** + * Safely remove a mkdtemp-created directory. Guards against accidentally + * deleting the system temp root if a caller passes os.tmpdir() itself. + */ +function cleanupTempDir(filePath, expectedPrefix) { + const parentDir = path.dirname(filePath); + if (parentDir !== os.tmpdir() && path.basename(parentDir).startsWith(`${expectedPrefix}-`)) { + fs.rmSync(parentDir, { recursive: true, force: true }); + } +} + const EXPERIMENTAL = process.env.NEMOCLAW_EXPERIMENTAL === "1"; const USE_COLOR = !process.env.NO_COLOR && !!process.stdout.isTTY; const DIM = USE_COLOR ? "\x1b[2m" : ""; @@ -683,7 +699,7 @@ function getProbeRecovery(probe, options = {}) { // eslint-disable-next-line complexity function runCurlProbe(argv) { - const bodyFile = path.join(os.tmpdir(), `nemoclaw-curl-probe-${Date.now()}-${Math.random().toString(36).slice(2)}.json`); + const bodyFile = secureTempFile("nemoclaw-curl-probe", ".json"); try { const args = [...argv]; const url = args.pop(); @@ -736,7 +752,7 @@ function runCurlProbe(argv) { message: summarizeCurlFailure(error?.status || 1, error?.message || String(error)), }; } finally { - fs.rmSync(bodyFile, { force: true }); + cleanupTempDir(bodyFile, "nemoclaw-curl-probe"); } } @@ -3196,7 +3212,7 @@ async function setupOpenclaw(sandboxName, model, provider) { { stdio: ["ignore", "ignore", "inherit"] }, ); } finally { - fs.rmSync(path.dirname(scriptFile), { recursive: true, force: true }); + cleanupTempDir(scriptFile, "nemoclaw-sync"); } } diff --git a/test/onboard.test.js b/test/onboard.test.js index 0f47db0f56a..20f9a5c61d7 100644 --- a/test/onboard.test.js +++ b/test/onboard.test.js @@ -1823,5 +1823,4 @@ const { setupInference } = require(${onboardPath}); assert.match(fnBody, /isNonInteractive\(\)/); assert.match(fnBody, /process\.exit\(1\)/); }); - }); From c588b844a3f071d26ba6d3aa2b0b09e04261af9e Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 31 Mar 2026 16:24:16 -0700 Subject: [PATCH 5/6] fix: remove nolint --- bin/lib/onboard.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index cf670c6ddcb..1d6f215ed85 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -2257,7 +2257,7 @@ async function recoverGatewayRuntime() { // ── Step 3: Sandbox ────────────────────────────────────────────── async function promptValidatedSandboxName() { - // eslint-disable-next-line no-constant-condition + while (true) { while (true) { const nameAnswer = await promptOrDefault( " Sandbox name (lowercase, numbers, hyphens) [my-assistant]: ", From d8e19b66f4e8f48d6d64482ea359fcc511e554fc Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Tue, 31 Mar 2026 16:25:57 -0700 Subject: [PATCH 6/6] fix: remove duplicate line --- bin/lib/onboard.js | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 1d6f215ed85..f68cce37036 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -2257,7 +2257,6 @@ async function recoverGatewayRuntime() { // ── Step 3: Sandbox ────────────────────────────────────────────── async function promptValidatedSandboxName() { - while (true) { while (true) { const nameAnswer = await promptOrDefault( " Sandbox name (lowercase, numbers, hyphens) [my-assistant]: ",