fix(credentials): clean up empty legacy credentials.json on upgrade

src/lib/credentials.ts

@@ -383,6 +383,91 @@ export function removeLegacyCredentialsFile(): void {
   secureUnlink(legacyFile);
 }
 
+/**
+ * Securely remove the legacy plaintext credentials.json *iff* it carries
+ * no migratable credential payload — i.e. it's an empty `{}`, contains
+ * only keys outside `KNOWN_CREDENTIAL_ENV_KEYS`, or every allowlisted key
+ * has a blank/non-string value. Used by the onboard completion path to
+ * clean up the stale empty file left behind on upgrades from pre-gateway
+ * NemoClaw versions (#3105).
+ *
+ * Refuses to act on a missing/symlinked/oversized/non-JSON/non-object
+ * file — those are either nothing-to-do or "leave for inspection" cases
+ * that the regular migration path already handles. Returns `true` only
+ * if the file was actually removed.
+ */
+export function removeLegacyCredentialsFileIfEmpty(): boolean {
+  const legacyFile = getCredsFile();
+
+  try {
+    rejectSymlinksOnPath(path.dirname(legacyFile));
+  } catch {
+    return false;
+  }
+
+  let fd: number;
+  try {
+    fd = fs.openSync(legacyFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW);
+  } catch {
+    return false;
+  }
+
+  let raw: string;
+  try {
+    const stat = fs.fstatSync(fd);
+    if (!stat.isFile()) return false;
+    if (stat.size > LEGACY_CREDS_FILE_MAX_BYTES) return false;
+    raw = fs.readFileSync(fd, "utf-8");
+  } catch {
+    return false;
+  } finally {
+    try {
+      fs.closeSync(fd);
+    } catch {
+      /* fd already closed; ignore */
+    }
+  }
+
+  // A 0-byte or whitespace-only file is functionally identical to an
+  // empty {} — there's no migratable payload, so skip JSON.parse (which
+  // would throw on the empty input) and fall through to the unlink.
+  if (raw.trim() !== "") {
+    let parsed: unknown;
+    try {
+      parsed = JSON.parse(raw);
+    } catch {
+      return false;
+    }
+
+    if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
+      return false;
+    }
+
+    const allowed = new Set<string>(KNOWN_CREDENTIAL_ENV_KEYS);
+    for (const [key, value] of Object.entries(parsed as Record<string, unknown>)) {
+      if (!allowed.has(key)) continue;
+      if (typeof value !== "string") continue;
+      if (normalizeCredentialValue(value)) {
+        return false;
+      }
+    }
+  }
+
+  // secureUnlink is best-effort and swallows errors. Verify the file is
+  // actually gone before claiming a successful removal — otherwise the
+  // runner would log "Removed stale ..." on a permission-denied unlink.
+  secureUnlink(legacyFile);
+  try {
+    fs.lstatSync(legacyFile);
+  } catch (error) {
+    if (isErrnoException(error) && error.code === "ENOENT") {
+      return true;
+    }
+    return false;
+  }
+  return false;
+}
+
 /**
  * Read a secret value from a TTY without echoing typed characters
  * (asterisks are written instead). Resolves to the trimmed answer or

src/lib/host-artifact-cleanup.ts

@@ -0,0 +1,59 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Post-upgrade stale-file cleanup.
+//
+// As NemoClaw evolves, host-side state migrates: older versions wrote files
+// under the user's home that newer versions no longer use. Each entry in
+// STALE_FILES owns the safety logic for one specific stale file and reports
+// back when it actually removed something. The runner invokes them all once
+// during the onboard completion path so a fresh post-upgrade onboard sweeps
+// every leftover in a single uniform pass. Adding a future cleanup is one
+// entry on STALE_FILES; the loop does not change. If a future leftover is a
+// folder or symlink instead of a file, widen the shape at that point.
+
+import { removeLegacyCredentialsFileIfEmpty } from "./credentials";
+
+interface StaleHostFile {
+  /** Human-readable description for the success log line. */
+  readonly description: string;
+  /**
+   * Atomically inspect-and-remove the file iff its safety guards
+   * permit it. Returns true iff the file was actually removed.
+   * Splitting "is-removable?" from "remove" would be TOCTOU-unsafe,
+   * so each entry exposes a single combined operation.
+   */
+  readonly tryRemove: () => boolean;
+}
+
+const STALE_FILES: readonly StaleHostFile[] = [
+  {
+    description: "~/.nemoclaw/credentials.json (no migratable credentials)",
+    tryRemove: removeLegacyCredentialsFileIfEmpty,
+  },
+];
+
+/**
+ * Sweep every registered stale host file left behind by older NemoClaw
+ * versions. Best-effort: a failure inside one entry is logged to stderr
+ * but never aborts the others or the surrounding onboard. Logs a single
+ * line to stdout per file actually removed so the user can audit what
+ * changed. Safe to call after a successful onboard regardless of which
+ * migration paths fired earlier — every entry is a no-op when its
+ * target doesn't exist or doesn't satisfy the entry's safety rules.
+ */
+export function cleanupStaleHostFiles(): void {
+  for (const file of STALE_FILES) {
+    try {
+      if (file.tryRemove()) {
+        console.log(`  Removed stale ${file.description}.`);
+      }
+    } catch (error) {
+      console.error(
+        `  Skipped stale-file cleanup ${file.description}: ${
+          error instanceof Error ? error.message : String(error)
+        }`,
+      );
+    }
+  }
+}

