Greptile Summary

This PR fixes gateway restart/status health checks by switching the daemon probe path from the heavier status RPC (via callGateway) to a lightweight health-only probe (via probeGateway with detailLevel: "health"), and hardens the security boundary around URL overrides so daemon-derived auth cannot silently satisfy explicit-auth requirements on CLI-supplied URL overrides.

Key changes:

• src/gateway/explicit-auth.ts (new): ExplicitGatewayAuth, resolveExplicitGatewayAuth, and ensureExplicitGatewayAuth extracted from call.ts so the daemon probe path doesn't pull in the full gateway call module.
• src/cli/daemon-cli/probe.ts: Replaces callGateway({ method: "status" }) with probeGateway({ detailLevel: "health" }), and adds two pre-dial security gates — explicit-auth enforcement for URL overrides, and isSecureWebSocketUrl rejection of plaintext non-loopback addresses.
• src/cli/daemon-cli/restart-health.ts: Introduces deadline-bounded probe timeouts (createRestartHealthDeadline / clampRestartProbeTimeoutMs). Per-probe timeout increases from 3 s to up to DEFAULT_LOCAL_GATEWAY_REACHABILITY_TIMEOUT_MS = 10 s, capped by remaining retry budget — giving hot-startup gateways more connection time while keeping overall wait bounded. Note: the deadline covers only inter-attempt delay, not probe execution time itself, so heavy-startup probes can shift the effective retry count.
• src/gateway/probe.ts: Adds "health" detailLevel, allowLoopbackDeviceIdentity flag, and tlsFingerprint parameter. The allowLoopbackDeviceIdentity flag lets trusted daemon loopback probes retain device identity without explicit credentials — correctly wired via probeUrlOverride === null in status.gather.ts.
• Test coverage is thorough for the new security boundary and the restart-health deadline logic; one minor gap exists for waitForGatewayHealthyListener's deadline-bounded timeout path (see inline comment).

Confidence Score: 4/5

• Safe to merge; no correctness bugs found. The increased per-probe timeout (3 s → 10 s) is an intentional behavioral change bounded by the new deadline mechanism, and the security hardening logic is correct.
• The refactoring is clean, the security invariants are well-reasoned and tested, and the deadline-bounded probe logic is correct. Score is 4 rather than 5 because: (1) the comment on disableDeviceIdentity in probe.ts is now misleading with the new allowLoopbackDeviceIdentity flag; (2) the waitForGatewayHealthyListener deadline-bounded timeout path lacks a focused test; and (3) the 3× increase in per-probe timeout is a meaningful behavioral change (though intentional and documented in the PR).
• src/cli/daemon-cli/restart-health.ts — review the deadline accounting comment and add a timeout-bounding test for waitForGatewayHealthyListener; src/gateway/probe.ts — update the disableDeviceIdentity comment to cover the new allowLoopbackDeviceIdentity case.

This is a comment left during a code review.
Path: src/gateway/probe.ts
Line: 50-55

Comment:
**Comment doesn't cover the new `allowLoopbackDeviceIdentity` path**

The block comment still reads "Local authenticated probes should stay device-bound", which only describes the `!(token || password)` branch. The new `allowLoopbackDeviceIdentity` flag introduces a second reason `disableDeviceIdentity` can be false for unauthenticated loopback probes. Future readers who see the comment may incorrectly conclude that unauthenticated loopback probes *always* disable device identity.

Consider updating the comment to describe both conditions:

