From f385491c230cc0f97177fcce3e63d5bfd1078c8b Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:18:22 -0500 Subject: [PATCH] fix: clarify gateway SecretRef auth diagnostics (#92290) * fix gateway secretref health diagnostics * fix gateway health result type narrowing --- .../src/cli/browser-cli-manage.test.ts | 20 +++++ .../browser/src/cli/browser-cli-manage.ts | 20 ++++- src/cli/gateway-cli.coverage.test.ts | 10 +++ src/cli/gateway-cli/register.ts | 44 ++++++++-- .../channels.status.command-flow.test.ts | 25 ++++++ src/commands/channels/status-config-format.ts | 6 +- src/commands/channels/status.ts | 14 +++- src/commands/doctor-gateway-health.test.ts | 39 +++++++++ src/commands/doctor-gateway-health.ts | 7 +- src/commands/health.test.ts | 38 +++++++++ src/commands/health.ts | 81 +++++++++++++------ 11 files changed, 267 insertions(+), 37 deletions(-) diff --git a/extensions/browser/src/cli/browser-cli-manage.test.ts b/extensions/browser/src/cli/browser-cli-manage.test.ts index 29db03ea0271..d9f068817407 100644 --- a/extensions/browser/src/cli/browser-cli-manage.test.ts +++ b/extensions/browser/src/cli/browser-cli-manage.test.ts @@ -461,4 +461,24 @@ describe("browser manage output", () => { expect(output).toContain("OK gateway: browser control endpoint reachable"); expect(output).toContain("OK tabs: 1 visible, use tab reference t1"); }); + + it("prints a readable browser doctor failure when gateway auth SecretRefs are unavailable", async () => { + const error = Object.assign(new Error("gateway.auth.password unavailable"), { + code: "GATEWAY_SECRET_REF_UNAVAILABLE", + name: "GatewaySecretRefUnavailableError", + }); + getBrowserManageCallBrowserRequestMock().mockRejectedValueOnce(error); + + const program = createBrowserManageProgram(); + await expect(program.parseAsync(["browser", "doctor"], { from: "user" })).rejects.toThrow( + "__exit__:1", + ); + + const output = lastRuntimeLog(); + expect(output).toContain( + "FAIL gateway: Gateway auth SecretRef is unavailable in this command path", + ); + expect(output).toContain("OPENCLAW_GATEWAY_TOKEN"); + expect(output).not.toContain("GatewaySecretRefUnavailableError"); + }); }); diff --git a/extensions/browser/src/cli/browser-cli-manage.ts b/extensions/browser/src/cli/browser-cli-manage.ts index 698da14da189..d80d7ef3132e 100644 --- a/extensions/browser/src/cli/browser-cli-manage.ts +++ b/extensions/browser/src/cli/browser-cli-manage.ts @@ -152,6 +152,24 @@ function formatDoctorLine(check: BrowserDoctorCheck): string { return `${check.ok ? "OK" : "FAIL"} ${check.name}${check.detail ? `: ${check.detail}` : ""}`; } +function isGatewaySecretRefUnavailableErrorShape(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + const errorRecord = error as Error & { code?: unknown }; + return ( + errorRecord.name === "GatewaySecretRefUnavailableError" || + errorRecord.code === "GATEWAY_SECRET_REF_UNAVAILABLE" + ); +} + +function formatBrowserDoctorGatewayError(error: unknown): string { + if (!isGatewaySecretRefUnavailableErrorShape(error)) { + return String(error); + } + return "Gateway auth SecretRef is unavailable in this command path; browser doctor cannot reach the admin-scoped browser.request endpoint. Set OPENCLAW_GATEWAY_TOKEN or OPENCLAW_GATEWAY_PASSWORD, then retry."; +} + async function runBrowserDoctor(parent: BrowserParentOpts, profile?: string, deep?: boolean) { const checks: BrowserDoctorCheck[] = []; let status: BrowserStatus | null; @@ -167,7 +185,7 @@ async function runBrowserDoctor(parent: BrowserParentOpts, profile?: string, dee checks.push({ name: "gateway", ok: false, - detail: String(err), + detail: formatBrowserDoctorGatewayError(err), }); return { ok: false, checks }; } diff --git a/src/cli/gateway-cli.coverage.test.ts b/src/cli/gateway-cli.coverage.test.ts index 24f9d3f9041c..c48e380ad394 100644 --- a/src/cli/gateway-cli.coverage.test.ts +++ b/src/cli/gateway-cli.coverage.test.ts @@ -45,8 +45,18 @@ const { runtimeLogs, runtimeErrors, defaultRuntime } = mocks; vi.mock( new URL("../../gateway/call.ts", new URL("./gateway-cli/call.ts", import.meta.url)).href, () => ({ + buildGatewayConnectionDetails: () => ({ + message: "Gateway mode: local\nGateway target: ws://127.0.0.1:18789", + url: "ws://127.0.0.1:18789", + }), + buildGatewayProbeConnectionDetails: () => ({ + preauthHandshakeTimeoutMs: 1000, + tlsFingerprint: undefined, + url: "ws://127.0.0.1:18789", + }), callGateway: (opts: unknown) => callGateway(opts), formatGatewayTransportErrorJson: (error: unknown) => formatGatewayTransportErrorJson(error), + isGatewayCredentialsRequiredError: () => false, randomIdempotencyKey: () => "rk_test", }), ); diff --git a/src/cli/gateway-cli/register.ts b/src/cli/gateway-cli/register.ts index 02811af4a60b..7ccbfda3ba18 100644 --- a/src/cli/gateway-cli/register.ts +++ b/src/cli/gateway-cli/register.ts @@ -140,6 +140,19 @@ function parseDaysOption(raw: unknown, fallback = 30): number { return fallback; } +function parseGatewayRpcTimeoutOption(raw: unknown, fallback = 10_000): number { + if (typeof raw === "number" && Number.isFinite(raw) && raw > 0) { + return Math.floor(raw); + } + if (typeof raw === "string" && raw.trim() !== "") { + const parsed = parseStrictPositiveInteger(raw); + if (parsed !== undefined) { + return parsed; + } + } + return fallback; +} + function resolveGatewayRpcOptions( opts: T, command?: Command, @@ -534,17 +547,36 @@ export function registerGatewayCli(program: Command) { await runGatewayCommand( async () => { const rpcOpts = resolveGatewayRpcOptions(opts, command); - const [{ formatHealthChannelLines }, { styleHealthChannelLine }] = await Promise.all([ - loadGatewayHealthModule(), - loadHealthStyleModule(), - ]); - const result = await callGatewayCli("health", rpcOpts); + const [ + { emitReachableGatewayAuthDiagnostic, formatHealthChannelLines }, + { styleHealthChannelLine }, + ] = await Promise.all([loadGatewayHealthModule(), loadHealthStyleModule()]); + let result: unknown; + try { + result = await callGatewayCli("health", rpcOpts); + } catch (error) { + const { readBestEffortConfig } = await loadConfigModule(); + const handled = await emitReachableGatewayAuthDiagnostic({ + error, + config: await readBestEffortConfig(), + runtime: defaultRuntime, + timeoutMs: parseGatewayRpcTimeoutOption(rpcOpts.timeout), + token: rpcOpts.token, + password: rpcOpts.password, + json: Boolean(rpcOpts.json), + }); + if (handled) { + return; + } + throw error; + } if (rpcOpts.json) { defaultRuntime.writeJson(result); return; } const rich = isRich(); - const obj: Record = result && typeof result === "object" ? result : {}; + const obj: Record = + result && typeof result === "object" ? (result as Record) : {}; const durationMs = typeof obj.durationMs === "number" ? obj.durationMs : null; defaultRuntime.log(colorize(rich, theme.heading, "Gateway Health")); defaultRuntime.log( diff --git a/src/commands/channels.status.command-flow.test.ts b/src/commands/channels.status.command-flow.test.ts index b86b22cacb18..6fc402cd3498 100644 --- a/src/commands/channels.status.command-flow.test.ts +++ b/src/commands/channels.status.command-flow.test.ts @@ -1,5 +1,6 @@ // Channels status command-flow tests cover gateway calls, config fallback, and timeout validation. import { beforeEach, describe, expect, it, vi } from "vitest"; +import { GatewaySecretRefUnavailableError } from "../gateway/credentials.js"; import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js"; import { channelsStatusCommand } from "./channels/status.js"; import { createCapturingTestRuntime } from "./test-runtime-config-helpers.js"; @@ -267,6 +268,29 @@ describe("channelsStatusCommand SecretRef fallback flow", () => { expect(joined).toContain("token:config (unavailable)"); }); + it("labels config-only fallback as auth-unavailable when gateway auth SecretRefs are unresolved", async () => { + mocks.callGateway.mockRejectedValue( + new GatewaySecretRefUnavailableError("gateway.auth.password"), + ); + mocks.requireValidConfigSnapshot.mockResolvedValue({ secretResolved: false, channels: {} }); + mocks.resolveCommandConfigWithSecrets.mockResolvedValue({ + resolvedConfig: { secretResolved: false, channels: {} }, + effectiveConfig: { secretResolved: false, channels: {} }, + diagnostics: [], + }); + const { runtime, logs, errors } = createCapturingTestRuntime(); + + await channelsStatusCommand({ probe: false }, runtime as never); + + const errorOutput = errors.join("\n"); + expect(errorOutput).toContain("Gateway auth unavailable"); + expect(errorOutput).not.toContain("Gateway not reachable"); + const joined = logs.join("\n"); + expect(joined).toContain("Gateway auth unavailable; showing config-only status."); + expect(joined).not.toContain("Gateway not reachable; showing config-only status."); + expect(joined).toContain("configured, secret unavailable in this command path"); + }); + it("prefers resolved snapshots when command-local SecretRef resolution succeeds", async () => { mocks.callGateway.mockRejectedValue(new Error("gateway closed")); mocks.requireValidConfigSnapshot.mockResolvedValue({ secretResolved: false, channels: {} }); @@ -349,6 +373,7 @@ describe("channelsStatusCommand SecretRef fallback flow", () => { expect(payload.error).not.toContain("fallback-user:fallback-pass"); expect(payload.error).not.toContain("fallback-secret"); expect(payload.gatewayReachable).toBe(false); + expect(payload.gatewayAuthUnavailable).toBe(false); expect(payload.configOnly).toBe(true); expect(payload.configuredChannels).toStrictEqual([]); }); diff --git a/src/commands/channels/status-config-format.ts b/src/commands/channels/status-config-format.ts index eeedaa320843..5a111454ea08 100644 --- a/src/commands/channels/status-config-format.ts +++ b/src/commands/channels/status-config-format.ts @@ -36,10 +36,12 @@ type ChannelStatusPluginLabel = { export async function formatConfigChannelsStatusLines( cfg: OpenClawConfig, meta: { path?: string; mode?: "local" | "remote" }, - opts?: { sourceConfig?: OpenClawConfig; channel?: string }, + opts?: { sourceConfig?: OpenClawConfig; channel?: string; fallbackReason?: string }, ): Promise { const lines: string[] = []; - lines.push(theme.warn("Gateway not reachable; showing config-only status.")); + lines.push( + theme.warn(opts?.fallbackReason ?? "Gateway not reachable; showing config-only status."), + ); if (meta.path) { lines.push(`Config: ${meta.path}`); } diff --git a/src/commands/channels/status.ts b/src/commands/channels/status.ts index e3bdb0096f88..7e071e378acc 100644 --- a/src/commands/channels/status.ts +++ b/src/commands/channels/status.ts @@ -10,6 +10,7 @@ import { parseTimeoutMsWithFallback } from "../../cli/parse-timeout.js"; import { withProgress } from "../../cli/progress.js"; import { readConfigFileSnapshot } from "../../config/config.js"; import { callGateway } from "../../gateway/call.js"; +import { isGatewaySecretRefUnavailableError } from "../../gateway/credentials.js"; import { collectChannelStatusIssues } from "../../infra/channels-status-issues.js"; import { formatErrorMessage } from "../../infra/errors.js"; import { formatTimeAgo } from "../../infra/format-time/format-relative.ts"; @@ -213,7 +214,7 @@ export function formatGatewayChannelsStatusLines(payload: Record vi.fn()); const isGatewayCredentialsRequiredError = vi.hoisted(() => vi.fn(() => false)); +const isGatewaySecretRefUnavailableError = vi.hoisted(() => vi.fn(() => false)); const probeGatewayStatus = vi.hoisted(() => vi.fn()); const note = vi.hoisted(() => vi.fn()); const TEST_GATEWAY_URL = "ws://127.0.0.1:18789"; @@ -28,6 +29,10 @@ vi.mock("../gateway/call.js", () => ({ isGatewayCredentialsRequiredError, })); +vi.mock("../gateway/credentials.js", () => ({ + isGatewaySecretRefUnavailableError, +})); + vi.mock("../cli/daemon-cli/probe.js", () => ({ probeGatewayStatus, })); @@ -49,6 +54,8 @@ describe("checkGatewayHealth", () => { callGateway.mockReset(); isGatewayCredentialsRequiredError.mockReset(); isGatewayCredentialsRequiredError.mockReturnValue(false); + isGatewaySecretRefUnavailableError.mockReset(); + isGatewaySecretRefUnavailableError.mockReturnValue(false); probeGatewayStatus.mockReset(); note.mockReset(); }); @@ -143,6 +150,38 @@ describe("checkGatewayHealth", () => { ); expect(callGateway).toHaveBeenCalledTimes(1); }); + + it("reports credentials-required when status RPC auth SecretRefs are unavailable", async () => { + const error = new Error("gateway.auth.password unavailable"); + callGateway.mockRejectedValueOnce(error); + isGatewaySecretRefUnavailableError.mockReturnValueOnce(true); + probeGatewayStatus.mockResolvedValueOnce({ + ok: false, + kind: "connect", + error: TEST_AUTH_CLOSE_ERROR, + }); + const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; + + await expect( + checkGatewayHealth({ runtime: runtime as never, cfg, timeoutMs: 3000 }), + ).resolves.toEqual({ authenticated: false, healthOk: true }); + + expect(isGatewaySecretRefUnavailableError).toHaveBeenCalledWith(error); + expect(probeGatewayStatus).toHaveBeenCalledWith({ + url: TEST_GATEWAY_URL, + timeoutMs: 3000, + tlsFingerprint: TEST_TLS_FINGERPRINT, + preauthHandshakeTimeoutMs: 4321, + config: cfg, + json: true, + }); + expect(runtime.error).not.toHaveBeenCalled(); + expect(note).toHaveBeenCalledWith( + GATEWAY_HEALTH_CREDENTIALS_REQUIRED_MESSAGE, + GATEWAY_HEALTH_CREDENTIALS_REQUIRED_TITLE, + ); + expect(callGateway).toHaveBeenCalledTimes(1); + }); }); describe("probeGatewayMemoryStatus", () => { diff --git a/src/commands/doctor-gateway-health.ts b/src/commands/doctor-gateway-health.ts index 103b7677eaee..597385810eff 100644 --- a/src/commands/doctor-gateway-health.ts +++ b/src/commands/doctor-gateway-health.ts @@ -8,6 +8,7 @@ import { callGateway, isGatewayCredentialsRequiredError, } from "../gateway/call.js"; +import { isGatewaySecretRefUnavailableError } from "../gateway/credentials.js"; import type { DoctorMemoryStatusPayload } from "../gateway/server-methods/doctor.js"; import { collectChannelStatusIssues } from "../infra/channels-status-issues.js"; import { formatErrorMessage } from "../infra/errors.js"; @@ -38,6 +39,10 @@ function isGatewayCallTimeout(message: string): boolean { return /^gateway timeout after \d+ms(?:\n|$)/.test(message); } +function isGatewayHealthAuthUnavailableError(error: unknown): boolean { + return isGatewayCredentialsRequiredError(error) || isGatewaySecretRefUnavailableError(error); +} + function noteCliGatewayVersionSkew(status: StatusSummary | undefined): void { const gatewayVersion = status?.runtimeVersion?.trim(); if (!gatewayVersion || gatewayVersion === VERSION) { @@ -102,7 +107,7 @@ export async function checkGatewayHealth(params: { } return { healthOk, authenticated: true, status }; } catch (err) { - if (isGatewayCredentialsRequiredError(err)) { + if (isGatewayHealthAuthUnavailableError(err)) { const probeDetails = await buildGatewayProbeConnectionDetails({ config: params.cfg }); const probe = await probeGatewayStatus({ url: probeDetails.url, diff --git a/src/commands/health.test.ts b/src/commands/health.test.ts index 1b7842ff3ae2..32263fdd64f7 100644 --- a/src/commands/health.test.ts +++ b/src/commands/health.test.ts @@ -64,6 +64,7 @@ const createHealthSummary = (params: { const callGatewayMock = vi.fn(); const isGatewayCredentialsRequiredErrorMock = vi.fn((_value: unknown) => false); +const isGatewaySecretRefUnavailableErrorMock = vi.fn((_value: unknown) => false); const TEST_GATEWAY_URL = "ws://127.0.0.1:18789"; const TEST_GATEWAY_MESSAGE = `Gateway mode: local\nGateway target: ${TEST_GATEWAY_URL}`; const TEST_AUTH_CLOSE_ERROR = "gateway closed (1008):"; @@ -92,6 +93,11 @@ vi.mock("../gateway/call.js", () => ({ isGatewayCredentialsRequiredErrorMock(value), })); +vi.mock("../gateway/credentials.js", () => ({ + isGatewaySecretRefUnavailableError: (value: unknown) => + isGatewaySecretRefUnavailableErrorMock(value), +})); + vi.mock("../cli/daemon-cli/probe.js", () => ({ probeGatewayStatus: (...args: unknown[]) => probeGatewayStatusMock(...args), })); @@ -139,6 +145,7 @@ describe("healthCommand", () => { }); formatGatewayTransportErrorJsonMock.mockReturnValue(null); isGatewayCredentialsRequiredErrorMock.mockReturnValue(false); + isGatewaySecretRefUnavailableErrorMock.mockReturnValue(false); probeGatewayStatusMock.mockReset(); }); @@ -321,6 +328,37 @@ describe("healthCommand", () => { }, ); + it("reports reachable gateway diagnostics when configured auth SecretRefs are unavailable", async () => { + const error = new Error("gateway.auth.password is unavailable"); + callGatewayMock.mockRejectedValueOnce(error); + isGatewaySecretRefUnavailableErrorMock.mockReturnValueOnce(true); + probeGatewayStatusMock.mockResolvedValueOnce({ + ok: false, + kind: "connect", + error: TEST_AUTH_CLOSE_ERROR, + }); + + await healthCommand({ json: false, timeoutMs: 5000, config: {} }, runtime as never); + + expect(isGatewaySecretRefUnavailableErrorMock).toHaveBeenCalledWith(error); + expect(probeGatewayStatusMock).toHaveBeenCalledWith({ + url: TEST_GATEWAY_URL, + token: undefined, + password: undefined, + tlsFingerprint: TEST_TLS_FINGERPRINT, + preauthHandshakeTimeoutMs: 4321, + timeoutMs: 5000, + config: {}, + json: false, + }); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(runtime.log.mock.calls).toEqual([ + [GATEWAY_HEALTH_REACHABLE_LINE], + [GATEWAY_HEALTH_CREDENTIALS_REQUIRED_MESSAGE], + ]); + expect(runtime.error).not.toHaveBeenCalled(); + }); + it("formats degraded model-pricing health as a warning", () => { const snapshot = createHealthSummary({ channels: {}, diff --git a/src/commands/health.ts b/src/commands/health.ts index ab20d57aba1d..8108b4e31e37 100644 --- a/src/commands/health.ts +++ b/src/commands/health.ts @@ -31,6 +31,7 @@ import { DEFAULT_CHANNEL_STALE_EVENT_THRESHOLD_MS, evaluateChannelHealth, } from "../gateway/channel-health-policy.js"; +import { isGatewaySecretRefUnavailableError } from "../gateway/credentials.js"; import { getGatewayModelPricingHealth } from "../gateway/model-pricing-cache-state.js"; import { isGatewayModelPricingEnabled } from "../gateway/model-pricing-config.js"; import type { ChannelRuntimeSnapshot } from "../gateway/server-channel-runtime.types.js"; @@ -74,6 +75,52 @@ const debugHealth = (...args: unknown[]) => { } }; +function isGatewayHealthAuthUnavailableError(error: unknown): boolean { + return isGatewayCredentialsRequiredError(error) || isGatewaySecretRefUnavailableError(error); +} + +export async function emitReachableGatewayAuthDiagnostic(params: { + error: unknown; + config: OpenClawConfig; + runtime: RuntimeEnv; + timeoutMs?: number; + token?: string; + password?: string; + json?: boolean; +}): Promise { + if (!isGatewayHealthAuthUnavailableError(params.error)) { + return false; + } + const details = await buildGatewayProbeConnectionDetails({ + config: params.config, + token: params.token, + password: params.password, + }); + const probe = await probeGatewayStatus({ + url: details.url, + token: params.token, + password: params.password, + tlsFingerprint: details.tlsFingerprint, + preauthHandshakeTimeoutMs: details.preauthHandshakeTimeoutMs, + timeoutMs: params.timeoutMs ?? DEFAULT_TIMEOUT_MS, + config: params.config, + json: params.json, + }); + if (!gatewayProbeResultSawGateway(probe)) { + return false; + } + const diagnostic = buildCredentialsRequiredHealthDiagnostic(); + if (params.json) { + writeRuntimeJson(params.runtime, diagnostic); + params.runtime.exit(1); + return true; + } + params.runtime.log(GATEWAY_HEALTH_REACHABLE_LINE); + params.runtime.log(diagnostic.error.message); + params.runtime.exit(1); + return true; +} + const loadConfigRuntime = async () => await import("../config/config.js"); const PUBLIC_IMESSAGE_FULL_DISK_ACCESS_ERROR = @@ -664,34 +711,20 @@ export async function healthCommand( }), ); } catch (error) { - if (isGatewayCredentialsRequiredError(error)) { - const details = await buildGatewayProbeConnectionDetails({ + if ( + await emitReachableGatewayAuthDiagnostic({ + error, config: cfg, + runtime, + timeoutMs: opts.timeoutMs, token: opts.token, password: opts.password, - }); - const probe = await probeGatewayStatus({ - url: details.url, - token: opts.token, - password: opts.password, - tlsFingerprint: details.tlsFingerprint, - preauthHandshakeTimeoutMs: details.preauthHandshakeTimeoutMs, - timeoutMs: opts.timeoutMs ?? DEFAULT_TIMEOUT_MS, - config: cfg, json: opts.json, - }); - if (gatewayProbeResultSawGateway(probe)) { - const diagnostic = buildCredentialsRequiredHealthDiagnostic(); - if (opts.json) { - writeRuntimeJson(runtime, diagnostic); - runtime.exit(1); - return; - } - runtime.log(GATEWAY_HEALTH_REACHABLE_LINE); - runtime.log(diagnostic.error.message); - runtime.exit(1); - return; - } + }) + ) { + return; + } + if (isGatewayHealthAuthUnavailableError(error)) { throw error; } if (opts.json) {