fix(onboard): clean up build context temp dir on sandbox creation failure

Josue Gomez · futhgar · commit de0c943783b4 · 2026-03-28T17:08:10.000-04:00
Register a process 'exit' handler to guarantee the build context temp
directory is removed even when run() calls process.exit() on command
failure (which bypasses try/finally). Wrap the sandbox creation block
in try/finally for immediate cleanup on sync exceptions and on the
success path.

Signed-off-by: Josue Gomez &lt;josue@guatulab.com&gt;
diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js
@@ -1760,117 +1760,128 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null,
     registry.removeSandbox(sandboxName);
   }
 
-  // Stage build context
+  // Stage build context.
+  // The build context contains source code, scripts, and potentially API keys
+  // in env args, so it must not persist in /tmp after a failed sandbox create.
+  // run() calls process.exit() on failure (bypassing try/finally), so we
+  // register a process 'exit' handler to guarantee cleanup in all cases.
   const buildCtx = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-build-"));
-  const stagedDockerfile = path.join(buildCtx, "Dockerfile");
-  fs.copyFileSync(path.join(ROOT, "Dockerfile"), stagedDockerfile);
-  copyBuildContextDir(path.join(ROOT, "nemoclaw"), path.join(buildCtx, "nemoclaw"));
-  copyBuildContextDir(path.join(ROOT, "nemoclaw-blueprint"), path.join(buildCtx, "nemoclaw-blueprint"));
-  copyBuildContextDir(path.join(ROOT, "scripts"), path.join(buildCtx, "scripts"));
-
-  // Create sandbox (use -- echo to avoid dropping into interactive shell)
-  // Pass the base policy so sandbox starts in proxy mode (required for policy updates later)
-  const basePolicyPath = path.join(ROOT, "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml");
-  const createArgs = [
-    "--from", `${buildCtx}/Dockerfile`,
-    "--name", sandboxName,
-    "--policy", basePolicyPath,
-  ];
-  // --gpu is intentionally omitted. See comment in startGateway().
-
-  console.log(`  Creating sandbox '${sandboxName}' (this takes a few minutes on first run)...`);
-  const chatUiUrl = process.env.CHAT_UI_URL || "http://127.0.0.1:18789";
-  patchStagedDockerfile(stagedDockerfile, model, chatUiUrl, String(Date.now()), provider, preferredInferenceApi);
-  // Only pass non-sensitive env vars to the sandbox. NVIDIA_API_KEY is NOT
-  // needed inside the sandbox — inference is proxied through the OpenShell
-  // gateway which injects the stored credential server-side. The gateway
-  // also strips any Authorization headers sent by the sandbox client.
-  // See: crates/openshell-sandbox/src/proxy.rs (header stripping),
-  //      crates/openshell-router/src/backend.rs (server-side auth injection).
-  const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)];
-  const sandboxEnv = { ...process.env };
-  delete sandboxEnv.NVIDIA_API_KEY;
-  const discordToken = getCredential("DISCORD_BOT_TOKEN") || process.env.DISCORD_BOT_TOKEN;
-  if (discordToken) {
-    sandboxEnv.DISCORD_BOT_TOKEN = discordToken;
-  }
-  const slackToken = getCredential("SLACK_BOT_TOKEN") || process.env.SLACK_BOT_TOKEN;
-  if (slackToken) {
-    sandboxEnv.SLACK_BOT_TOKEN = slackToken;
-  }
-
-  // Run without piping through awk — the pipe masked non-zero exit codes
-  // 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([
-    "sandbox",
-    "create",
-    ...createArgs,
-    "--",
-    "env",
-    ...envArgs,
-    "nemoclaw-start",
-  ])} 2>&1`;
-  const createResult = await streamSandboxCreate(createCommand, sandboxEnv, {
-    readyCheck: () => {
-      const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true });
-      return isSandboxReady(list, sandboxName);
-    },
-  });
+  const cleanupBuildCtx = () => {
+    try { fs.rmSync(buildCtx, { recursive: true, force: true }); } catch {}
+  };
+  process.on("exit", cleanupBuildCtx);
 
-  // Clean up build context regardless of outcome
-  run(`rm -rf "${buildCtx}"`, { ignoreError: true });
+  try {
+    const stagedDockerfile = path.join(buildCtx, "Dockerfile");
+    fs.copyFileSync(path.join(ROOT, "Dockerfile"), stagedDockerfile);
+    copyBuildContextDir(path.join(ROOT, "nemoclaw"), path.join(buildCtx, "nemoclaw"));
+    copyBuildContextDir(path.join(ROOT, "nemoclaw-blueprint"), path.join(buildCtx, "nemoclaw-blueprint"));
+    copyBuildContextDir(path.join(ROOT, "scripts"), path.join(buildCtx, "scripts"));
+
+    // Create sandbox (use -- echo to avoid dropping into interactive shell)
+    // Pass the base policy so sandbox starts in proxy mode (required for policy updates later)
+    const basePolicyPath = path.join(ROOT, "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml");
+    const createArgs = [
+      "--from", `${buildCtx}/Dockerfile`,
+      "--name", sandboxName,
+      "--policy", basePolicyPath,
+    ];
+    // --gpu is intentionally omitted. See comment in startGateway().
+
+    console.log(`  Creating sandbox '${sandboxName}' (this takes a few minutes on first run)...`);
+    const chatUiUrl = process.env.CHAT_UI_URL || "http://127.0.0.1:18789";
+    patchStagedDockerfile(stagedDockerfile, model, chatUiUrl, String(Date.now()), provider, preferredInferenceApi);
+    // Only pass non-sensitive env vars to the sandbox. NVIDIA_API_KEY is NOT
+    // needed inside the sandbox — inference is proxied through the OpenShell
+    // gateway which injects the stored credential server-side. The gateway
+    // also strips any Authorization headers sent by the sandbox client.
+    // See: crates/openshell-sandbox/src/proxy.rs (header stripping),
+    //      crates/openshell-router/src/backend.rs (server-side auth injection).
+    const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)];
+    const sandboxEnv = { ...process.env };
+    delete sandboxEnv.NVIDIA_API_KEY;
+    const discordToken = getCredential("DISCORD_BOT_TOKEN") || process.env.DISCORD_BOT_TOKEN;
+    if (discordToken) {
+      sandboxEnv.DISCORD_BOT_TOKEN = discordToken;
+    }
+    const slackToken = getCredential("SLACK_BOT_TOKEN") || process.env.SLACK_BOT_TOKEN;
+    if (slackToken) {
+      sandboxEnv.SLACK_BOT_TOKEN = slackToken;
+    }
+
+    // Run without piping through awk — the pipe masked non-zero exit codes
+    // 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([
+      "sandbox",
+      "create",
+      ...createArgs,
+      "--",
+      "env",
+      ...envArgs,
+      "nemoclaw-start",
+    ])} 2>&1`;
+    const createResult = await streamSandboxCreate(createCommand, sandboxEnv, {
+      readyCheck: () => {
+        const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true });
+        return isSandboxReady(list, sandboxName);
+      },
+    });
 