```suggestion
      // Unauthenticated local probes disable device identity to avoid silently inflating
      // the probe's privilege via device-token fallback.
      // `allowLoopbackDeviceIdentity: true` opts the caller back in for trusted daemon
      // loopback probes where device identity is the intended auth mechanism.
      return (
        isLoopbackHost(hostname) &&
        !(opts.auth?.token || opts.auth?.password) &&
        opts.allowLoopbackDeviceIdentity !== true
      );
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/daemon-cli/restart-health.ts
Line: 74-77

Comment:
**Deadline doesn't account for initial probe duration**

`createRestartHealthDeadline` budgets exactly `attempts × delayMs`, but `waitForGatewayHealthyRestart` (and `waitForGatewayHealthyListener`) perform an *initial probe before the first sleep*. With default values (`DEFAULT_LOCAL_GATEWAY_REACHABILITY_TIMEOUT_MS = 10 000 ms`) the first probe can consume up to 10 s of a 60 s budget before any sleep occurs, so the effective number of retry probes that will actually fire can be lower than the configured `attempts`.

This won't cause incorrect behaviour (the budget still terminates the loop safely), but it may be surprising to callers who set small `attempts × delayMs` windows expecting a fixed retry count. A doc-comment noting that the deadline covers delays only (not probe execution time) would help future maintainers understand the trade-off.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/cli/daemon-cli/restart-health.ts
Line: 278-297

Comment:
**`waitForGatewayHealthyListener` has no test covering deadline-bounded timeouts**

`waitForGatewayHealthyRestart` gains a focused test (`"bounds restart probe timeouts to the remaining retry budget"`) that asserts how `probeGateway` timeouts shrink across iterations. `waitForGatewayHealthyListener` received identical deadline threading (`createRestartHealthDeadline` + `getRemainingRestartBudgetMs`) but has no corresponding test.

While the implementation looks correct, the symmetry gap means a future refactoring that accidentally breaks the listener deadline path (e.g. forgetting to thread `probeTimeoutMs` through) would go undetected. A parallel test for `waitForGatewayHealthyListener` matching the pattern at line 299–345 of `restart-health.test.ts` would close this coverage gap.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: bbcb796}

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🟠 OPENCLAW_ALLOW_INSECURE_PRIVATE_WS allows plaintext ws:// to arbitrary hostnames (security check bypass)

Description

isSecureWebSocketUrl() is intended to allow plaintext ws:// only for loopback by default, with an opt-in “break-glass” mode (allowPrivateWs) for private networks only.

However, when allowPrivateWs is enabled, any non-IP hostname is accepted, including public domains (e.g. ws://evil.com) and deceptive userinfo URLs (e.g. ws://127.0.0.1@​evil.com). This undermines the new CLI-side enforcement (and other call sites) by permitting cleartext WebSocket connections to non-private remote hosts, exposing credentials and chat data to interception.

Why this happens:

• For ws:// URLs, if the host is not loopback and not a private IP literal, the code returns net.isIP(hostForIpCheck) === 0.
• net.isIP(x) === 0 means “x is not an IP address” (i.e., it is a hostname), so the function returns true for essentially all hostnames.

Vulnerable code:

if (opts?.allowPrivateWs) {
  if (isPrivateOrLoopbackHost(parsed.hostname)) {
    return true;
  }
  const hostForIpCheck =
    parsed.hostname.startsWith("[") && parsed.hostname.endsWith("]")
      ? parsed.hostname.slice(1, -1)
      : parsed.hostname;
  return net.isIP(hostForIpCheck) === 0; // <-- hostnames always pass
}

Recommendation

Tighten the break-glass behavior so it does not implicitly allow all hostnames.

Minimum safe fix (recommended): when allowPrivateWs is enabled, only allow private/loopback IP literals (and optionally a small, explicit hostname allowlist if you truly need it). For everything else, require wss://.

Example:

if (opts?.allowPrivateWs) {
  ​// Allow private/loopback *IP literals* only
  if (isPrivateOrLoopbackHost(parsed.hostname)) {
    return true;
  }
  return false; // hostnames must use wss://
}

If hostname support is required for private DNS (VPN/tailnet), introduce an async validator that resolves DNS and verifies that all returned A/AAAA records are private/loopback and ensure the WebSocket connection uses the resolved IP (to mitigate DNS rebinding / TOCTOU).

2. 🟠 Implicit device-token leakage to non-loopback URLs during unauthenticated gateway probes

Description

probeGateway() only disables device identity for unauthenticated loopback URLs. For any non-loopback URL (including attacker-controlled wss://...) when opts.auth is omitted/empty, it leaves deviceIdentity enabled (passed as undefined).

Because GatewayClient treats deviceIdentity: undefined as “load or create a real device identity”, the connect handshake can:

• Send a stable device identifier/public key (tracking) to the remote endpoint
• Load a cached device token from disk and send it in the connect auth fields (implicit credential disclosure)

This violates the security invariant “don’t send device identity / cached device token to untrusted remote endpoints unless explicitly intended”.

Vulnerable logic (probe keeps identity enabled for non-loopback, unauthenticated URLs):

return (
  isLoopbackHost(hostname) &&
  !(opts.auth?.token || opts.auth?.password) &&
  opts.allowLoopbackDeviceIdentity !== true
);
...
deviceIdentity: disableDeviceIdentity ? null : undefined,

Why this leaks credentials (data flow):

• Input: opts.url is supplied by callers such as gateway discovery/status commands and can be user-controlled.
• Condition: when no explicit token/password is provided for a non-loopback URL, disableDeviceIdentity becomes false.
• Sink: GatewayClient loads device identity and may load a cached device token and include it in the WebSocket connect handshake auth payload sent to that URL.

Impact:

• Attacker-operated wss:// endpoint can collect the client’s device identity and cached device token.
• That token can enable authentication attempts or real-time credential forwarding attacks against the legitimate gateway, and at minimum discloses a sensitive credential to an untrusted party.

Recommendation

Make probes opt-out of device identity (and thus device-token fallback) by default, and only opt-in for explicitly trusted cases.

Recommended change in probeGateway():

• Disable device identity whenever explicit shared credentials are not provided, unless the caller explicitly opts in and the endpoint is trusted (e.g., loopback, or wss:// with a pinned tlsFingerprint).

Example:

const parsed = new URL(opts.url);
const isLoopback = isLoopbackHost(parsed.hostname);
const hasExplicitSharedAuth = Boolean(opts.auth?.token || opts.auth?.password);

​// Only allow device identity when explicitly intended.
const allowDeviceIdentity =
  hasExplicitSharedAuth ||
  (isLoopback && opts.allowLoopbackDeviceIdentity === true);

const client = new GatewayClient({
  ...,
  ​// Passing null disables identity; undefined loads/creates identity.
  deviceIdentity: allowDeviceIdentity ? undefined : null,
});

Optionally (defense-in-depth): update GatewayClient.selectConnectAuth() to never use stored device-token fallback unless an explicit allowDeviceTokenFallback flag is set and the URL is trusted.

Analyzed PR: #48472 at commit cbbec1b

_{Last updated on: 2026-03-17T15:28:11Z}

@joshavant @visionik is this something that follows under your radar? Thanks

@greptileai please review

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR should stay open as maintainer review material, not as a cleanup close: current main has later gateway auth/status hardening that the stale branch conflicts with, while the distinct timeout/health-probe idea still needs a narrow rebase and real behavior proof before it can land.

Canonical path: Close this PR as superseded by #72405.

So I’m closing this here and keeping the remaining discussion on #72405.

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

ClawSweeper applied the proposed close for this PR.

• Action: closed this PR.
• Close reason: duplicate or superseded.
• Evidence: durable ClawSweeper review.