src/lib/onboard.ts

@@ -272,6 +272,9 @@ const {
   saveCredential,
 } = credentials;
 const { hashCredential }: typeof import("./credential-hash") = require("./credential-hash");
+const {
+  cleanupStaleHostFiles,
+}: typeof import("./host-artifact-cleanup") = require("./host-artifact-cleanup");
 const registry: typeof import("./state/registry") = require("./state/registry");
 const nim: typeof import("./nim") = require("./nim");
 const onboardSession: typeof import("./onboard-session") = require("./onboard-session");
@@ -9954,6 +9957,13 @@ async function onboard(opts: OnboardOptions = {}): Promise<void> {
           `providers/channels enabled to migrate them, then the file is removed automatically.`,
       );
     }
+    // Sweep stale host files left over from older NemoClaw versions —
+    // e.g. an empty/orphaned ~/.nemoclaw/credentials.json from upgrades
+    // before the credentials-gateway move (issue #3105). Each registered
+    // entry enforces its own safety guards; this call is a no-op when
+    // every target is already clean.
+    cleanupStaleHostFiles();
+
     // Post-deployment verification — confirm the full delivery chain is
     // operational before telling the user "YOUR AGENT IS LIVE". Fixes #2342.
     const verifyDeploymentModule: typeof import("./verify-deployment") = require("./verify-deployment");

test/credentials.test.ts

@@ -20,7 +20,8 @@ function isCredentialsModule(value: object | null): value is CredentialsModule {
     typeof Reflect.get(value, "getCredential") === "function" &&
     typeof Reflect.get(value, "saveCredential") === "function" &&
     typeof Reflect.get(value, "stageLegacyCredentialsToEnv") === "function" &&
-    typeof Reflect.get(value, "removeLegacyCredentialsFile") === "function"
+    typeof Reflect.get(value, "removeLegacyCredentialsFile") === "function" &&
+    typeof Reflect.get(value, "removeLegacyCredentialsFileIfEmpty") === "function"
   );
 }
 
@@ -441,6 +442,182 @@ describe("legacy credentials.json migration (two-phase: stage then remove)", ()
   });
 });
 