-  if (createResult.status !== 0) {
-    console.error("");
-    console.error(`  Sandbox creation failed (exit ${createResult.status}).`);
-    if (createResult.output) {
+    if (createResult.status !== 0) {
       console.error("");
-      console.error(createResult.output);
-    }
-    console.error("  Try:  openshell sandbox list        # check gateway state");
-    printSandboxCreateRecoveryHints(createResult.output);
-    process.exit(createResult.status || 1);
-  }
-
-  // Wait for sandbox to reach Ready state in k3s before registering.
-  // On WSL2 + Docker Desktop the pod can take longer to initialize;
-  // without this gate, NemoClaw registers a phantom sandbox that
-  // causes "sandbox not found" on every subsequent connect/status call.
-  console.log("  Waiting for sandbox to become ready...");
-  let ready = false;
-  for (let i = 0; i < 30; i++) {
-    const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true });
-    if (isSandboxReady(list, sandboxName)) {
-      ready = true;
-      break;
+      console.error(`  Sandbox creation failed (exit ${createResult.status}).`);
+      if (createResult.output) {
+        console.error("");
+        console.error(createResult.output);
+      }
+      console.error("  Try:  openshell sandbox list        # check gateway state");
+      printSandboxCreateRecoveryHints(createResult.output);
+      process.exit(createResult.status || 1);
+    }
+
+    // Wait for sandbox to reach Ready state in k3s before registering.
+    // On WSL2 + Docker Desktop the pod can take longer to initialize;
+    // without this gate, NemoClaw registers a phantom sandbox that
+    // causes "sandbox not found" on every subsequent connect/status call.
+    console.log("  Waiting for sandbox to become ready...");
+    let ready = false;
+    for (let i = 0; i < 30; i++) {
+      const list = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true });
+      if (isSandboxReady(list, sandboxName)) {
+        ready = true;
+        break;
+      }
+      require("child_process").spawnSync("sleep", ["2"]);
     }
-    require("child_process").spawnSync("sleep", ["2"]);
-  }
 
