fix(security): close P1 code scanning findings

agents/hermes/start.sh

@@ -333,7 +333,9 @@ cleanup_orphan_socat_forwarders() {
         kill "$pid" 2>/dev/null || true
         ;;
       *socat*"TCP-LISTEN:${dashboard_public_port}"*"TCP:127.0.0.1:${dashboard_internal_port}"*)
-        [ -n "$dashboard_public_port" ] && [ -n "$dashboard_internal_port" ] || continue
+        if [ -z "$dashboard_public_port" ] || [ -z "$dashboard_internal_port" ]; then
+          continue
+        fi
         echo "[gateway] Removing orphaned dashboard socat forwarder for ${dashboard_public_port}->${dashboard_internal_port} (pid ${pid})" >&2
         kill "$pid" 2>/dev/null || true
         ;;

scripts/nemoclaw-start.sh

@@ -2789,7 +2789,11 @@ NODE
     openclaw_pkg_roots+=("/usr/local/lib/node_modules/openclaw")
     if openclaw_bin="$(command -v openclaw 2>/dev/null)"; then
       openclaw_real="$(readlink -f "$openclaw_bin" 2>/dev/null || printf '%s\n' "$openclaw_bin")"
-      openclaw_pkg="$(cd "$(dirname "$openclaw_real")/.." 2>/dev/null && pwd -P || true)"
+      openclaw_pkg="$(
+        if cd "$(dirname "$openclaw_real")/.." 2>/dev/null; then
+          pwd -P
+        fi
+      )"
       if [ -n "$openclaw_pkg" ]; then
         openclaw_pkg_roots+=("$openclaw_pkg")
       fi

src/lib/actions/dev/npm-link-or-shim.test.ts

@@ -20,8 +20,8 @@ function setupRepo(): { binPath: string; homeDir: string; repoDir: string; tmpDi
   return { binPath, homeDir, repoDir, tmpDir };
 }
 
