fix(rebuild): isolate ambient onboard env from sandbox recreate (#5735)

scripts/install.sh

@@ -507,7 +507,14 @@ print_done() {
   local _needs_cli_refresh=false
   needs_shell_reload && _needs_cli_refresh=true
 
-  info "=== Installation complete ==="
+  # #5735: do not claim a clean install when the post-onboard auto-upgrade of a
+  # pre-existing sandbox failed (it may have been destroyed before its recreate
+  # failed). Surface an explicit incomplete/recovery status instead.
+  if [[ "${_UPGRADE_SANDBOXES_FAILED:-false}" == true ]]; then
+    warn "=== Installation completed with warnings ==="
+  else
+    info "=== Installation complete ==="
+  fi
   printf "\n"
   printf "  ${C_GREEN}${C_BOLD}%s${C_RESET}  ${C_DIM}(%ss)${C_RESET}\n" "$_CLI_DISPLAY" "$elapsed"
   printf "\n"
@@ -552,6 +559,11 @@ print_done() {
     print_cli_path_refresh_actions
     printf "  %s$%s %s onboard\n" "$C_GREEN" "$C_RESET" "$_CLI_BIN"
   fi
+  if [[ "${_UPGRADE_SANDBOXES_FAILED:-false}" == true ]]; then
+    printf "\n"
+    printf "  ${C_YELLOW}${C_BOLD}Existing sandbox upgrade did not finish.${C_RESET}\n"
+    printf "  ${C_YELLOW}One or more pre-existing sandboxes failed to upgrade. See the messages above for the affected sandbox name, any preserved backup path, and recovery steps (${C_BOLD}%s onboard --resume${C_RESET}${C_YELLOW} / ${C_BOLD}%s <name> rebuild${C_RESET}${C_YELLOW}).${C_RESET}\n" "$_CLI_BIN" "$_CLI_BIN"
+  fi
   printf "\n"
   printf "  ${C_BOLD}GitHub${C_RESET}  ${C_DIM}https://github.com/nvidia/nemoclaw${C_RESET}\n"
   printf "  ${C_BOLD}Docs${C_RESET}    ${C_DIM}https://docs.nvidia.com/nemoclaw/latest/${C_RESET}\n"
@@ -863,6 +875,10 @@ ONBOARD_RAN=false
 # auto-onboarding (#3276).
 _CLI_PATH=""
 _PREEXISTING_SANDBOX_COUNT=0
+# #5735: set when the post-onboard auto-upgrade of pre-existing sandboxes
+# reported a failure. A failed/destructive rebuild must not be reported as a
+# clean install, so print_done downgrades the final banner when this is true.
+_UPGRADE_SANDBOXES_FAILED=false
 
 # Compare two semver strings (major.minor.patch). Returns 0 if $1 >= $2.
 # Rejects prerelease suffixes (e.g. "22.16.0-rc.1") to avoid arithmetic errors.
@@ -2682,7 +2698,16 @@ main() {
       # Uses --auto so it runs non-interactively in piped/CI contexts.
       if [ "${_PREEXISTING_SANDBOX_COUNT:-0}" -gt 0 ] 2>/dev/null && [ -n "$_cli_runner" ]; then
         info "Checking for sandboxes that need upgrading…"
-        "$_cli_runner" upgrade-sandboxes --auto 2>&1 || warn "Sandbox upgrade check failed (non-fatal)."
+        # #5735: a non-zero exit here can mean an existing sandbox was rebuilt
+        # destructively and its recreate failed. Record it so print_done reports
+        # the install as incomplete with recovery guidance instead of a clean
+        # banner. The CLI already prints the affected sandbox name and the
+        # preserved backup path on failure.
+        if ! "$_cli_runner" upgrade-sandboxes --auto 2>&1; then
+          _UPGRADE_SANDBOXES_FAILED=true
+          warn "One or more existing sandboxes could not be upgraded automatically."
+          warn "Review the messages above — affected sandboxes may need '${_CLI_BIN} onboard --resume' or '${_CLI_BIN} <name> rebuild', and any backup path shown above can restore workspace state."
+        fi
       fi
       restore_onboard_forward_after_post_checks || error "Hermes host forward restore failed."
     elif [ "${NON_INTERACTIVE:-}" = "1" ]; then
@@ -2694,7 +2719,22 @@ main() {
     warn "Skipping onboarding — could not locate the ${_CLI_BIN} executable on disk."
   fi
 
+  finalize_install
+}
+
+# Print the completion summary, then propagate a fatal/non-zero result when the
+# post-onboard auto-upgrade of a pre-existing sandbox failed (#5735, PRA-5). The
+# new sandbox may have onboarded fine, but a failed auto-upgrade can have left an
+# *existing* sandbox destroyed or backup-only, so the install must not be
+# reported as success. print_done() has already shown the affected sandbox and
+# recovery guidance (and the "completed with warnings" banner); exiting non-zero
+# here is what keeps automation and operators from treating it as a clean
+# install. Extracted from main() so it is unit-testable.
+finalize_install() {
   print_done
+  if [[ "${_UPGRADE_SANDBOXES_FAILED:-false}" == true ]]; then
+    error "Installation incomplete: one or more existing sandboxes failed to upgrade. See the recovery guidance above."
+  fi
 }
 
 if [[ "${BASH_SOURCE[0]:-}" == "$0" ]] || { [[ -z "${BASH_SOURCE[0]:-}" ]] && { [[ "$0" == "bash" ]] || [[ "$0" == "-bash" ]]; }; }; then

src/lib/actions/sandbox/rebuild-env-isolation.test.ts

@@ -0,0 +1,129 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { describe, expect, it } from "vitest";
+
+import {
+  AMBIENT_RECREATE_ENV_VARS,
+  assessAmbientRecreateEnv,
+  isolateAmbientRecreateEnv,
+  sanitizeEnvValueForDisplay,
+} from "../../../../dist/lib/actions/sandbox/rebuild-env-isolation.js";
+
+describe("sanitizeEnvValueForDisplay (#5735 PRA-7)", () => {
+  it("collapses a multi-line / ANSI value into a single safe line", () => {
+    // Untrusted NEMOCLAW_AGENT with a newline + CR + ANSI escape that tries to
+    // paint a fake "Installation complete" status line.
+    const malicious = "deepagents\n\u001b[2K\rInstallation complete \u001b[32mOK\u001b[0m";
+    const out = sanitizeEnvValueForDisplay(malicious);
+    expect(out).not.toContain("\n");
+    expect(out).not.toContain("\r");
+    expect(out).not.toContain("\u001b"); // ESC stripped — no ANSI sequence survives
+    expect(out).toContain("deepagents"); // visible text preserved on one line
+  });
+
+  it("strips control characters and trims/collapses whitespace", () => {
+    expect(sanitizeEnvValueForDisplay("a\tb\u0000c   d")).toBe("a b c d");
+    expect(sanitizeEnvValueForDisplay("  spaced  ")).toBe("spaced");
+  });
+
+  it("caps overly long values with an ellipsis", () => {
+    const out = sanitizeEnvValueForDisplay("x".repeat(200), 80);
+    expect(out.length).toBeLessThanOrEqual(81);
+    expect(out.endsWith("…")).toBe(true);
+  });
+});
+
+describe("AMBIENT_RECREATE_ENV_VARS contract (#5735 PRA-4)", () => {
+  it("pins the exact onboard-selection env set the recreate must isolate", () => {
+    // Mirrors the ambient selection env vars `onboard --resume` reads at its
+    // source boundary. Adding a new onboard-selection env var must be a conscious
+    // change here too, or rebuild recreates could be re-contaminated by an
+    // unrelated onboard. Keep in sync with the documented source reads in
+    // rebuild-env-isolation.ts.
+    expect([...AMBIENT_RECREATE_ENV_VARS]).toEqual([
+      "NEMOCLAW_AGENT",
+      "NEMOCLAW_PROVIDER",
+      "NEMOCLAW_PROVIDER_KEY",
+      "NEMOCLAW_ENDPOINT_URL",
+      "NEMOCLAW_MODEL",
+    ]);
+  });
+});
+
+describe("assessAmbientRecreateEnv", () => {
+  it("reports no contamination when no ambient onboard env is set", () => {
+    const result = assessAmbientRecreateEnv("openclaw", {});
+    expect(result.presentVars).toEqual([]);
+    expect(result.agentMismatch).toBeNull();
+  });
+
+  it("flags an ambient NEMOCLAW_AGENT that differs from the registry agent", () => {
+    const result = assessAmbientRecreateEnv("openclaw", {
+      NEMOCLAW_AGENT: "langchain-deepagents-code",
+      NEMOCLAW_PROVIDER_KEY: "sk-bogus",
+    });
+    expect(result.presentVars).toEqual(["NEMOCLAW_AGENT", "NEMOCLAW_PROVIDER_KEY"]);
+    expect(result.agentMismatch).toEqual({
+      envAgent: "langchain-deepagents-code",
+      registryAgent: "openclaw",
+    });
+  });
+
+  it("treats a null registry agent as the default OpenClaw runtime", () => {
+    const result = assessAmbientRecreateEnv(null, { NEMOCLAW_AGENT: "hermes" });
+    expect(result.agentMismatch).toEqual({ envAgent: "hermes", registryAgent: "openclaw" });
+  });
+
+  it("does not flag a mismatch when ambient NEMOCLAW_AGENT matches the registry", () => {
+    const result = assessAmbientRecreateEnv("hermes", { NEMOCLAW_AGENT: "hermes" });
+    expect(result.agentMismatch).toBeNull();
+    expect(result.presentVars).toEqual(["NEMOCLAW_AGENT"]);
+  });
+
+  it("ignores empty/whitespace env values", () => {
+    const result = assessAmbientRecreateEnv("openclaw", {
+      NEMOCLAW_AGENT: "   ",
+      NEMOCLAW_MODEL: "",
+    });
+    expect(result.presentVars).toEqual([]);
+    expect(result.agentMismatch).toBeNull();
+  });
+});
+
+describe("isolateAmbientRecreateEnv", () => {
+  it("removes ambient selection vars and restores the originals (including unset)", () => {
+    const env: NodeJS.ProcessEnv = {
+      NEMOCLAW_AGENT: "langchain-deepagents-code",
+      NEMOCLAW_PROVIDER_KEY: "sk-bogus",
+      NEMOCLAW_MODEL: "some-model",
+      // not part of the selection set — must be left untouched
+      NVIDIA_API_KEY: "nvapi-keep-me",
+    };
+
+    const restore = isolateAmbientRecreateEnv(env);
+
+    for (const name of AMBIENT_RECREATE_ENV_VARS) {
+      expect(env[name]).toBeUndefined();
+    }
+    expect(env.NVIDIA_API_KEY).toBe("nvapi-keep-me");
+
+    restore();
+
+    expect(env.NEMOCLAW_AGENT).toBe("langchain-deepagents-code");
+    expect(env.NEMOCLAW_PROVIDER_KEY).toBe("sk-bogus");
+    expect(env.NEMOCLAW_MODEL).toBe("some-model");
+    expect(env.NVIDIA_API_KEY).toBe("nvapi-keep-me");
+    // A var that was never set stays unset after restore.
+    expect("NEMOCLAW_PROVIDER" in env).toBe(false);
+  });
+
+  it("is idempotent — a second restore call is a no-op", () => {
+    const env: NodeJS.ProcessEnv = { NEMOCLAW_AGENT: "hermes" };
+    const restore = isolateAmbientRecreateEnv(env);
+    restore();
+    env.NEMOCLAW_AGENT = "changed-after-restore";
+    restore();
+    expect(env.NEMOCLAW_AGENT).toBe("changed-after-restore");
+  });
+});

src/lib/actions/sandbox/rebuild-env-isolation.ts

@@ -0,0 +1,111 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// #5735: A rebuild recreates a sandbox from its persisted registry/session
+// config. Ambient onboarding-selection env vars left over from an *unrelated*
+// onboard (e.g. the installer's just-completed Deep Agents onboard right before
+// `upgrade-sandboxes --auto`) must never steer `onboard --resume` away from the
+// target sandbox's recorded agent/provider/model/credential. These are the env
+// vars that onboard's resume path reads to pick the agent, provider, model,
+// endpoint, and credential — isolating them during the recreate forces the
+// pinned session + gateway-registered provider to win.
+//
+// SOURCE-OF-TRUTH NOTE (#5735, PRA-4): the real source boundary is
+// `onboard --resume`, which still reads these from the global `process.env`:
+//   - NEMOCLAW_AGENT        → src/lib/agent/defs.ts resolveAgentName()
+//   - NEMOCLAW_PROVIDER     → src/lib/onboard/providers.ts getNonInteractiveProvider()
+//   - NEMOCLAW_PROVIDER_KEY → src/lib/onboard/provider-key-bridge.ts / providers.ts
+//   - NEMOCLAW_ENDPOINT_URL → src/lib/onboard.ts (remote endpoint override)
+//   - NEMOCLAW_MODEL        → src/lib/onboard.ts (model override)
+// This list MUST stay in sync with those reads; a contract test in
+// rebuild-env-isolation.test.ts pins the exact set so adding a new
+// onboard-selection env var forces a conscious update here.
+// REMOVAL CONDITION: delete this isolation once `onboard --resume` accepts an
+// explicit registry-derived recreate config (or a constrained env map) and
+// stops consulting ambient selection env for rebuild recreates — then the
+// source boundary enforces the invariant and this wrapper is redundant.
+export const AMBIENT_RECREATE_ENV_VARS = [
+  "NEMOCLAW_AGENT",
+  "NEMOCLAW_PROVIDER",
+  "NEMOCLAW_PROVIDER_KEY",
+  "NEMOCLAW_ENDPOINT_URL",
+  "NEMOCLAW_MODEL",
+] as const;
+
+/**
+ * Render an untrusted env value safe to print on a single terminal line (#5735,
+ * PRA-7). `NEMOCLAW_AGENT` is process-environment input and may contain
+ * newlines, ANSI escape sequences, or other control characters that could
+ * inject fake status lines into a destructive rebuild/recovery path. Strip
+ * control + C1 characters (including ESC, which neuters any ANSI sequence),
+ * collapse whitespace runs, and cap the length so the displayed value is a
+ * single bounded token.
+ */
+export function sanitizeEnvValueForDisplay(value: string, maxLength = 80): string {
+  const stripped = value
+    // biome-ignore lint/suspicious/noControlCharactersInRegex: deliberately stripping control chars from untrusted env input before display.
+    .replace(/[\u0000-\u001f\u007f-\u009f]/g, " ")
+    .replace(/\s+/g, " ")
+    .trim();
+  return stripped.length > maxLength ? `${stripped.slice(0, maxLength)}…` : stripped;
+}
+
+export interface AmbientRecreateEnvAssessment {
+  /** Ambient onboard-selection env vars currently set (non-empty). */
+  readonly presentVars: string[];
+  /**
+   * Set when ambient `NEMOCLAW_AGENT` would recreate the sandbox as a different
+   * agent than the registry records — the structural target change behind the
+   * reporter's destroyed-then-recreated-as-Deep-Agents failure.
+   */
+  readonly agentMismatch: { readonly envAgent: string; readonly registryAgent: string } | null;
+}
+
+/**
+ * Describe how the ambient process env would alter this sandbox's recreate,
+ * relative to its authoritative registry agent. Pure — does not mutate env.
+ */
+export function assessAmbientRecreateEnv(
+  registryAgent: string | null | undefined,
+  env: NodeJS.ProcessEnv = process.env,
+): AmbientRecreateEnvAssessment {
+  const presentVars = AMBIENT_RECREATE_ENV_VARS.filter(
+    (name) => typeof env[name] === "string" && env[name]?.trim() !== "",
+  );
+
+  // The registry's null agent is the default OpenClaw runtime.
+  const effectiveRegistryAgent = (registryAgent || "openclaw").trim();
+  const envAgent = (env.NEMOCLAW_AGENT || "").trim();
+  const agentMismatch =
+    envAgent && envAgent !== effectiveRegistryAgent
+      ? { envAgent, registryAgent: effectiveRegistryAgent }
+      : null;
+
+  return { presentVars: [...presentVars], agentMismatch };
+}
+
+/**
+ * Remove the ambient onboard-selection env vars so the immediate
+ * `onboard --resume` recreate cannot read a different onboard's values.
+ * Returns a restore function that puts the original values back (including
+ * re-deleting any var that was unset). Always pair with a `finally`.
+ */
+export function isolateAmbientRecreateEnv(env: NodeJS.ProcessEnv = process.env): () => void {
+  const saved = new Map<string, string | undefined>();
+  for (const name of AMBIENT_RECREATE_ENV_VARS) {
+    saved.set(name, env[name]);
+    delete env[name];
+  }
+  let restored = false;
+  return () => {
+    if (restored) return;
+    restored = true;
+    for (const [name, value] of saved) {
+      if (value === undefined) {
+        delete env[name];
+      } else {
+        env[name] = value;
+      }
+    }
+  };
+}