+describe("removeLegacyCredentialsFileIfEmpty (post-upgrade cleanup, #3105)", () => {
+  it("removes an empty {} legacy file (regression #3105)", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(legacyFile, "{}", { mode: 0o600 });
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+
+  it("removes a file containing only unknown keys", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(
+      legacyFile,
+      JSON.stringify({ FOO: "bar", PATH: "/etc/passwd" }),
+      { mode: 0o600 },
+    );
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+
+  it("removes a file where every allowlisted value is blank/whitespace", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(
+      legacyFile,
+      JSON.stringify({ NVIDIA_API_KEY: "", OPENAI_API_KEY: "   \r\n\t  " }),
+      { mode: 0o600 },
+    );
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+
+  it("keeps a file with at least one non-empty allowlisted credential", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    const payload = JSON.stringify({ NVIDIA_API_KEY: "nvapi-real-secret", FOO: "bar" });
+    fs.writeFileSync(legacyFile, payload, { mode: 0o600 });
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false);
+    expect(fs.existsSync(legacyFile)).toBe(true);
+    expect(fs.readFileSync(legacyFile, "utf-8")).toBe(payload);
+  });
+
+  it("returns false when no legacy file exists", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false);
+  });
+
+  it("refuses to act on a symlinked legacy path (target untouched)", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+
+    const victimFile = path.join(home, "victim.json");
+    fs.writeFileSync(victimFile, "{}", { mode: 0o600 });
+    fs.symlinkSync(victimFile, legacyFile);
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false);
+    expect(fs.existsSync(legacyFile)).toBe(true);
+    expect(fs.existsSync(victimFile)).toBe(true);
+  });
+
+  it("leaves a corrupt legacy file in place for inspection", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(legacyFile, "{not-json", { mode: 0o600 });
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false);
+    expect(fs.existsSync(legacyFile)).toBe(true);
+  });
+
+  it("removes a 0-byte legacy file (CodeRabbit nit: whitespace-only doesn't throw)", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(legacyFile, "", { mode: 0o600 });
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+
+  it("removes a whitespace-only legacy file", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(legacyFile, "   \n\t\r\n  ", { mode: 0o600 });
+
+    const credentials = await importCredentialsModule(home);
+    expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+
+  it("returns false when the secure unlink silently fails (CodeRabbit nit)", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    fs.writeFileSync(legacyFile, "{}", { mode: 0o600 });
+
+    // Simulate a swallowed unlink failure: secureUnlink internally calls
+    // fs.unlinkSync with try/catch, so a no-op stub leaves the file intact.
+    // The helper must detect this and return false rather than lying.
+    const spy = vi.spyOn(fs, "unlinkSync").mockImplementation(() => undefined);
+    try {
+      const credentials = await importCredentialsModule(home);
+      expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false);
+    } finally {
+      spy.mockRestore();
+    }
+
+    expect(fs.existsSync(legacyFile)).toBe(true);
+  });
+
+  it("zero-fills an empty file before unlinking (defence in depth)", async () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-"));
+    const credsDir = path.join(home, ".nemoclaw");
+    const legacyFile = path.join(credsDir, "credentials.json");
+    fs.mkdirSync(credsDir, { recursive: true });
+    const cleartext = "{}";
+    fs.writeFileSync(legacyFile, cleartext, { mode: 0o600 });
+
+    const originalUnlink = fs.unlinkSync;
+    const captured: { bytes: Buffer | null } = { bytes: null };
+    const spy = vi.spyOn(fs, "unlinkSync").mockImplementation((p) => {
+      if (typeof p === "string" && p === legacyFile && captured.bytes === null) {
+        try {
+          captured.bytes = fs.readFileSync(p);
+        } catch {
+          /* file already gone */
+        }
+      }
+      return originalUnlink(p);
+    });
+
+    try {
+      const credentials = await importCredentialsModule(home);
+      expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true);
+    } finally {
+      spy.mockRestore();
+    }
+
+    const bytesAtUnlink = captured.bytes;
+    expect(bytesAtUnlink).not.toBeNull();
+    if (bytesAtUnlink !== null) {
+      expect(bytesAtUnlink.length).toBe(Buffer.byteLength(cleartext));
+      expect(bytesAtUnlink.every((b) => b === 0)).toBe(true);
+    }
+    expect(fs.existsSync(legacyFile)).toBe(false);
+  });
+});
+
 describe("prompt machinery (unchanged)", () => {
   it("exits cleanly when answers are staged through a pipe", () => {
     const script = `