-function failingNpm() {
-  return vi.fn(() => ({ status: 1, stdout: "", stderr: "npm error code EACCES\n" }));
+function failingNpm(output = "npm error code EACCES\n") {
+  return vi.fn(() => ({ status: 1, stdout: "", stderr: output }));
 }
 
 describe("runNpmLinkOrShim", () => {
@@ -66,8 +66,37 @@ describe("runNpmLinkOrShim", () => {
     const shim = fs.readFileSync(shimPath, "utf-8");
     expect(shim).toContain(DEV_SHIM_MARKER);
     expect(shim).toContain(`exec "${binPath}" "$@"`);
-    expect(errors.join("\n")).toContain("npm link failed");
-    expect(errors.join("\n")).toContain("Created user-local shim");
+    const logOutput = errors.join("\n");
+    expect(logOutput).toContain("npm link failed");
+    expect(logOutput).toContain("Created user-local shim at ~/.local/bin/nemoclaw");
+    expect(logOutput).not.toContain(homeDir);
+    expect(logOutput).not.toContain(repoDir);
+  });
+
+  it("redacts fallback npm output before logging", () => {
+    const { homeDir, repoDir } = setupRepo();
+    const errors: string[] = [];
+    const token = "nvapi-secret-value";
+
+    const result = runNpmLinkOrShim(
+      { env: { HOME: homeDir }, repoRoot: repoDir },
+      {
+        commandPath: () => process.execPath,
+        logError: (message) => errors.push(message),
+        run: failingNpm(`npm failed in ${repoDir} under ${homeDir}\nNVIDIA_API_KEY=${token}\nAuthorization: Bearer ${token}\n`),
+      },
+    );
+
+    const logOutput = errors.join("\n");
+    expect(result.status).toBe(0);
+    expect(logOutput).toContain("npm link failed");
+    expect(logOutput).toContain("<repo-root>");
+    expect(logOutput).toContain("~");
+    expect(logOutput).toContain("NVIDIA_API_KEY=[REDACTED]");
+    expect(logOutput).toContain("Bearer [REDACTED]");
+    expect(logOutput).not.toContain(homeDir);
+    expect(logOutput).not.toContain(repoDir);
+    expect(logOutput).not.toContain(token);
   });
 
   it("refuses to overwrite a foreign shim", () => {
@@ -117,7 +146,10 @@ describe("runNpmLinkOrShim", () => {
     );
 
     expect(result.status).toBe(1);
-    expect(errors.join("\n")).toContain("shim creation failed");
+    const logOutput = errors.join("\n");
+    expect(logOutput).toContain("shim creation failed");
+    expect(logOutput).toContain("~/.local/bin");
+    expect(logOutput).not.toContain(homeDir);
     expect(fs.readFileSync(path.join(homeDir, ".local"), "utf-8")).toBe("not-a-directory\n");
   });
 });

src/lib/actions/dev/npm-link-or-shim.ts

@@ -61,9 +61,30 @@ function defaultCommandPath(command: string, env: NodeJS.ProcessEnv): string | n
   return result.status === 0 && resolved ? resolved : null;
 }
 
-function formatNpmLinkFailure(output: string): string[] {
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+function redactKnownPath(value: string, rawPath: string, replacement: string): string {
+  if (!rawPath) return value;
+  return value.replace(new RegExp(escapeRegExp(rawPath), "g"), replacement);
+}
+
+function sanitizeNpmLinkOutput(output: string, paths: { home: string; repoRoot: string }): string {
+  let sanitized = output;
+  sanitized = redactKnownPath(sanitized, paths.repoRoot, "<repo-root>");
+  sanitized = redactKnownPath(sanitized, paths.home, "~");
+  sanitized = sanitized.replace(
+    /\b(([A-Z0-9_-]*(?:api[_-]?key|token|secret|password)[A-Z0-9_-]*)\s*[:=]\s*)([^\s'"`]+)/gi,
+    "$1[REDACTED]",
+  );
+  sanitized = sanitized.replace(/\b(Bearer\s+)([^\s'"`]+)/gi, "$1[REDACTED]");
+  return sanitized;
+}
+
+function formatNpmLinkFailure(output: string, paths: { home: string; repoRoot: string }): string[] {
   const lines = ["[nemoclaw] npm link failed; falling back to user-local shim."];
-  for (const line of output.split(/\r?\n/).filter(Boolean)) {
+  for (const line of sanitizeNpmLinkOutput(output, paths).split(/\r?\n/).filter(Boolean)) {
     lines.push(`[nemoclaw]   ${line}`);
   }
   return lines;
@@ -95,6 +116,8 @@ export function runNpmLinkOrShim(
   const home = env.HOME || os.homedir();
   const shimDir = path.join(home, ".local", "bin");
   const shimPath = path.join(shimDir, "nemoclaw");
+  const shimDirDisplay = "~/.local/bin";
+  const shimPathDisplay = "~/.local/bin/nemoclaw";
 
   const logError = deps.logError ?? ((message: string) => console.error(message));
   const exists = deps.exists ?? ((filePath: string) => fs.existsSync(filePath));
@@ -133,7 +156,7 @@ export function runNpmLinkOrShim(
   const linkResult = run("npm", ["link"], { cwd: repoRoot, env: installEnv });
   if (linkResult.status === 0) return { status: 0 };
 
-  for (const line of formatNpmLinkFailure(`${linkResult.stdout}${linkResult.stderr}`)) logError(line);
+  for (const line of formatNpmLinkFailure(`${linkResult.stdout}${linkResult.stderr}`, { home, repoRoot })) logError(line);
 
   const nodePath = findNodePath(installEnv, { commandPath, isExecutable });
   if (!nodePath) {
@@ -145,7 +168,7 @@ export function runNpmLinkOrShim(
   if (exists(shimPath)) {
     const classification = classifyDevShim(safeRead(readFile, shimPath));
     if (classification === "foreign") {
-      logError(`[nemoclaw] ${shimPath} already exists and is not managed by NemoClaw; not overwriting.`);
+      logError(`[nemoclaw] ${shimPathDisplay} already exists and is not managed by NemoClaw; not overwriting.`);
       logError("[nemoclaw] Move it aside and re-run 'npm install' to install the dev shim.");
       return { shimPath, status: 1 };
     }
@@ -160,21 +183,22 @@ export function runNpmLinkOrShim(
     rename(shimTmp, shimPath);
     shimTmp = null;
   } catch (error) {
-    logError(`[nemoclaw] shim creation failed: ${error instanceof Error ? error.message : String(error)}`);
+    const reason = error instanceof Error && "code" in error && typeof error.code === "string" ? ` (${error.code})` : "";
+    logError(`[nemoclaw] shim creation failed${reason}; check permissions for ${shimDirDisplay}`);
     return { shimPath, status: 1 };
   } finally {
     if (shimTmp) unlink(shimTmp);
   }
 
   if (!isExecutable(shimPath)) {
-    logError(`[nemoclaw] shim creation failed: ${shimPath} is not executable after write`);
+    logError(`[nemoclaw] shim creation failed: ${shimPathDisplay} is not executable after write`);
     return { shimPath, status: 1 };
   }
 
-  logError(`[nemoclaw] Created user-local shim at ${shimPath} -> ${binPath}`);
+  logError(`[nemoclaw] Created user-local shim at ${shimPathDisplay}`);
   if (!pathContainsDirectory(env.PATH, shimDir)) {
-    logError(`[nemoclaw] ${shimDir} is not on PATH. Add it to your shell profile, e.g.:`);
-    logError(`[nemoclaw]   echo 'export PATH="${shimDir}:$PATH"' >> ~/.bashrc`);
+    logError(`[nemoclaw] ${shimDirDisplay} is not on PATH. Add it to your shell profile, e.g.:`);
+    logError('[nemoclaw]   echo \'export PATH="$HOME/.local/bin:$PATH"\' >> ~/.bashrc');
   }
   return { shimPath, status: 0 };
 }

src/lib/actions/sandbox/snapshot.test.ts

@@ -0,0 +1,97 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+const backupSandboxStateMock = vi.fn();
+const captureOpenshellMock = vi.fn(() => ({ status: 0, output: "alpha Ready\n" }));
+const dockerInspectMock = vi.fn(() => ({ status: 0, stdout: "true\n" }));
+const getSandboxMock = vi.fn(() => null);
+const isGatewayHealthyMock = vi.fn(() => true);
+const parseLiveSandboxNamesMock = vi.fn(() => new Set(["alpha"]));
+
+vi.mock("../../adapters/docker", () => ({
+  dockerCapture: vi.fn(() => ""),
+  dockerInspect: dockerInspectMock,
+}));
+
+vi.mock("../../adapters/openshell/runtime", () => ({
+  captureOpenshell: captureOpenshellMock,
+  getOpenshellBinary: vi.fn(() => "openshell"),
+  runOpenshell: vi.fn(() => ({ status: 0, output: "" })),
+}));
+
+vi.mock("../../credentials/store", () => ({
+  prompt: vi.fn(),
+}));
+
+vi.mock("../../domain/sandbox/destroy", () => ({
+  getSandboxDeleteOutcome: vi.fn(() => ({ alreadyGone: false })),
+}));
+
+vi.mock("../../policy", () => ({}));
+
+vi.mock("../../runner", () => ({
+  ROOT: "/repo",
+  run: vi.fn(() => ({ status: 0 })),
+  shellQuote: (value: string) => `'${value}'`,
+  validateName: vi.fn(),
+}));
+
+vi.mock("../../runtime-recovery", () => ({
+  parseLiveSandboxNames: parseLiveSandboxNamesMock,
+}));
+
+vi.mock("../../shields", () => ({
+  isShieldsDown: undefined,
+}));
+
+vi.mock("../../state/gateway", () => ({
+  isGatewayHealthy: isGatewayHealthyMock,
+}));
+
+vi.mock("../../state/registry", () => ({
+  getSandbox: getSandboxMock,
+  registerSandbox: vi.fn(),
+  removeSandbox: vi.fn(),
+}));
+
+vi.mock("../../state/sandbox", () => ({
+  backupSandboxState: backupSandboxStateMock,
+  findBackup: vi.fn(),
+  listBackups: vi.fn(() => []),
+}));
+
+vi.mock("./destroy", () => ({
+  cleanupShieldsDestroyArtifacts: vi.fn(),
+  removeSandboxRegistryEntry: vi.fn(),
+}));
+
+describe("runSandboxSnapshot", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    captureOpenshellMock.mockReturnValue({ status: 0, output: "alpha Ready\n" });
+    dockerInspectMock.mockReturnValue({ status: 0, stdout: "true\n" });
+    getSandboxMock.mockReturnValue(null);
+    isGatewayHealthyMock.mockReturnValue(true);
+    parseLiveSandboxNamesMock.mockReturnValue(new Set(["alpha"]));
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("refuses snapshot creation before backup when the shields gate helper is unavailable", async () => {
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+    const { runSandboxSnapshot } = await import("./snapshot");
+
+    await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({
+      exitCode: 1,
+    });
+
+    expect(backupSandboxStateMock).not.toHaveBeenCalled();
+    expect(consoleError.mock.calls.flat().join("\n")).toContain(
+      "Cannot verify shields state. Refusing to create snapshot.",
+    );
+  });
+});

src/lib/actions/sandbox/snapshot.ts

@@ -12,7 +12,7 @@ import { getSandboxDeleteOutcome } from "../../domain/sandbox/destroy";
 import * as policies from "../../policy";
 import { ROOT, run, shellQuote, validateName } from "../../runner";
 import { parseLiveSandboxNames } from "../../runtime-recovery";
-import { isShieldsDown } from "../../shields";
+import * as shields from "../../shields";
 import { isGatewayHealthy } from "../../state/gateway";
 import type { SandboxEntry } from "../../state/registry";
 import * as registry from "../../state/registry";
@@ -323,6 +323,18 @@ function probeGatewayRunning(sandboxName?: string): boolean {
   return result.status === 0 && String(result.stdout || "").trim() === "true";
 }
 
+function isSnapshotCreationAllowedByShields(sandboxName: string): boolean {
+  // Snapshot creation is a shields/policy boundary. If a packaged or mocked
+  // CommonJS interop surface ever omits the helper, fail closed before any
+  // backup side effect instead of throwing an ambiguous TypeError.
+  const isShieldsDown = shields.isShieldsDown;
+  if (typeof isShieldsDown !== "function") {
+    console.error("  Cannot verify shields state. Refusing to create snapshot.");
+    return false;
+  }
+  return isShieldsDown(sandboxName);
+}
+
 export async function runSandboxSnapshot(
   sandboxName: string,
   request: SnapshotRequest = { kind: "help" },
@@ -339,7 +351,7 @@ export async function runSandboxSnapshot(
         console.error(`  Sandbox '${sandboxName}' is not running. Cannot create snapshot.`);
         snapshotExit(1);
       }
-      if (!isShieldsDown(sandboxName)) {
+      if (!isSnapshotCreationAllowedByShields(sandboxName)) {
         console.error("  Cannot create snapshot while shields are up.");
         console.error(`  Run \`${CLI_NAME} ${sandboxName} shields down\` first, then retry.`);
         snapshotExit(1);