From fd3b4a3bbf3fa72dd0fb04a68de8b15fe128043f Mon Sep 17 00:00:00 2001 From: Hakancan Date: Wed, 18 Mar 2026 18:50:41 +0000 Subject: [PATCH 1/3] fix: quote sandbox names in shell commands using shellEscape() Fixes #166 - Add shellEscape() function to properly escape sandbox names in shell commands by wrapping in single quotes and handling embedded single quotes correctly - Apply shell escaping to all shell commands that use sandboxName: - openshell sandbox delete - openshell sandbox create --name - openshell forward start This replaces the previous double-quoted interpolation ("${sandboxName}") with shellEscape(), which is more robust and prevents shell injection when sandbox names contain special characters. --- bin/lib/onboard.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 060b5ba2120..63d3491c9a8 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -57,6 +57,19 @@ async function promptOrDefault(question, envVar, defaultValue) { // ── Helpers ────────────────────────────────────────────────────── +/** + * Escapes a string for safe use in shell commands. + * Wraps in single quotes and handles embedded single quotes. + */ +function shellEscape(str) { + if (typeof str !== "string") { + throw new Error("shellEscape: expected string argument"); + } + // Use single quotes and escape any embedded single quotes + // by ending the quote, adding an escaped quote, and starting a new quote + return "'" + str.replace(/'/g, "'\"'\"'") + "'"; +} + /** * Check if a sandbox is in Ready state from `openshell sandbox list` output. * Strips ANSI codes and exact-matches the sandbox name in the first column. @@ -438,7 +451,7 @@ async function createSandbox(gpu) { } } // Destroy old sandbox - run(`openshell sandbox delete "${sandboxName}" 2>/dev/null || true`, { ignoreError: true }); + run(`openshell sandbox delete ${shellEscape(sandboxName)} 2>/dev/null || true`, { ignoreError: true }); registry.removeSandbox(sandboxName); } @@ -457,7 +470,7 @@ async function createSandbox(gpu) { const basePolicyPath = path.join(ROOT, "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml"); const createArgs = [ `--from "${buildCtx}/Dockerfile"`, - `--name "${sandboxName}"`, + `--name ${shellEscape(sandboxName)}`, `--policy "${basePolicyPath}"`, ]; // --gpu is intentionally omitted. See comment in startGateway(). @@ -525,7 +538,7 @@ async function createSandbox(gpu) { // which would silently prevent the new sandbox's dashboard from being reachable. run(`openshell forward stop 18789 2>/dev/null || true`, { ignoreError: true }); // Forward dashboard port to the new sandbox - run(`openshell forward start --background 18789 "${sandboxName}"`, { ignoreError: true }); + run(`openshell forward start --background 18789 ${shellEscape(sandboxName)}`, { ignoreError: true }); // Register only after confirmed ready — prevents phantom entries registry.registerSandbox({ From 09afb419de703393c9ea247a3a455002bbb263a3 Mon Sep 17 00:00:00 2001 From: Hakancan Date: Sat, 21 Mar 2026 09:22:29 +0000 Subject: [PATCH 2/3] refactor: consolidate shell quoting to use existing shellQuote() helper Removes the newly added shellEscape() function which duplicated the pre-existing shellQuote() helper. All call sites updated to use shellQuote(). --- bin/lib/onboard.js | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 63d3491c9a8..ddd95b40634 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -57,19 +57,6 @@ async function promptOrDefault(question, envVar, defaultValue) { // ── Helpers ────────────────────────────────────────────────────── -/** - * Escapes a string for safe use in shell commands. - * Wraps in single quotes and handles embedded single quotes. - */ -function shellEscape(str) { - if (typeof str !== "string") { - throw new Error("shellEscape: expected string argument"); - } - // Use single quotes and escape any embedded single quotes - // by ending the quote, adding an escaped quote, and starting a new quote - return "'" + str.replace(/'/g, "'\"'\"'") + "'"; -} - /** * Check if a sandbox is in Ready state from `openshell sandbox list` output. * Strips ANSI codes and exact-matches the sandbox name in the first column. @@ -451,7 +438,7 @@ async function createSandbox(gpu) { } } // Destroy old sandbox - run(`openshell sandbox delete ${shellEscape(sandboxName)} 2>/dev/null || true`, { ignoreError: true }); + run(`openshell sandbox delete ${shellQuote(sandboxName)} 2>/dev/null || true`, { ignoreError: true }); registry.removeSandbox(sandboxName); } @@ -470,7 +457,7 @@ async function createSandbox(gpu) { const basePolicyPath = path.join(ROOT, "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml"); const createArgs = [ `--from "${buildCtx}/Dockerfile"`, - `--name ${shellEscape(sandboxName)}`, + `--name ${shellQuote(sandboxName)}`, `--policy "${basePolicyPath}"`, ]; // --gpu is intentionally omitted. See comment in startGateway(). @@ -538,7 +525,7 @@ async function createSandbox(gpu) { // which would silently prevent the new sandbox's dashboard from being reachable. run(`openshell forward stop 18789 2>/dev/null || true`, { ignoreError: true }); // Forward dashboard port to the new sandbox - run(`openshell forward start --background 18789 ${shellEscape(sandboxName)}`, { ignoreError: true }); + run(`openshell forward start --background 18789 ${shellQuote(sandboxName)}`, { ignoreError: true }); // Register only after confirmed ready — prevents phantom entries registry.registerSandbox({ From 691ceb98cdd808cdc2eba3ecbdf13de1205a2170 Mon Sep 17 00:00:00 2001 From: Hakancan Date: Sat, 28 Mar 2026 23:45:00 +0000 Subject: [PATCH 3/3] fix: apply shellQuote consistently to all sandboxName usages --- bin/lib/onboard.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index ddd95b40634..688004bdd1f 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -221,7 +221,7 @@ function sleep(seconds) { function waitForSandboxReady(sandboxName, attempts = 10, delaySeconds = 2) { for (let i = 0; i < attempts; i += 1) { - const exists = runCapture(`openshell sandbox get "${sandboxName}" 2>/dev/null`, { ignoreError: true }); + const exists = runCapture(`openshell sandbox get ${shellQuote(sandboxName)} 2>/dev/null`, { ignoreError: true }); if (exists) return true; sleep(delaySeconds); } @@ -507,7 +507,7 @@ async function createSandbox(gpu) { if (!ready) { // Clean up the orphaned sandbox so the next onboard retry with the same // name doesn't fail on "sandbox already exists". - const delResult = run(`openshell sandbox delete "${sandboxName}" 2>/dev/null || true`, { ignoreError: true }); + const delResult = run(`openshell sandbox delete ${shellQuote(sandboxName)} 2>/dev/null || true`, { ignoreError: true }); console.error(""); console.error(` Sandbox '${sandboxName}' was created but did not become ready within 60s.`); if (delResult.status === 0) { @@ -800,7 +800,7 @@ async function setupOpenclaw(sandboxName, model, provider) { onboardedAt: new Date().toISOString(), }; const script = buildSandboxConfigSyncScript(sandboxConfig); - run(`cat <<'EOF_NEMOCLAW_SYNC' | openshell sandbox connect "${sandboxName}" + run(`cat <<'EOF_NEMOCLAW_SYNC' | openshell sandbox connect ${shellQuote(sandboxName)} ${script} EOF_NEMOCLAW_SYNC`, { stdio: ["ignore", "ignore", "inherit"] }); }