-  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 = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true });
-    console.error("");
-    console.error(`  Sandbox '${sandboxName}' was created but did not become ready within 60s.`);
-    if (delResult.status === 0) {
-      console.error("  The orphaned sandbox has been removed — you can safely retry.");
-    } else {
-      console.error(`  Could not remove the orphaned sandbox. Manual cleanup:`);
-      console.error(`    openshell sandbox delete "${sandboxName}"`);
+    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 = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true });
+      console.error("");
+      console.error(`  Sandbox '${sandboxName}' was created but did not become ready within 60s.`);
+      if (delResult.status === 0) {
+        console.error("  The orphaned sandbox has been removed — you can safely retry.");
+      } else {
+        console.error(`  Could not remove the orphaned sandbox. Manual cleanup:`);
+        console.error(`    openshell sandbox delete "${sandboxName}"`);
+      }
+      console.error("  Retry: nemoclaw onboard");
+      process.exit(1);
     }
-    console.error("  Retry: nemoclaw onboard");
-    process.exit(1);
-  }
 
-  // Release any stale forward on port 18789 before claiming it for the new sandbox.
-  // A previous onboard run may have left the port forwarded to a different sandbox,
-  // which would silently prevent the new sandbox's dashboard from being reachable.
-  runOpenshell(["forward", "stop", "18789"], { ignoreError: true });
-  // Forward dashboard port to the new sandbox
-  runOpenshell(["forward", "start", "--background", "18789", sandboxName], { ignoreError: true });
+    // Release any stale forward on port 18789 before claiming it for the new sandbox.
+    // A previous onboard run may have left the port forwarded to a different sandbox,
+    // which would silently prevent the new sandbox's dashboard from being reachable.
+    runOpenshell(["forward", "stop", "18789"], { ignoreError: true });
+    // Forward dashboard port to the new sandbox
+    runOpenshell(["forward", "start", "--background", "18789", sandboxName], { ignoreError: true });
+  } finally {
+    cleanupBuildCtx();
+    process.removeListener("exit", cleanupBuildCtx);
+  }
 
   // Register only after confirmed ready — prevents phantom entries
   registry.registerSandbox({
diff --git a/test/onboard-build-cleanup.test.js b/test/onboard-build-cleanup.test.js
@@ -0,0 +1,72 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { describe, it, expect } from "vitest";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { spawnSync } from "node:child_process";
+
+describe("onboard build context cleanup", () => {
+  it("removes the build context temp dir when a command fails mid-build", () => {
+    // Simulate the pattern used in createSandbox: register a process 'exit'
+    // handler to clean up the temp dir, then exit non-zero (as run() does
+    // via process.exit on command failure). The handler must still fire.
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cleanup-test-"));
+    const marker = path.join(tmp, "sentinel.txt");
+    fs.writeFileSync(marker, "build context contents");
+
+    const result = spawnSync(
+      "node",
+      [
+        "-e",
+        `
+        const fs = require("fs");
+        const buildCtx = ${JSON.stringify(tmp)};
+        const cleanup = () => {
+          try { fs.rmSync(buildCtx, { recursive: true, force: true }); } catch {}
+        };
+        process.on("exit", cleanup);
+        // Simulate run() calling process.exit() on command failure
+        process.exit(1);
+        `,
+      ],
+      { encoding: "utf-8", timeout: 5000 },
+    );
+
+    expect(result.status).toBe(1);
+    expect(fs.existsSync(tmp)).toBe(false);
+  });
+
+  it("removes the build context on success and deregisters the handler", () => {
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cleanup-test-"));
+    fs.writeFileSync(path.join(tmp, "sentinel.txt"), "build context contents");
+
+    const result = spawnSync(
+      "node",
+      [
+        "-e",
+        `
+        const fs = require("fs");
+        const buildCtx = ${JSON.stringify(tmp)};
+        const cleanup = () => {
+          try { fs.rmSync(buildCtx, { recursive: true, force: true }); } catch {}
+        };
+        process.on("exit", cleanup);
+        // Simulate successful path: explicit cleanup + deregister
+        cleanup();
+        process.removeListener("exit", cleanup);
+        // Verify the specific handler was deregistered
+        if (process.listeners("exit").includes(cleanup)) {
+          console.error("exit handler was not deregistered");
+          process.exit(2);
+        }
+        `,
+      ],
+      { encoding: "utf-8", timeout: 5000 },
+    );
+
+    expect(result.status).toBe(0);
+    expect(fs.existsSync(tmp)).toBe(false);
+  });
+});
