From 1fbaf7bf102eb734c3b6a867913662347c2254f5 Mon Sep 17 00:00:00 2001 From: heavenlost Date: Fri, 20 Mar 2026 22:22:50 +0800 Subject: [PATCH] fix(gateway): restore loopback detail probes and identity fallback --- src/commands/gateway-status.ts | 2 +- src/commands/gateway-status/helpers.test.ts | 42 ++++++++++++++++++++ src/commands/gateway-status/helpers.ts | 26 +++++++++++-- src/gateway/call.test.ts | 32 ++++++++++++++- src/gateway/call.ts | 26 ++++++------- src/gateway/probe.test.ts | 43 +++++++++++++++++++-- src/gateway/probe.ts | 17 ++++++-- 7 files changed, 161 insertions(+), 27 deletions(-) diff --git a/src/commands/gateway-status.ts b/src/commands/gateway-status.ts index ecdeeaa9570..c338d7fe55b 100644 --- a/src/commands/gateway-status.ts +++ b/src/commands/gateway-status.ts @@ -176,7 +176,7 @@ export async function gatewayStatusCommand( token: authResolution.token, password: authResolution.password, }; - const timeoutMs = resolveProbeBudgetMs(overallTimeoutMs, target.kind); + const timeoutMs = resolveProbeBudgetMs(overallTimeoutMs, target); const probe = await probeGateway({ url: target.url, auth, diff --git a/src/commands/gateway-status/helpers.test.ts b/src/commands/gateway-status/helpers.test.ts index e0c1ecee763..65d61154c2d 100644 --- a/src/commands/gateway-status/helpers.test.ts +++ b/src/commands/gateway-status/helpers.test.ts @@ -5,6 +5,7 @@ import { isProbeReachable, isScopeLimitedProbeFailure, renderProbeSummaryLine, + resolveProbeBudgetMs, resolveAuthForTarget, } from "./helpers.js"; @@ -273,3 +274,44 @@ describe("probe reachability classification", () => { expect(renderProbeSummaryLine(probe, false)).toContain("RPC: failed"); }); }); + +describe("resolveProbeBudgetMs", () => { + it("gives loopback probes enough time for detail RPCs", () => { + expect( + resolveProbeBudgetMs(10_000, { + kind: "localLoopback", + url: "ws://127.0.0.1:18789", + }), + ).toBe(3000); + expect( + resolveProbeBudgetMs(1200, { + kind: "localLoopback", + url: "ws://127.0.0.1:18789", + }), + ).toBe(1200); + expect( + resolveProbeBudgetMs(10_000, { + kind: "explicit", + url: "ws://127.0.0.1:18789", + }), + ).toBe(3000); + expect( + resolveProbeBudgetMs(10_000, { + kind: "explicit", + url: "wss://localhost:18789/ws", + }), + ).toBe(3000); + expect( + resolveProbeBudgetMs(10_000, { + kind: "explicit", + url: "wss://gateway.example/ws", + }), + ).toBe(1500); + expect( + resolveProbeBudgetMs(10_000, { + kind: "sshTunnel", + url: "wss://gateway.example/ws", + }), + ).toBe(2000); + }); +}); diff --git a/src/commands/gateway-status/helpers.ts b/src/commands/gateway-status/helpers.ts index 5f1a5e2f5ee..727eafc31f4 100644 --- a/src/commands/gateway-status/helpers.ts +++ b/src/commands/gateway-status/helpers.ts @@ -3,6 +3,7 @@ import { resolveGatewayPort } from "../../config/config.js"; import type { OpenClawConfig, ConfigFileSnapshot } from "../../config/types.js"; import { hasConfiguredSecretInput } from "../../config/types.secrets.js"; import { readGatewayPasswordEnv, readGatewayTokenEnv } from "../../gateway/credentials.js"; +import { isLoopbackHost } from "../../gateway/net.js"; import type { GatewayProbeResult } from "../../gateway/probe.js"; import { resolveConfiguredSecretInputString } from "../../gateway/resolve-configured-secret-input-string.js"; import { pickPrimaryTailnetIPv4 } from "../../infra/tailnet.js"; @@ -116,11 +117,28 @@ export function resolveTargets(cfg: OpenClawConfig, explicitUrl?: string): Gatew return targets; } -export function resolveProbeBudgetMs(overallMs: number, kind: TargetKind): number { - if (kind === "localLoopback") { - return Math.min(800, overallMs); +function isLoopbackProbeTarget(target: Pick): boolean { + if (target.kind === "localLoopback") { + return true; } - if (kind === "sshTunnel") { + try { + return isLoopbackHost(new URL(target.url).hostname); + } catch { + return false; + } +} + +export function resolveProbeBudgetMs( + overallMs: number, + target: Pick, +): number { + if (isLoopbackProbeTarget(target)) { + // Full localhost detail probes can take longer than the old short budgets, + // especially when they exercise status + heartbeat + presence RPCs. + // Treat explicit loopback URLs the same way as discovered local loopback. + return Math.min(3000, overallMs); + } + if (target.kind === "sshTunnel") { return Math.min(2000, overallMs); } return Math.min(1500, overallMs); diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index 5ffd3ce3c51..8cfa5b0ffac 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -8,6 +8,11 @@ import { resolveGatewayPortMock as resolveGatewayPort, } from "./gateway-connection.test-mocks.js"; +const deviceIdentityState = vi.hoisted(() => ({ + value: { id: "test-device-identity" } as Record, + throwOnLoad: false, +})); + let lastClientOptions: { url?: string; token?: string; @@ -73,6 +78,15 @@ vi.mock("./client.js", () => ({ }, })); +vi.mock("../infra/device-identity.js", () => ({ + loadOrCreateDeviceIdentity: () => { + if (deviceIdentityState.throwOnLoad) { + throw new Error("read-only identity dir"); + } + return deviceIdentityState.value; + }, +})); + const { buildGatewayConnectionDetails, callGateway, callGatewayCli, callGatewayScoped } = await import("./call.js"); @@ -87,6 +101,7 @@ function resetGatewayCallMocks() { closeCode = 1006; closeReason = ""; helloMethods = ["health", "secrets.resolve"]; + deviceIdentityState.throwOnLoad = false; } function setGatewayNetworkDefaults(port = 18789) { @@ -219,7 +234,22 @@ describe("callGateway url resolution", () => { expect(lastClientOptions?.url).toBe("ws://127.0.0.1:18789"); expect(lastClientOptions?.token).toBe("explicit-token"); - expect(lastClientOptions?.deviceIdentity).toBeDefined(); + expect(lastClientOptions?.deviceIdentity).toEqual(deviceIdentityState.value); + }); + + it("falls back to token/password auth when device identity cannot be persisted", async () => { + setLocalLoopbackGatewayConfig(); + deviceIdentityState.throwOnLoad = true; + + await callGateway({ + method: "health", + token: "explicit-token", + }); + + expect(lastClientOptions?.url).toBe("ws://127.0.0.1:18789"); + expect(lastClientOptions?.token).toBe("explicit-token"); + expect(lastClientOptions?.deviceIdentity).toBeNull(); + expect(lastRequestOptions?.method).toBe("health"); }); it("uses OPENCLAW_GATEWAY_URL env override in remote mode when remote URL is missing", async () => { diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 98793dd4071..74c3fad9490 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -81,17 +81,17 @@ export type GatewayConnectionDetails = { message: string; }; -function shouldAttachDeviceIdentityForGatewayCall(params: { - url: string; - token?: string; - password?: string; -}): boolean { - void params; - // Shared-auth local calls used to skip device identity as an optimization, but - // device-less operator connects now have their self-declared scopes stripped. - // Keep identity enabled so local authenticated calls stay device-bound and - // retain their least-privilege scopes. - return true; +function resolveDeviceIdentityForGatewayCall() { + // Shared-auth local calls should still stay device-bound so operator scopes + // remain available for detail RPCs such as status / system-presence / + // last-heartbeat. + try { + return loadOrCreateDeviceIdentity(); + } catch { + // Read-only or restricted environments should still be able to call the + // gateway with token/password auth without crashing before the RPC. + return null; + } } export type ExplicitGatewayAuth = { @@ -828,9 +828,7 @@ async function executeGatewayRequestWithScopes(params: { mode: opts.mode ?? GATEWAY_CLIENT_MODES.CLI, role: "operator", scopes, - deviceIdentity: shouldAttachDeviceIdentityForGatewayCall({ url, token, password }) - ? loadOrCreateDeviceIdentity() - : undefined, + deviceIdentity: resolveDeviceIdentityForGatewayCall(), minProtocol: opts.minProtocol ?? PROTOCOL_VERSION, maxProtocol: opts.maxProtocol ?? PROTOCOL_VERSION, onHelloOk: async (hello) => { diff --git a/src/gateway/probe.test.ts b/src/gateway/probe.test.ts index 4a2374e17cb..9f214a21215 100644 --- a/src/gateway/probe.test.ts +++ b/src/gateway/probe.test.ts @@ -1,10 +1,15 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; const gatewayClientState = vi.hoisted(() => ({ options: null as Record | null, requests: [] as string[], })); +const deviceIdentityState = vi.hoisted(() => ({ + value: { id: "test-device-identity" } as Record, + throwOnLoad: false, +})); + class MockGatewayClient { private readonly opts: Record; @@ -40,9 +45,21 @@ vi.mock("./client.js", () => ({ GatewayClient: MockGatewayClient, })); +vi.mock("../infra/device-identity.js", () => ({ + loadOrCreateDeviceIdentity: () => { + if (deviceIdentityState.throwOnLoad) { + throw new Error("read-only identity dir"); + } + return deviceIdentityState.value; + }, +})); + const { probeGateway } = await import("./probe.js"); describe("probeGateway", () => { + beforeEach(() => { + deviceIdentityState.throwOnLoad = false; + }); it("connects with operator.read scope", async () => { const result = await probeGateway({ url: "ws://127.0.0.1:18789", @@ -51,7 +68,7 @@ describe("probeGateway", () => { }); expect(gatewayClientState.options?.scopes).toEqual(["operator.read"]); - expect(gatewayClientState.options?.deviceIdentity).toBeUndefined(); + expect(gatewayClientState.options?.deviceIdentity).toEqual(deviceIdentityState.value); expect(gatewayClientState.requests).toEqual([ "health", "status", @@ -68,7 +85,7 @@ describe("probeGateway", () => { timeoutMs: 1_000, }); - expect(gatewayClientState.options?.deviceIdentity).toBeUndefined(); + expect(gatewayClientState.options?.deviceIdentity).toEqual(deviceIdentityState.value); }); it("keeps device identity disabled for unauthenticated loopback probes", async () => { @@ -88,9 +105,29 @@ describe("probeGateway", () => { }); expect(result.ok).toBe(true); + expect(gatewayClientState.options?.deviceIdentity).toBeNull(); expect(gatewayClientState.requests).toEqual([]); }); + it("falls back to token/password auth when device identity cannot be persisted", async () => { + deviceIdentityState.throwOnLoad = true; + + const result = await probeGateway({ + url: "ws://127.0.0.1:18789", + auth: { token: "secret" }, + timeoutMs: 1_000, + }); + + expect(result.ok).toBe(true); + expect(gatewayClientState.options?.deviceIdentity).toBeNull(); + expect(gatewayClientState.requests).toEqual([ + "health", + "status", + "system-presence", + "config.get", + ]); + }); + it("fetches only presence for presence-only probes", async () => { const result = await probeGateway({ url: "ws://127.0.0.1:18789", diff --git a/src/gateway/probe.ts b/src/gateway/probe.ts index bbd36639b78..d18d7ad4e67 100644 --- a/src/gateway/probe.ts +++ b/src/gateway/probe.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { loadOrCreateDeviceIdentity } from "../infra/device-identity.js"; import { formatErrorMessage } from "../infra/errors.js"; import type { SystemPresence } from "../infra/system-presence.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; @@ -42,14 +43,22 @@ export async function probeGateway(opts: { let connectError: string | null = null; let close: GatewayProbeClose | null = null; - const disableDeviceIdentity = (() => { + const deviceIdentity = (() => { + if (opts.includeDetails === false) { + return null; + } try { const hostname = new URL(opts.url).hostname; // Local authenticated probes should stay device-bound so read/detail RPCs // are not scope-limited by the shared-auth scope stripping hardening. - return isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password); + if (isLoopbackHost(hostname) && !(opts.auth?.token || opts.auth?.password)) { + return null; + } + return loadOrCreateDeviceIdentity(); } catch { - return false; + // Read-only or restricted environments should still be able to run + // token/password-auth detail probes without crashing on identity persistence. + return null; } })(); @@ -76,7 +85,7 @@ export async function probeGateway(opts: { clientVersion: "dev", mode: GATEWAY_CLIENT_MODES.PROBE, instanceId, - deviceIdentity: disableDeviceIdentity ? null : undefined, + deviceIdentity, onConnectError: (err) => { connectError = formatErrorMessage(err); },