fix(security): fail closed while scrubbing migration backups

ci/test-file-size-budget.json

@@ -2,7 +2,7 @@
   "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0",
   "defaultMaxLines": 1500,
   "legacyMaxLines": {
-    "nemoclaw/src/commands/migration-state.test.ts": 1565,
+    "nemoclaw/src/commands/migration-state.test.ts": 1564,
     "src/lib/inference/nim.test.ts": 2068,
     "src/lib/onboard/preflight.test.ts": 1904,
     "test/generate-openclaw-config.test.ts": 1941,

docs/manage-sandboxes/backup-restore.mdx

@@ -38,6 +38,12 @@ Snapshots capture all workspace state directories defined in the agent manifest
 Agent manifests can also declare durable top-level state files.
 Treat snapshot directories as private local data.
 
+Before NemoClaw marks a snapshot complete, it strips recognized credential values from copied JSON, YAML, and `.env` files.
+It preserves OpenShell credential placeholders so rebuild can reattach the host-side provider.
+If NemoClaw cannot sanitize a copied configuration or environment file, it omits that file from the snapshot.
+If it cannot remove the unsafe file, snapshot creation returns an error.
+It deletes the incomplete backup when cleanup succeeds and reports when the backup remains.
+
 <AgentOnly variant="hermes">
 Hermes snapshots include `SOUL.md`, the Web Dashboard profile under `.hermes/dashboard-home/`, the SQLite database behind `.hermes/state.db`, and the default kanban board in `.hermes/kanban.db`.
 The default-profile snapshot also includes cron execution history in `.hermes/runtime/cron-executions.db` and Discord replay state in `.hermes/gateway/discord_message_recovery.db`.

docs/reference/host-files-and-state.mdx

@@ -45,14 +45,18 @@ If you see `registry.json` in older tests, notes, or discussions, treat it as le
 | Path | Purpose | Safe to delete |
 |---|---|---|
 | `~/.nemoclaw/state/` | Operational coordination and history for lifecycle locks, shields transitions, timers, and audit events, local routing, and port-forward helpers. | No. Deleting it can disrupt an active operation and discard security or recovery context. |
-| `~/.nemoclaw/snapshots/` | Full copies of host `~/.openclaw` state created by blueprint migration and rollback flows. | Only after you no longer need the corresponding rollback or restore point. The host CLI does not expose the direct runner's retention actions. |
+| `~/.nemoclaw/snapshots/` | Copies of host `~/.openclaw` state and configured external roots created by blueprint migration and rollback flows. NemoClaw excludes known authentication-state files and strips recognized credential values from copied JSON, YAML, and `.env` files. | Only after you no longer need the corresponding rollback or restore point. The host CLI does not expose the direct runner's retention actions. |
 | `~/.nemoclaw/rebuild-backups/` | Host-side snapshots written by `backup-all`, `snapshot create`, and rebuild flows. | Only after you no longer need rollback or restore points. |
 | `~/.nemoclaw/backups/` | Workspace backups written by legacy backup helpers and some recovery flows. | Only after confirming you no longer need those workspace archives. |
 | `~/.nemoclaw/mounts/` | Default local mount points created by share or mount commands. | Unmount first, then remove unused directories. |
 | `~/.nemoclaw/blueprints/` | Cached blueprint inputs used by onboarding and sandbox recreation. | Avoid manual deletion unless you plan to rerun onboarding from fresh inputs. |
 
 ## Migration Snapshot Retention
 
+Before NemoClaw retains a migration snapshot, it recursively sanitizes the copied OpenClaw state and every configured external root.
+It preserves empty or comment-only YAML files and omits copied JSON, YAML, or `.env` files that it cannot sanitize.
+If NemoClaw cannot remove an unsafe copied artifact, snapshot creation fails and attempts to delete the incomplete snapshot directory.
+
 The direct blueprint runner accepts these action arguments for migration snapshots:
 
 ```text

nemoclaw/src/commands/migration-state.test.ts

@@ -1,31 +1,23 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { describe, it, expect, beforeEach, vi } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 import type { PluginLogger } from "../index.js";
 import { setConfigValue } from "./migration-state.js";
 
-// ---------------------------------------------------------------------------
 // fs mock — thin in-memory store keyed by absolute path
-// ---------------------------------------------------------------------------
-
-interface FsEntry {
-  type: "file" | "dir" | "symlink";
-  content?: string;
-}
-
+type FsEntry = { type: "file" | "dir" | "symlink"; content?: string };
 const store = new Map<string, FsEntry>();
-
-function addDir(p: string): void {
-  store.set(p, { type: "dir" });
-}
-
-function addFile(p: string, content: string): void {
-  store.set(p, { type: "file", content });
-}
-
-function addSymlink(p: string): void {
-  store.set(p, { type: "symlink" });
+const descriptors = new Map<number, string>();
+const addDir = (p: string): void => void store.set(p, { type: "dir" });
+const addFile = (p: string, content: string): void => void store.set(p, { type: "file", content });
+const addSymlink = (p: string): void => void store.set(p, { type: "symlink" });
+// Keep mock failures explicit without adding control-flow branches to the test budget.
+function fileAt(p: string | number): FsEntry {
+  const resolvedPath = typeof p === "number" ? descriptors.get(p) : p;
+  const entry = resolvedPath === undefined ? undefined : store.get(resolvedPath);
+  expect(entry?.type, `expected file at ${String(resolvedPath ?? p)}`).toBe("file");
+  return entry as FsEntry;
 }
 
 vi.mock("node:fs", async (importOriginal) => {
@@ -37,11 +29,15 @@ vi.mock("node:fs", async (importOriginal) => {
       addDir(p);
     }),
     chmodSync: vi.fn(),
-    readFileSync: (p: string) => {
-      const entry = store.get(p);
-      if (entry?.type !== "file") throw new Error(`ENOENT: ${p}`);
-      return entry.content ?? "";
+    readFileSync: (p: string | number) => fileAt(p).content ?? "",
+    openSync: (p: string) => {
+      fileAt(p);
+      const fd = Math.max(99, ...descriptors.keys()) + 1;
+      descriptors.set(fd, p);
+      return fd;
     },
+    fstatSync: (fd: number) => ({ isFile: () => fileAt(fd).type === "file" }),
+    closeSync: (fd: number) => descriptors.delete(fd),
     writeFileSync: vi.fn((p: string, data: string) => {
       store.set(p, { type: "file", content: data });
     }),
@@ -60,7 +56,7 @@ vi.mock("node:fs", async (importOriginal) => {
         }
       }
     }),
-    rmSync: vi.fn(),
+    rmSync: vi.fn((p: string) => store.delete(p)),
     renameSync: vi.fn((oldPath: string, newPath: string) => {
       for (const [k, v] of store) {
         if (k === oldPath || k.startsWith(oldPath + "/")) {
@@ -103,13 +99,13 @@ vi.mock("tar", () => ({
 }));
 
 import {
-  detectHostOpenClaw,
-  createSnapshotBundle,
   cleanupSnapshotBundle,
   createArchiveFromDirectory,
+  createSnapshotBundle,
+  detectHostOpenClaw,
+  type HostOpenClawState,
   loadSnapshotManifest,
   restoreSnapshotToHost,
-  type HostOpenClawState,
   type SnapshotManifest,
 } from "./migration-state.js";
 
@@ -125,6 +121,7 @@ function makeLogger(): PluginLogger {
 describe("commands/migration-state", () => {
   beforeEach(() => {
     store.clear();
+    descriptors.clear();
     vi.clearAllMocks();
   });
 
@@ -579,6 +576,8 @@ describe("commands/migration-state", () => {
       const logger = makeLogger();
       addDir("/home/user/.openclaw");
       addFile("/home/user/.openclaw/openclaw.json", JSON.stringify({ version: 1 }));
+      addDir("/home/user/.openclaw/agents");
+      addDir("/home/user/.openclaw/agents/main");
       addDir("/home/user/.openclaw/agents/main/agent");
       addFile(
         "/home/user/.openclaw/agents/main/agent/auth-profiles.json",

nemoclaw/src/commands/migration-state.ts

@@ -1,6 +1,7 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+import { createHash } from "node:crypto";
 import {
   chmodSync,
   copyFileSync,
@@ -16,10 +17,14 @@ import {
 } from "node:fs";
 import os from "node:os";
 import path from "node:path";
-import { create as createTar } from "tar";
-import { createHash } from "node:crypto";
 import JSON5 from "json5";
+import { create as createTar } from "tar";
 import type { PluginLogger } from "../index.js";
+import { isSensitiveFile } from "../security/credential-filter.js";
+import {
+  sanitizeMigrationDirectory,
+  sanitizeOpenClawConfigFile,
+} from "../security/snapshot-sanitizer.js";
 import { isObjectRecord, type UnknownRecord } from "../shared/object-record.js";
 
 const SANDBOX_MIGRATION_DIR = "/sandbox/.nemoclaw/migration";
@@ -503,79 +508,6 @@ export function detectHostOpenClaw(env: NodeJS.ProcessEnv = process.env): HostOp
   };
 }
 
-// ---------------------------------------------------------------------------
-// Credential sanitization
-// ---------------------------------------------------------------------------
-
-/**
- * Basenames that MUST NOT be copied into snapshot bundles.
- * These files contain credential references or session tokens
- * that should never cross the sandbox boundary.
- */
-const CREDENTIAL_SENSITIVE_BASENAMES = new Set(["auth-profiles.json"]);
-
-/**
- * Credential field names that MUST be stripped from config files
- * before they enter the sandbox. Credentials should be injected
- * at runtime via OpenShell's provider credential mechanism.
- */
-const CREDENTIAL_FIELDS = new Set([
-  "apiKey",
-  "api_key",
-  "token",
-  "secret",
-  "password",
-  "resolvedKey",
-]);
-
-/**
- * Pattern-based detection for credential field names not covered by the
- * explicit set above. Matches common suffixes like accessToken, privateKey,
- * clientSecret, etc.
- */
-const CREDENTIAL_FIELD_PATTERN =
-  /(?:access|refresh|client|bearer|auth|api|private|public|signing|session)(?:Token|Key|Secret|Password)$/;
-
-function isCredentialField(key: string): boolean {
-  return CREDENTIAL_FIELDS.has(key) || CREDENTIAL_FIELD_PATTERN.test(key);
-}
-
-/**
- * Recursively strip credential fields from a JSON-like object.
- * Returns a new object with sensitive values replaced by a placeholder.
- */
-function stripCredentials(obj: unknown): unknown {
-  if (Array.isArray(obj)) return obj.map(stripCredentials);
-  if (!isObjectRecord(obj)) return obj;
-
-  return stripCredentialsFromRecord(obj);
-}
-
-function stripCredentialsFromRecord(obj: UnknownRecord): UnknownRecord {
-  const result: UnknownRecord = {};
-  for (const [key, value] of Object.entries(obj)) {
-    if (isCredentialField(key)) {
-      result[key] = "[STRIPPED_BY_MIGRATION]";
-    } else {
-      result[key] = stripCredentials(value);
-    }
-  }
-  return result;
-}
-
-/**
- * Strip credential fields from openclaw.json and remove the gateway
- * config section (contains auth tokens — regenerated by sandbox entrypoint).
- */
-function sanitizeConfigFile(configPath: string): void {
-  const config = loadConfigDocument(configPath);
-  if (!config) return;
-  delete config.gateway;
-  const sanitized = stripCredentialsFromRecord(config);
-  writeFileSync(configPath, JSON.stringify(sanitized, null, 2));
-  chmodSync(configPath, 0o600);
-}
-
 function computeFileDigest(filePath: string): string {
   if (!existsSync(filePath)) {
     throw new Error(`Blueprint file not found: ${filePath}`);
@@ -593,7 +525,7 @@ function copyDirectory(
   cpSync(sourcePath, destinationPath, {
     recursive: true,
     filter: options?.stripCredentials
-      ? (source: string) => !CREDENTIAL_SENSITIVE_BASENAMES.has(path.basename(source).toLowerCase())
+      ? (source: string) => !isSensitiveFile(path.basename(source))
       : undefined,
   });
 }
@@ -754,7 +686,9 @@ function prepareSandboxState(snapshotDir: string, manifest: SnapshotManifest): s
   // Credentials must be injected at runtime via OpenShell's provider credential
   // mechanism, not baked into the sandbox filesystem where a compromised agent
   // can read them.
-  sanitizeConfigFile(configPath);
+  if (!sanitizeOpenClawConfigFile(configPath)) {
+    throw new Error(`Failed to sanitize prepared OpenClaw config: ${configPath}`);
+  }
 
   return preparedStateDir;
 }
@@ -781,21 +715,32 @@ export function createSnapshotBundle(
     mkdirSync(parentDir, { recursive: true });
     const snapshotStateDir = path.join(parentDir, "openclaw");
     copyDirectory(hostState.stateDir, snapshotStateDir, { stripCredentials: true });
-    sanitizeConfigFile(path.join(snapshotStateDir, "openclaw.json"));
+    sanitizeMigrationDirectory(snapshotStateDir);
+    if (
+      hostState.configPath &&
+      !hostState.hasExternalConfig &&
+      existsSync(hostState.configPath) &&
+      !existsSync(path.join(snapshotStateDir, "openclaw.json"))
+    ) {
+      throw new Error("Failed to sanitize the copied OpenClaw configuration.");
+    }
 
     if (hostState.configPath && hostState.hasExternalConfig) {
       const configSnapshotDir = path.join(parentDir, "config");
       mkdirSync(configSnapshotDir, { recursive: true });
       const configSnapshotPath = path.join(configSnapshotDir, "openclaw.json");
       copyFileSync(hostState.configPath, configSnapshotPath);
-      sanitizeConfigFile(configSnapshotPath);
+      if (!sanitizeOpenClawConfigFile(configSnapshotPath)) {
+        throw new Error("Failed to sanitize the copied external OpenClaw configuration.");
+      }
     }
 
     const externalRoots: MigrationExternalRoot[] = [];
     for (const root of hostState.externalRoots) {
       const destination = path.join(parentDir, root.snapshotRelativePath);
       mkdirSync(path.dirname(destination), { recursive: true });
       copyDirectory(root.sourcePath, destination, { stripCredentials: true });
+      sanitizeMigrationDirectory(destination);
       externalRoots.push({
         ...root,
         symlinkPaths: collectSymlinkPaths(root.sourcePath),
@@ -829,7 +774,18 @@ export function createSnapshotBundle(
     };
   } catch (err: unknown) {
     const msg = err instanceof Error ? err.message : String(err);
-    logger.error(`Snapshot failed: ${msg}`);
+    let cleanupDetail = "";
+    try {
+      rmSync(parentDir, { recursive: true, force: true });
+      if (existsSync(parentDir)) {
+        cleanupDetail = " Incomplete snapshot cleanup did not remove the staging directory.";
+      }
+    } catch (cleanupError: unknown) {
+      cleanupDetail = ` Incomplete snapshot cleanup failed: ${
+        cleanupError instanceof Error ? cleanupError.message : String(cleanupError)
+      }`;
+    }
+    logger.error(`Snapshot failed: ${msg}.${cleanupDetail}`);
     return null;
   }
 }