fix(recovery): add connect probe recovery path (#2646)

## Summary
- adds `nemoclaw <sandbox> connect --probe-only` as a non-interactive
gateway recovery probe, plus `connect --help` handling that does not
require registry or OpenShell recovery
- runs gateway health and recovery through `openshell sandbox exec`
first, with SSH fallback, so recovery uses the same sandbox exec path as
the adversarial tamper probe
- centralizes OpenClaw recovery script generation and prepares
`/tmp/gateway.log` as the real gateway-owned log before writing
`[gateway-recovery] WARNING` and relaunching via `gosu gateway` when
available
- updates the #2478 e2e to use `connect --probe-only`, assert the
warning, and verify a negative-case respawn

## Verification
- `npm run build:cli`
- `npx vitest run src/lib/agent-runtime.test.ts
test/nemoclaw-cli-recovery.test.ts test/cli.test.ts -t "connect
--probe-only|shows connect help|buildRecoveryScript|nemoclaw CLI runtime
recovery"`
- `npm run typecheck:cli`
- `tmp_home=$(mktemp -d); HOME="$tmp_home" node bin/nemoclaw.js alpha
connect --help`
- `git diff --check`
- `npx prettier --check src/nemoclaw.ts src/lib/agent-runtime.ts
src/lib/agent-runtime.test.ts src/lib/command-registry.ts
test/cli.test.ts`
- `npx eslint src/nemoclaw.ts src/lib/agent-runtime.ts
src/lib/agent-runtime.test.ts src/lib/command-registry.ts
test/cli.test.ts` (exit 0; TS files ignored by local ESLint config
warnings)

## Local full-hook notes
- `npm run format:check -- ...` still runs the repo-wide configured
globs and fails on pre-existing formatting issues in unrelated test
files. Direct Prettier check on touched files passes.
- The full pre-commit/pre-push CLI test hook times out locally in
unrelated tests: `test/onboard.test.ts` and
`test/sandbox-build-context.test.ts`; after building `nemoclaw/dist`,
the focused regression tests above pass.
- Initial SSH push was blocked by NVIDIA SAML for the local SSH key;
branch was pushed through the authenticated HTTPS GitHub CLI path.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added --probe-only connect mode, implicit connect handling, and
OpenClaw-aware recovery with safer gateway start/log selection.

* **Bug Fixes**
* Hardened gateway log preparation (no symlink-following, append
redirection, fd-safe permission/ownership) and deterministic exec-based
sandbox probing with SSH fallback.
* Simplified/stricter agent-binary verification and improved
cross-platform config-owner detection.

* **Chores**
* Expanded tests and e2e scripts; updated permission-mode expectations
and added setgid handling for locking.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>

Author: ericksoa

scripts/nemoclaw-start.sh

@@ -230,7 +230,7 @@ normalize_mutable_config_perms() {
   # Detect shields-up. Config dir owned by root means shields are
   # currently locked; normalizing would weaken the contract.
   local config_dir_owner
-  config_dir_owner="$(stat -c '%U' "$config_dir" 2>/dev/null || echo unknown)"
+  config_dir_owner="$(stat -c '%U' "$config_dir" 2>/dev/null || stat -f '%Su' "$config_dir" 2>/dev/null || echo unknown)"
   if [ "$config_dir_owner" = "root" ]; then
     return 0
   fi

src/lib/agent-onboard.test.ts

@@ -5,7 +5,7 @@ import { describe, it, expect, beforeEach, afterEach, afterAll, vi } from "vites
 import fs from "node:fs";
 import path from "node:path";
 // Import from compiled dist/ so coverage is attributed correctly.
-import { printDashboardUi } from "../../dist/lib/agent-onboard";
+import { printDashboardUi, verifyAgentBinaryAvailable } from "../../dist/lib/agent-onboard";
 import type { AgentDefinition } from "./agent-defs";
 
 function makeAgent(overrides: Partial<AgentDefinition> = {}): AgentDefinition {
@@ -128,10 +128,10 @@ describe("handleAgentSetup guards", () => {
     const source = fs.readFileSync(path.join(import.meta.dirname, "agent-onboard.ts"), "utf-8");
 
     expect(source).toContain("verifyAgentBinaryAvailable");
-    expect(source).toContain(
-      'resolved="$(command -v ${shellQuote(executable)} 2>/dev/null || true)"',
-    );
-    expect(source).toContain('[ "$resolved" = ${shellQuote(binaryPath)} ]');
+    expect(source).toContain("[ -e ${shellQuote(binaryPath)} ]");
+    expect(source).toContain("[ -x ${shellQuote(binaryPath)} ]");
+    expect(source).not.toContain('[ -n "$resolved" ] || { echo not_found');
+    expect(source).not.toContain("path_mismatch");
     expect(source).toMatch(
       /"sandbox",\s*"exec",\s*"-n",\s*sandboxName,\s*"--",\s*"sh",\s*"-lc",\s*script/,
     );
@@ -150,4 +150,36 @@ describe("handleAgentSetup guards", () => {
     expect(source).toContain('parsed.status === "ok"');
     expect(source).not.toContain('.includes("ok")');
   });
+
+  it("accepts an executable configured binary path when PATH lookup is empty", () => {
+    let script = "";
+    const result = verifyAgentBinaryAvailable(
+      "alpha",
+      makeAgent({ name: "hermes", binary_path: "/usr/local/bin/hermes" }),
+      (args) => {
+        script = String(args[7] || "");
+        return "ok";
+      },
+    );
+
+    expect(result).toEqual({ available: true });
+    expect(script).toContain("[ -e '/usr/local/bin/hermes' ]");
+    expect(script).toContain("[ -x '/usr/local/bin/hermes' ]");
+    expect(script).not.toContain("command -v 'hermes'");
+  });
+
+  it("does not reject a configured binary when PATH resolves the symlink target", () => {
+    let script = "";
+    const result = verifyAgentBinaryAvailable(
+      "alpha",
+      makeAgent({ name: "hermes", binary_path: "/usr/local/bin/hermes" }),
+      (args) => {
+        script = String(args[7] || "");
+        return "ok";
+      },
+    );
+
+    expect(result).toEqual({ available: true });
+    expect(script).not.toContain("path_mismatch");
+  });
 });

src/lib/agent-onboard.ts

@@ -124,12 +124,12 @@ type AgentBinaryAvailability =
   | { available: true }
   | {
       available: false;
-      reason: "not_found" | "not_executable" | "path_mismatch";
+      reason: "not_found" | "not_executable";
       binaryPath?: string;
-      resolvedPath?: string;
     };
 
-function verifyAgentBinaryAvailable(
+// Exported for unit coverage of the sandbox-side guard without running onboarding.
+export function verifyAgentBinaryAvailable(
   sandboxName: string,
   agent: AgentDefinition,
   runCaptureOpenshell: OnboardContext["runCaptureOpenshell"],
@@ -138,10 +138,8 @@ function verifyAgentBinaryAvailable(
   const binaryPath = typeof agent.binary_path === "string" ? agent.binary_path.trim() : "";
   const script = binaryPath
     ? [
-        `resolved="$(command -v ${shellQuote(executable)} 2>/dev/null || true)"`,
-        `[ -n "$resolved" ] || { echo not_found; exit 1; }`,
+        `[ -e ${shellQuote(binaryPath)} ] || { echo not_found; exit 1; }`,
         `[ -x ${shellQuote(binaryPath)} ] || { echo not_executable; exit 1; }`,
-        `[ "$resolved" = ${shellQuote(binaryPath)} ] || { printf 'path_mismatch:%s\\n' "$resolved"; exit 1; }`,
         "echo ok",
       ].join(" && ")
     : `command -v ${shellQuote(executable)} >/dev/null 2>&1 && echo ok || echo not_found`;
@@ -156,15 +154,6 @@ function verifyAgentBinaryAvailable(
     return { available: true };
   }
   if (binaryPath && result) {
-    const mismatch = result.match(/path_mismatch:([^\n]+)/);
-    if (mismatch) {
-      return {
-        available: false,
-        reason: "path_mismatch",
-        binaryPath,
-        resolvedPath: mismatch[1].trim(),
-      };
-    }
     if (result.includes("not_executable")) {
       return { available: false, reason: "not_executable", binaryPath };
     }
@@ -178,9 +167,6 @@ function describeAgentBinaryFailure(
   result: Exclude<AgentBinaryAvailability, { available: true }>,
 ): string {
   const executable = agentExecutableName(agent);
-  if (result.reason === "path_mismatch") {
-    return `${agent.displayName} binary '${executable}' resolves to '${result.resolvedPath}', expected '${result.binaryPath}' inside sandbox '${sandboxName}'`;
-  }
   if (result.reason === "not_executable") {
     return `${agent.displayName} configured binary '${result.binaryPath}' is not executable inside sandbox '${sandboxName}'`;
   }

src/lib/agent-runtime.test.ts

@@ -3,7 +3,7 @@
 
 import { describe, it, expect } from "vitest";
 // Import from compiled dist/ so coverage is attributed correctly.
-import { buildRecoveryScript } from "../../dist/lib/agent-runtime";
+import { buildOpenClawRecoveryScript, buildRecoveryScript } from "../../dist/lib/agent-runtime";
 import type { AgentDefinition } from "./agent-defs";
 
 function makeAgent(overrides: Partial<AgentDefinition> = {}): AgentDefinition {
@@ -60,13 +60,13 @@ describe("buildRecoveryScript", () => {
   it("launches the default gateway command through the validated agent binary", () => {
     const script = buildRecoveryScript(minimalAgent, 19000);
     expect(script).toContain("command -v 'test-agent'");
-    expect(script).toContain('nohup "$AGENT_BIN" gateway run --port 19000');
+    expect(script).toContain('"$AGENT_BIN" gateway run --port 19000');
   });
 
   it("falls back to openclaw gateway run when gateway_command is absent", () => {
     const agent = makeAgent({ gateway_command: undefined });
     const script = buildRecoveryScript(agent, 19000);
-    expect(script).toContain('nohup "$AGENT_BIN" gateway run --port 19000');
+    expect(script).toContain('"$AGENT_BIN" gateway run --port 19000');
   });
 
   it("validates and launches custom gateway commands explicitly", () => {
@@ -123,26 +123,101 @@ describe("buildRecoveryScript", () => {
 
     it("writes the warning to gateway.log so it persists for sysadmin tail", () => {
       const script = buildRecoveryScript(minimalAgent, 19000);
-      // Both warnings must end up in /tmp/gateway.log, not just stderr —
+      // Both warnings must end up in the selected gateway log, not just stderr —
       // executeSandboxCommand silently discards stderr from the recovery
       // script, so a warning that only goes to stderr is invisible to
       // anyone debugging a crash-loop. (#2478)
-      expect(script).toContain('echo "$_W" >> /tmp/gateway.log');
+      expect(script).toContain('echo "$_W" >> "$_GATEWAY_LOG"');
       // And the warning must be deferred until AFTER gateway.log is
-      // freshly touched/chmod'd, otherwise the redirect targets a stale
-      // file that gets removed seconds later.
-      const touchIdx = script!.indexOf("touch /tmp/gateway.log");
-      const warnIdx = script!.indexOf('echo "$_W" >> /tmp/gateway.log');
-      expect(touchIdx).toBeLessThan(warnIdx);
+      // safely opened with O_NOFOLLOW, otherwise the redirect targets a
+      // stale or attacker-controlled file.
+      const gatewayPrepIdx = script!.indexOf(" /tmp/gateway.log || exit 1;");
+      const logSelectionIdx = script!.indexOf("_GATEWAY_LOG=/tmp/gateway.log");
+      const warnIdx = script!.indexOf('echo "$_W" >> "$_GATEWAY_LOG"');
+      expect(gatewayPrepIdx).toBeGreaterThanOrEqual(0);
+      expect(logSelectionIdx).toBeGreaterThanOrEqual(0);
+      expect(warnIdx).toBeGreaterThanOrEqual(0);
+      expect(gatewayPrepIdx).toBeLessThan(logSelectionIdx);
+      expect(logSelectionIdx).toBeLessThan(warnIdx);
+    });
+
+    it("stops recovery when hardened log setup fails", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      expect(script).toContain(" /tmp/gateway.log 'gateway' || exit 1;");
+      expect(script).toContain(" /tmp/auto-pair.log 'sandbox' || exit 1;");
     });
 
     it("appends (not truncates) gateway.log on launch so warnings survive", () => {
       const script = buildRecoveryScript(minimalAgent, 19000);
       // Truncating with `>` wipes the [gateway-recovery] WARNING that the
       // recovery script wrote moments earlier — meaning a sysadmin tailing
       // gateway.log would see the eventual crash without the explanation.
-      expect(script).toContain(">> /tmp/gateway.log 2>&1 &");
+      expect(script).toContain('>> "$_GATEWAY_LOG" 2>&1 &');
       expect(script).not.toMatch(/[^>]> \/tmp\/gateway\.log 2>&1 &/);
     });
+
+    it("preserves an existing gateway.log and has a writable fallback log", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      expect(script).not.toContain("rm -f /tmp/gateway.log");
+      expect(script).toContain("_GATEWAY_LOG=/tmp/gateway.log");
+      expect(script).toContain("_GATEWAY_LOG=/tmp/gateway-recovery.log");
+      expect(script).toContain('echo "$_W" >> "$_GATEWAY_LOG"');
+      expect(script).toContain('tail -5 "$_GATEWAY_LOG"');
+      expect(script).not.toContain('echo "$_W" >> /tmp/gateway.log');
+      expect(script).not.toContain("cat /tmp/gateway.log");
+    });
+
+    it("rejects a symlinked gateway.log before preparing the log", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      const noFollowIdx = script.indexOf("O_NOFOLLOW");
+      const openIdx = script.indexOf("os.open(path, flags, 0o644)");
+      const fchownIdx = script.indexOf("os.fchown(fd");
+      expect(script).toContain("refusing to prepare symlinked /tmp/gateway.log");
+      expect(script).toContain("sys.exit(1)");
+      expect(script).not.toContain(": > /tmp/gateway.log");
+      expect(script).not.toContain("chown 'gateway:gateway' /tmp/gateway.log");
+      expect(noFollowIdx).toBeGreaterThanOrEqual(0);
+      expect(openIdx).toBeGreaterThanOrEqual(0);
+      expect(fchownIdx).toBeGreaterThanOrEqual(0);
+      expect(noFollowIdx).toBeLessThan(openIdx);
+      expect(openIdx).toBeLessThan(fchownIdx);
+    });
+
+    it("prepares gateway.log for the real gateway-owned sandbox log", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      expect(script).toContain("os.fchown(fd");
+      expect(script).toContain("pw.pw_gid");
+      expect(script).not.toContain("grp.getgrnam");
+      expect(script).toContain("owner_mode = 0o644");
+      expect(script).toContain("os.fchmod(fd, owner_mode)");
+      expect(script).toContain("/tmp/gateway.log 'gateway'");
+      expect(script).toContain("gosu 'gateway'");
+    });
+
+    it("terminates the conditional launch branch before capturing the gateway pid", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      expect(script).toContain(" fi; GPID=$!");
+      expect(script).not.toContain(" fi GPID=$!");
+    });
+
+    it("prepares auto-pair.log without unlinking or following symlinks", () => {
+      const script = buildOpenClawRecoveryScript(18789);
+      expect(script).toContain("refusing to prepare symlinked /tmp/auto-pair.log");
+      expect(script).toContain("/tmp/auto-pair.log 'sandbox'");
+      expect(script).toContain("owner_mode = 0o600");
+      expect(script).not.toContain("rm -f /tmp/auto-pair.log");
+      expect(script).not.toContain(": > /tmp/auto-pair.log");
+      expect(script).not.toContain("touch /tmp/auto-pair.log");
+      expect(script).not.toContain("chown sandbox:sandbox /tmp/auto-pair.log");
+      expect(script).not.toContain("chmod 600 /tmp/auto-pair.log");
+    });
+
+    it("does not force non-OpenClaw agents to run as the gateway user", () => {
+      const script = buildRecoveryScript(minimalAgent, 19000);
+      expect(script).not.toContain("chown gateway:gateway /tmp/gateway.log");
+      expect(script).not.toContain("chown 'gateway:gateway' /tmp/gateway.log");
+      expect(script).not.toContain("gosu gateway");
+      expect(script).not.toContain("gosu 'gateway'");
+    });
   });
 });