From f4fef64fc141e927e0547ff3805a57aceb493ed0 Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Fri, 13 Mar 2026 23:13:33 -0500 Subject: [PATCH] Gateway: treat scope-limited probe RPC as degraded reachability (#45622) * Gateway: treat scope-limited probe RPC as degraded * Docs: clarify gateway probe degraded scope output * test: fix CI type regressions in gateway and outbound suites * Tests: fix Node24 diffs theme loading and Windows assertions * Tests: fix extension typing after main rebase * Tests: fix Windows CI regressions after rebase * Tests: normalize executable path assertions on Windows * Tests: remove duplicate gateway daemon result alias * Tests: stabilize Windows approval path assertions * Tests: fix Discord rate-limit startup fixture typing * Tests: use Windows-friendly relative exec fixtures --------- Co-authored-by: Mainframe --- docs/cli/gateway.md | 17 ++++ docs/help/troubleshooting.md | 2 +- extensions/diffs/index.test.ts | 77 +++++++++--------- extensions/diffs/src/render.ts | 50 +++++++++++- src/commands/gateway-status.test.ts | 84 +++++++++++++++++++- src/commands/gateway-status.ts | 19 ++++- src/commands/gateway-status/helpers.test.ts | 46 ++++++++++- src/commands/gateway-status/helpers.ts | 18 ++++- src/daemon/schtasks.startup-fallback.test.ts | 12 ++- src/daemon/schtasks.stop.test.ts | 22 +++-- src/discord/monitor/provider.test.ts | 2 +- src/infra/exec-allowlist-pattern.test.ts | 7 +- src/infra/exec-approvals-store.test.ts | 2 +- src/infra/exec-command-resolution.test.ts | 10 ++- src/infra/executable-path.test.ts | 8 +- src/infra/executable-path.ts | 45 ++++++++--- src/infra/hardlink-guards.test.ts | 5 +- src/infra/home-dir.test.ts | 2 +- src/infra/json-file.test.ts | 8 +- src/infra/run-node.test.ts | 4 +- src/infra/stable-node-path.ts | 10 ++- src/infra/update-global.test.ts | 2 +- src/node-host/invoke-system-run-plan.test.ts | 5 ++ src/node-host/invoke-system-run.test.ts | 15 ++++ src/shared/config-eval.test.ts | 15 ++-- 25 files changed, 394 insertions(+), 93 deletions(-) diff --git a/docs/cli/gateway.md b/docs/cli/gateway.md index 95c20e3aa7c..96367774948 100644 --- a/docs/cli/gateway.md +++ b/docs/cli/gateway.md @@ -126,6 +126,23 @@ openclaw gateway probe openclaw gateway probe --json ``` +Interpretation: + +- `Reachable: yes` means at least one target accepted a WebSocket connect. +- `RPC: ok` means detail RPC calls (`health`/`status`/`system-presence`/`config.get`) also succeeded. +- `RPC: limited - missing scope: operator.read` means connect succeeded but detail RPC is scope-limited. This is reported as **degraded** reachability, not full failure. +- Exit code is non-zero only when no probed target is reachable. + +JSON notes (`--json`): + +- Top level: + - `ok`: at least one target is reachable. + - `degraded`: at least one target had scope-limited detail RPC. +- Per target (`targets[].connect`): + - `ok`: reachability after connect + degraded classification. + - `rpcOk`: full detail RPC success. + - `scopeLimited`: detail RPC failed due to missing operator scope. + #### Remote over SSH (Mac app parity) The macOS app “Remote over SSH” mode uses a local port-forward so the remote gateway (which may be bound to loopback only) becomes reachable at `ws://127.0.0.1:`. diff --git a/docs/help/troubleshooting.md b/docs/help/troubleshooting.md index 951e1a480d7..a3988c4ea58 100644 --- a/docs/help/troubleshooting.md +++ b/docs/help/troubleshooting.md @@ -28,7 +28,7 @@ Good output in one line: - `openclaw status` → shows configured channels and no obvious auth errors. - `openclaw status --all` → full report is present and shareable. -- `openclaw gateway probe` → expected gateway target is reachable. +- `openclaw gateway probe` → expected gateway target is reachable (`Reachable: yes`). `RPC: limited - missing scope: operator.read` is degraded diagnostics, not a connect failure. - `openclaw gateway status` → `Runtime: running` and `RPC probe: ok`. - `openclaw doctor` → no blocking config/service errors. - `openclaw channels status --probe` → channels report `connected` or `ready`. diff --git a/extensions/diffs/index.test.ts b/extensions/diffs/index.test.ts index fe7533683ec..c38da12bfcd 100644 --- a/extensions/diffs/index.test.ts +++ b/extensions/diffs/index.test.ts @@ -1,4 +1,5 @@ import type { IncomingMessage } from "node:http"; +import type { OpenClawPluginApi } from "openclaw/plugin-sdk/diffs"; import { describe, expect, it, vi } from "vitest"; import { createMockServerResponse } from "../../src/test-utils/mock-http-response.js"; import { createTestPluginApi } from "../test-utils/plugin-api.js"; @@ -42,48 +43,46 @@ describe("diffs plugin registration", () => { }); it("applies plugin-config defaults through registered tool and viewer handler", async () => { - let registeredTool: - | { execute?: (toolCallId: string, params: Record) => Promise } - | undefined; - let registeredHttpRouteHandler: - | (( - req: IncomingMessage, - res: ReturnType, - ) => Promise) - | undefined; + type RegisteredTool = { + execute?: (toolCallId: string, params: Record) => Promise; + }; + type RegisteredHttpRouteParams = Parameters[0]; - plugin.register?.( - createTestPluginApi({ - id: "diffs", - name: "Diffs", - description: "Diffs", - source: "test", - config: { - gateway: { - port: 18789, - bind: "loopback", - }, + let registeredTool: RegisteredTool | undefined; + let registeredHttpRouteHandler: RegisteredHttpRouteParams["handler"] | undefined; + + const api = createTestPluginApi({ + id: "diffs", + name: "Diffs", + description: "Diffs", + source: "test", + config: { + gateway: { + port: 18789, + bind: "loopback", }, - pluginConfig: { - defaults: { - mode: "view", - theme: "light", - background: false, - layout: "split", - showLineNumbers: false, - diffIndicators: "classic", - lineSpacing: 2, - }, + }, + pluginConfig: { + defaults: { + mode: "view", + theme: "light", + background: false, + layout: "split", + showLineNumbers: false, + diffIndicators: "classic", + lineSpacing: 2, }, - runtime: {} as never, - registerTool(tool) { - registeredTool = typeof tool === "function" ? undefined : tool; - }, - registerHttpRoute(params) { - registeredHttpRouteHandler = params.handler as typeof registeredHttpRouteHandler; - }, - }), - ); + }, + runtime: {} as never, + registerTool(tool: Parameters[0]) { + registeredTool = typeof tool === "function" ? undefined : tool; + }, + registerHttpRoute(params: RegisteredHttpRouteParams) { + registeredHttpRouteHandler = params.handler; + }, + }); + + plugin.register?.(api as unknown as OpenClawPluginApi); const result = await registeredTool?.execute?.("tool-1", { before: "one\n", diff --git a/extensions/diffs/src/render.ts b/extensions/diffs/src/render.ts index fb3d089c90a..ce01091eea6 100644 --- a/extensions/diffs/src/render.ts +++ b/extensions/diffs/src/render.ts @@ -1,5 +1,12 @@ -import type { FileContents, FileDiffMetadata, SupportedLanguages } from "@pierre/diffs"; -import { parsePatchFiles } from "@pierre/diffs"; +import fs from "node:fs/promises"; +import { createRequire } from "node:module"; +import type { + FileContents, + FileDiffMetadata, + SupportedLanguages, + ThemeRegistrationResolved, +} from "@pierre/diffs"; +import { RegisteredCustomThemes, parsePatchFiles } from "@pierre/diffs"; import { preloadFileDiff, preloadMultiFileDiff } from "@pierre/diffs/ssr"; import type { DiffInput, @@ -13,6 +20,45 @@ import { VIEWER_LOADER_PATH } from "./viewer-assets.js"; const DEFAULT_FILE_NAME = "diff.txt"; const MAX_PATCH_FILE_COUNT = 128; const MAX_PATCH_TOTAL_LINES = 120_000; +const diffsRequire = createRequire(import.meta.resolve("@pierre/diffs")); + +let pierreThemesPatched = false; + +function createThemeLoader( + themeName: "pierre-dark" | "pierre-light", + themePath: string, +): () => Promise { + let cachedTheme: ThemeRegistrationResolved | undefined; + return async () => { + if (cachedTheme) { + return cachedTheme; + } + const raw = await fs.readFile(themePath, "utf8"); + const parsed = JSON.parse(raw) as Record; + cachedTheme = { + ...parsed, + name: themeName, + } as ThemeRegistrationResolved; + return cachedTheme; + }; +} + +function patchPierreThemeLoadersForNode24(): void { + if (pierreThemesPatched) { + return; + } + try { + const darkThemePath = diffsRequire.resolve("@pierre/theme/themes/pierre-dark.json"); + const lightThemePath = diffsRequire.resolve("@pierre/theme/themes/pierre-light.json"); + RegisteredCustomThemes.set("pierre-dark", createThemeLoader("pierre-dark", darkThemePath)); + RegisteredCustomThemes.set("pierre-light", createThemeLoader("pierre-light", lightThemePath)); + pierreThemesPatched = true; + } catch { + // Keep upstream loaders if theme files cannot be resolved. + } +} + +patchPierreThemeLoadersForNode24(); function escapeCssString(value: string): string { return value.replaceAll("\\", "\\\\").replaceAll('"', '\\"'); diff --git a/src/commands/gateway-status.test.ts b/src/commands/gateway-status.test.ts index 64d515c0b4d..452bcb3691b 100644 --- a/src/commands/gateway-status.test.ts +++ b/src/commands/gateway-status.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import type { GatewayProbeResult } from "../gateway/probe.js"; import type { RuntimeEnv } from "../runtime.js"; import { withEnvAsync } from "../test-utils/env.js"; @@ -33,7 +34,7 @@ const startSshPortForward = vi.fn(async (_opts?: unknown) => ({ stderr: [], stop: sshStop, })); -const probeGateway = vi.fn(async (opts: { url: string }) => { +const probeGateway = vi.fn(async (opts: { url: string }): Promise => { const { url } = opts; if (url.includes("127.0.0.1")) { return { @@ -52,7 +53,16 @@ const probeGateway = vi.fn(async (opts: { url: string }) => { }, sessions: { count: 0 }, }, - presence: [{ mode: "gateway", reason: "self", host: "local", ip: "127.0.0.1" }], + presence: [ + { + mode: "gateway", + reason: "self", + host: "local", + ip: "127.0.0.1", + text: "Gateway: local (127.0.0.1) · app test · mode gateway · reason self", + ts: Date.now(), + }, + ], configSnapshot: { path: "/tmp/cfg.json", exists: true, @@ -81,7 +91,16 @@ const probeGateway = vi.fn(async (opts: { url: string }) => { }, sessions: { count: 2 }, }, - presence: [{ mode: "gateway", reason: "self", host: "remote", ip: "100.64.0.2" }], + presence: [ + { + mode: "gateway", + reason: "self", + host: "remote", + ip: "100.64.0.2", + text: "Gateway: remote (100.64.0.2) · app test · mode gateway · reason self", + ts: Date.now(), + }, + ], configSnapshot: { path: "/tmp/remote.json", exists: true, @@ -201,6 +220,54 @@ describe("gateway-status command", () => { expect(targets[0]?.summary).toBeTruthy(); }); + it("treats missing-scope RPC probe failures as degraded but reachable", async () => { + const { runtime, runtimeLogs, runtimeErrors } = createRuntimeCapture(); + readBestEffortConfig.mockResolvedValueOnce({ + gateway: { + mode: "local", + auth: { mode: "token", token: "ltok" }, + }, + } as never); + probeGateway.mockResolvedValueOnce({ + ok: false, + url: "ws://127.0.0.1:18789", + connectLatencyMs: 51, + error: "missing scope: operator.read", + close: null, + health: null, + status: null, + presence: null, + configSnapshot: null, + }); + + await runGatewayStatus(runtime, { timeout: "1000", json: true }); + + expect(runtimeErrors).toHaveLength(0); + const parsed = JSON.parse(runtimeLogs.join("\n")) as { + ok?: boolean; + degraded?: boolean; + warnings?: Array<{ code?: string; targetIds?: string[] }>; + targets?: Array<{ + connect?: { + ok?: boolean; + rpcOk?: boolean; + scopeLimited?: boolean; + }; + }>; + }; + expect(parsed.ok).toBe(true); + expect(parsed.degraded).toBe(true); + expect(parsed.targets?.[0]?.connect).toMatchObject({ + ok: true, + rpcOk: false, + scopeLimited: true, + }); + const scopeLimitedWarning = parsed.warnings?.find( + (warning) => warning.code === "probe_scope_limited", + ); + expect(scopeLimitedWarning?.targetIds).toContain("localLoopback"); + }); + it("surfaces unresolved SecretRef auth diagnostics in warnings", async () => { const { runtime, runtimeLogs, runtimeErrors } = createRuntimeCapture(); await withEnvAsync({ MISSING_GATEWAY_TOKEN: undefined }, async () => { @@ -361,7 +428,16 @@ describe("gateway-status command", () => { }, sessions: { count: 1 }, }, - presence: [{ mode: "gateway", reason: "self", host: "remote", ip: "100.64.0.2" }], + presence: [ + { + mode: "gateway", + reason: "self", + host: "remote", + ip: "100.64.0.2", + text: "Gateway: remote (100.64.0.2) · app test · mode gateway · reason self", + ts: Date.now(), + }, + ], configSnapshot: { path: "/tmp/secretref-config.json", exists: true, diff --git a/src/commands/gateway-status.ts b/src/commands/gateway-status.ts index 4ac54eca0c4..be0b9abf69a 100644 --- a/src/commands/gateway-status.ts +++ b/src/commands/gateway-status.ts @@ -10,6 +10,8 @@ import { colorize, isRich, theme } from "../terminal/theme.js"; import { buildNetworkHints, extractConfigSummary, + isProbeReachable, + isScopeLimitedProbeFailure, type GatewayStatusTarget, parseTimeoutMs, pickGatewaySelfPresence, @@ -193,8 +195,10 @@ export async function gatewayStatusCommand( }, ); - const reachable = probed.filter((p) => p.probe.ok); + const reachable = probed.filter((p) => isProbeReachable(p.probe)); const ok = reachable.length > 0; + const degradedScopeLimited = probed.filter((p) => isScopeLimitedProbeFailure(p.probe)); + const degraded = degradedScopeLimited.length > 0; const multipleGateways = reachable.length > 1; const primary = reachable.find((p) => p.target.kind === "explicit") ?? @@ -236,12 +240,21 @@ export async function gatewayStatusCommand( }); } } + for (const result of degradedScopeLimited) { + warnings.push({ + code: "probe_scope_limited", + message: + "Probe diagnostics are limited by gateway scopes (missing operator.read). Connection succeeded, but status details may be incomplete. Hint: pair device identity or use credentials with operator.read.", + targetIds: [result.target.id], + }); + } if (opts.json) { runtime.log( JSON.stringify( { ok, + degraded, ts: Date.now(), durationMs: Date.now() - startedAt, timeoutMs: overallTimeoutMs, @@ -274,7 +287,9 @@ export async function gatewayStatusCommand( active: p.target.active, tunnel: p.target.tunnel ?? null, connect: { - ok: p.probe.ok, + ok: isProbeReachable(p.probe), + rpcOk: p.probe.ok, + scopeLimited: isScopeLimitedProbeFailure(p.probe), latencyMs: p.probe.connectLatencyMs, error: p.probe.error, close: p.probe.close, diff --git a/src/commands/gateway-status/helpers.test.ts b/src/commands/gateway-status/helpers.test.ts index 688959f0748..e0c1ecee763 100644 --- a/src/commands/gateway-status/helpers.test.ts +++ b/src/commands/gateway-status/helpers.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from "vitest"; import { withEnvAsync } from "../../test-utils/env.js"; -import { extractConfigSummary, resolveAuthForTarget } from "./helpers.js"; +import { + extractConfigSummary, + isProbeReachable, + isScopeLimitedProbeFailure, + renderProbeSummaryLine, + resolveAuthForTarget, +} from "./helpers.js"; describe("extractConfigSummary", () => { it("marks SecretRef-backed gateway auth credentials as configured", () => { @@ -229,3 +235,41 @@ describe("resolveAuthForTarget", () => { ); }); }); + +describe("probe reachability classification", () => { + it("treats missing-scope RPC failures as scope-limited and reachable", () => { + const probe = { + ok: false, + url: "ws://127.0.0.1:18789", + connectLatencyMs: 51, + error: "missing scope: operator.read", + close: null, + health: null, + status: null, + presence: null, + configSnapshot: null, + }; + + expect(isScopeLimitedProbeFailure(probe)).toBe(true); + expect(isProbeReachable(probe)).toBe(true); + expect(renderProbeSummaryLine(probe, false)).toContain("RPC: limited"); + }); + + it("keeps non-scope RPC failures as unreachable", () => { + const probe = { + ok: false, + url: "ws://127.0.0.1:18789", + connectLatencyMs: 43, + error: "unknown method: status", + close: null, + health: null, + status: null, + presence: null, + configSnapshot: null, + }; + + expect(isScopeLimitedProbeFailure(probe)).toBe(false); + expect(isProbeReachable(probe)).toBe(false); + expect(renderProbeSummaryLine(probe, false)).toContain("RPC: failed"); + }); +}); diff --git a/src/commands/gateway-status/helpers.ts b/src/commands/gateway-status/helpers.ts index 7697d6af143..5f1a5e2f5ee 100644 --- a/src/commands/gateway-status/helpers.ts +++ b/src/commands/gateway-status/helpers.ts @@ -9,6 +9,8 @@ import { pickPrimaryTailnetIPv4 } from "../../infra/tailnet.js"; import { colorize, theme } from "../../terminal/theme.js"; import { pickGatewaySelfPresence } from "../gateway-presence.js"; +const MISSING_SCOPE_PATTERN = /\bmissing scope:\s*[a-z0-9._-]+/i; + type TargetKind = "explicit" | "configRemote" | "localLoopback" | "sshTunnel"; export type GatewayStatusTarget = { @@ -324,6 +326,17 @@ export function renderTargetHeader(target: GatewayStatusTarget, rich: boolean) { return `${colorize(rich, theme.heading, kindLabel)} ${colorize(rich, theme.muted, target.url)}`; } +export function isScopeLimitedProbeFailure(probe: GatewayProbeResult): boolean { + if (probe.ok || probe.connectLatencyMs == null) { + return false; + } + return MISSING_SCOPE_PATTERN.test(probe.error ?? ""); +} + +export function isProbeReachable(probe: GatewayProbeResult): boolean { + return probe.ok || isScopeLimitedProbeFailure(probe); +} + export function renderProbeSummaryLine(probe: GatewayProbeResult, rich: boolean) { if (probe.ok) { const latency = @@ -335,7 +348,10 @@ export function renderProbeSummaryLine(probe: GatewayProbeResult, rich: boolean) if (probe.connectLatencyMs != null) { const latency = typeof probe.connectLatencyMs === "number" ? `${probe.connectLatencyMs}ms` : "unknown"; - return `${colorize(rich, theme.success, "Connect: ok")} (${latency}) · ${colorize(rich, theme.error, "RPC: failed")}${detail}`; + const rpcStatus = isScopeLimitedProbeFailure(probe) + ? colorize(rich, theme.warn, "RPC: limited") + : colorize(rich, theme.error, "RPC: failed"); + return `${colorize(rich, theme.success, "Connect: ok")} (${latency}) · ${rpcStatus}${detail}`; } return `${colorize(rich, theme.error, "Connect: failed")}${detail}`; diff --git a/src/daemon/schtasks.startup-fallback.test.ts b/src/daemon/schtasks.startup-fallback.test.ts index 4d8d6325366..55e678052f3 100644 --- a/src/daemon/schtasks.startup-fallback.test.ts +++ b/src/daemon/schtasks.startup-fallback.test.ts @@ -59,6 +59,14 @@ function expectStartupFallbackSpawn(env: Record) { ); } +function expectGatewayTermination(pid: number) { + if (process.platform === "win32") { + expect(killProcessTree).not.toHaveBeenCalled(); + return; + } + expect(killProcessTree).toHaveBeenCalledWith(pid, { graceMs: 300 }); +} + function addStartupFallbackMissingResponses( extraResponses: Array<{ code: number; stdout: string; stderr: string }> = [], ) { @@ -179,7 +187,7 @@ describe("Windows startup fallback", () => { await expect(restartScheduledTask({ env, stdout })).resolves.toEqual({ outcome: "completed", }); - expect(killProcessTree).toHaveBeenCalledWith(5151, { graceMs: 300 }); + expectGatewayTermination(5151); expectStartupFallbackSpawn(env); }); }); @@ -214,7 +222,7 @@ describe("Windows startup fallback", () => { delete envWithoutPort.OPENCLAW_GATEWAY_PORT; await stopScheduledTask({ env: envWithoutPort, stdout }); - expect(killProcessTree).toHaveBeenCalledWith(5151, { graceMs: 300 }); + expectGatewayTermination(5151); }); }); }); diff --git a/src/daemon/schtasks.stop.test.ts b/src/daemon/schtasks.stop.test.ts index 2844196e5ad..04e5f1fced1 100644 --- a/src/daemon/schtasks.stop.test.ts +++ b/src/daemon/schtasks.stop.test.ts @@ -59,6 +59,14 @@ function busyPortUsage( }; } +function expectGatewayTermination(pid: number) { + if (process.platform === "win32") { + expect(killProcessTree).not.toHaveBeenCalled(); + return; + } + expect(killProcessTree).toHaveBeenCalledWith(pid, { graceMs: 300 }); +} + async function withPreparedGatewayTask( run: (context: { env: Record; stdout: PassThrough }) => Promise, ) { @@ -92,7 +100,7 @@ describe("Scheduled Task stop/restart cleanup", () => { await stopScheduledTask({ env, stdout }); expect(findVerifiedGatewayListenerPidsOnPortSync).toHaveBeenCalledWith(GATEWAY_PORT); - expect(killProcessTree).toHaveBeenCalledWith(4242, { graceMs: 300 }); + expectGatewayTermination(4242); expect(inspectPortUsage).toHaveBeenCalledTimes(2); }); }); @@ -111,8 +119,12 @@ describe("Scheduled Task stop/restart cleanup", () => { await stopScheduledTask({ env, stdout }); - expect(killProcessTree).toHaveBeenNthCalledWith(1, 4242, { graceMs: 300 }); - expect(killProcessTree).toHaveBeenNthCalledWith(2, expect.any(Number), { graceMs: 300 }); + if (process.platform !== "win32") { + expect(killProcessTree).toHaveBeenNthCalledWith(1, 4242, { graceMs: 300 }); + expect(killProcessTree).toHaveBeenNthCalledWith(2, expect.any(Number), { graceMs: 300 }); + } else { + expect(killProcessTree).not.toHaveBeenCalled(); + } expect(inspectPortUsage.mock.calls.length).toBeGreaterThanOrEqual(22); }); }); @@ -132,7 +144,7 @@ describe("Scheduled Task stop/restart cleanup", () => { await stopScheduledTask({ env, stdout }); - expect(killProcessTree).toHaveBeenCalledWith(6262, { graceMs: 300 }); + expectGatewayTermination(6262); expect(inspectPortUsage).toHaveBeenCalledTimes(2); }); }); @@ -150,7 +162,7 @@ describe("Scheduled Task stop/restart cleanup", () => { }); expect(findVerifiedGatewayListenerPidsOnPortSync).toHaveBeenCalledWith(GATEWAY_PORT); - expect(killProcessTree).toHaveBeenCalledWith(5151, { graceMs: 300 }); + expectGatewayTermination(5151); expect(inspectPortUsage).toHaveBeenCalledTimes(2); expect(schtasksCalls.at(-1)).toEqual(["/Run", "/TN", "OpenClaw Gateway"]); }); diff --git a/src/discord/monitor/provider.test.ts b/src/discord/monitor/provider.test.ts index d10862b380b..13869e4904f 100644 --- a/src/discord/monitor/provider.test.ts +++ b/src/discord/monitor/provider.test.ts @@ -806,7 +806,7 @@ describe("monitorDiscordProvider", () => { expect(clientFetchUserMock).toHaveBeenCalledWith("@me"); expect(monitorLifecycleMock).toHaveBeenCalledTimes(1); expect(runtime.log).toHaveBeenCalledWith( - expect.stringContaining("daily application command create limit reached"), + expect.stringContaining("native command deploy skipped"), ); }); diff --git a/src/infra/exec-allowlist-pattern.test.ts b/src/infra/exec-allowlist-pattern.test.ts index f7834a4c9fc..50e241f912d 100644 --- a/src/infra/exec-allowlist-pattern.test.ts +++ b/src/infra/exec-allowlist-pattern.test.ts @@ -1,3 +1,4 @@ +import path from "node:path"; import { describe, expect, it } from "vitest"; import { matchesExecAllowlistPattern } from "./exec-allowlist-pattern.js"; @@ -28,9 +29,11 @@ describe("matchesExecAllowlistPattern", () => { const prevHome = process.env.HOME; process.env.OPENCLAW_HOME = "/srv/openclaw-home"; process.env.HOME = "/home/other"; + const openClawHome = path.join(path.resolve("/srv/openclaw-home"), "bin", "tool"); + const fallbackHome = path.join(path.resolve("/home/other"), "bin", "tool"); try { - expect(matchesExecAllowlistPattern("~/bin/tool", "/srv/openclaw-home/bin/tool")).toBe(true); - expect(matchesExecAllowlistPattern("~/bin/tool", "/home/other/bin/tool")).toBe(false); + expect(matchesExecAllowlistPattern("~/bin/tool", openClawHome)).toBe(true); + expect(matchesExecAllowlistPattern("~/bin/tool", fallbackHome)).toBe(false); } finally { if (prevOpenClawHome === undefined) { delete process.env.OPENCLAW_HOME; diff --git a/src/infra/exec-approvals-store.test.ts b/src/infra/exec-approvals-store.test.ts index d30b3263129..4dc6ab71c7e 100644 --- a/src/infra/exec-approvals-store.test.ts +++ b/src/infra/exec-approvals-store.test.ts @@ -112,7 +112,7 @@ describe("exec approvals store helpers", () => { expect(missing.exists).toBe(false); expect(missing.raw).toBeNull(); expect(missing.file).toEqual(normalizeExecApprovals({ version: 1, agents: {} })); - expect(missing.path).toBe(approvalsFilePath(dir)); + expect(path.normalize(missing.path)).toBe(path.normalize(approvalsFilePath(dir))); fs.mkdirSync(path.dirname(approvalsFilePath(dir)), { recursive: true }); fs.writeFileSync(approvalsFilePath(dir), "{invalid", "utf8"); diff --git a/src/infra/exec-command-resolution.test.ts b/src/infra/exec-command-resolution.test.ts index 4621383a547..4bdff0947a9 100644 --- a/src/infra/exec-command-resolution.test.ts +++ b/src/infra/exec-command-resolution.test.ts @@ -80,12 +80,13 @@ describe("exec-command-resolution", () => { setup: () => { const dir = makeTempDir(); const cwd = path.join(dir, "project"); - const script = path.join(cwd, "scripts", "run.sh"); + const scriptName = process.platform === "win32" ? "run.cmd" : "run.sh"; + const script = path.join(cwd, "scripts", scriptName); fs.mkdirSync(path.dirname(script), { recursive: true }); fs.writeFileSync(script, ""); fs.chmodSync(script, 0o755); return { - command: "./scripts/run.sh --flag", + command: `./scripts/${scriptName} --flag`, cwd, envPath: undefined as NodeJS.ProcessEnv | undefined, expectedPath: script, @@ -98,12 +99,13 @@ describe("exec-command-resolution", () => { setup: () => { const dir = makeTempDir(); const cwd = path.join(dir, "project"); - const script = path.join(cwd, "bin", "tool"); + const scriptName = process.platform === "win32" ? "tool.cmd" : "tool"; + const script = path.join(cwd, "bin", scriptName); fs.mkdirSync(path.dirname(script), { recursive: true }); fs.writeFileSync(script, ""); fs.chmodSync(script, 0o755); return { - command: '"./bin/tool" --version', + command: `"./bin/${scriptName}" --version`, cwd, envPath: undefined as NodeJS.ProcessEnv | undefined, expectedPath: script, diff --git a/src/infra/executable-path.test.ts b/src/infra/executable-path.test.ts index 31437cafe49..8c7412fb385 100644 --- a/src/infra/executable-path.test.ts +++ b/src/infra/executable-path.test.ts @@ -66,8 +66,12 @@ describe("executable path helpers", () => { await fs.chmod(pathTool, 0o755); expect(resolveExecutablePath(absoluteTool)).toBe(absoluteTool); - expect(resolveExecutablePath("~/home-tool", { env: { HOME: homeDir } })).toBe(homeTool); - expect(resolveExecutablePath("runner", { env: { Path: binDir } })).toBe(pathTool); + expect( + path.normalize(resolveExecutablePath("~/home-tool", { env: { HOME: homeDir } }) ?? ""), + ).toBe(path.normalize(homeTool)); + expect(path.normalize(resolveExecutablePath("runner", { env: { Path: binDir } }) ?? "")).toBe( + path.normalize(pathTool), + ); expect(resolveExecutablePath("~/missing-tool", { env: { HOME: homeDir } })).toBeUndefined(); }); }); diff --git a/src/infra/executable-path.ts b/src/infra/executable-path.ts index bf648c7cb6a..a1d596cccf8 100644 --- a/src/infra/executable-path.ts +++ b/src/infra/executable-path.ts @@ -12,15 +12,33 @@ function resolveWindowsExecutableExtensions( if (path.extname(executable).length > 0) { return [""]; } - return ( - env?.PATHEXT ?? - env?.Pathext ?? - process.env.PATHEXT ?? - process.env.Pathext ?? - ".EXE;.CMD;.BAT;.COM" - ) - .split(";") - .map((ext) => ext.toLowerCase()); + return [ + "", + ...( + env?.PATHEXT ?? + env?.Pathext ?? + process.env.PATHEXT ?? + process.env.Pathext ?? + ".EXE;.CMD;.BAT;.COM" + ) + .split(";") + .map((ext) => ext.toLowerCase()), + ]; +} + +function resolveWindowsExecutableExtSet(env: NodeJS.ProcessEnv | undefined): Set { + return new Set( + ( + env?.PATHEXT ?? + env?.Pathext ?? + process.env.PATHEXT ?? + process.env.Pathext ?? + ".EXE;.CMD;.BAT;.COM" + ) + .split(";") + .map((ext) => ext.toLowerCase()) + .filter(Boolean), + ); } export function isExecutableFile(filePath: string): boolean { @@ -29,9 +47,14 @@ export function isExecutableFile(filePath: string): boolean { if (!stat.isFile()) { return false; } - if (process.platform !== "win32") { - fs.accessSync(filePath, fs.constants.X_OK); + if (process.platform === "win32") { + const ext = path.extname(filePath).toLowerCase(); + if (!ext) { + return true; + } + return resolveWindowsExecutableExtSet(undefined).has(ext); } + fs.accessSync(filePath, fs.constants.X_OK); return true; } catch { return false; diff --git a/src/infra/hardlink-guards.test.ts b/src/infra/hardlink-guards.test.ts index e96d826c1d8..1a8f7205bcb 100644 --- a/src/infra/hardlink-guards.test.ts +++ b/src/infra/hardlink-guards.test.ts @@ -50,6 +50,7 @@ describe("assertNoHardlinkedFinalPath", () => { await fs.writeFile(source, "hello", "utf8"); await fs.link(source, linked); const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(root); + const expectedLinkedPath = path.join("~", "linked.txt"); try { await expect( @@ -58,7 +59,9 @@ describe("assertNoHardlinkedFinalPath", () => { root, boundaryLabel: "workspace", }), - ).rejects.toThrow("Hardlinked path is not allowed under workspace (~): ~/linked.txt"); + ).rejects.toThrow( + `Hardlinked path is not allowed under workspace (~): ${expectedLinkedPath}`, + ); } finally { homedirSpy.mockRestore(); } diff --git a/src/infra/home-dir.test.ts b/src/infra/home-dir.test.ts index e0f19f865b6..9faeda1dee5 100644 --- a/src/infra/home-dir.test.ts +++ b/src/infra/home-dir.test.ts @@ -109,7 +109,7 @@ describe("expandHomePrefix", () => { name: "expands exact ~ using explicit home", input: "~", opts: { home: " /srv/openclaw-home " }, - expected: path.resolve("/srv/openclaw-home"), + expected: "/srv/openclaw-home", }, { name: "expands ~\\\\ using resolved env home", diff --git a/src/infra/json-file.test.ts b/src/infra/json-file.test.ts index 60dd0e3a237..4b204fb21bc 100644 --- a/src/infra/json-file.test.ts +++ b/src/infra/json-file.test.ts @@ -35,8 +35,12 @@ describe("json-file helpers", () => { const fileMode = fs.statSync(pathname).mode & 0o777; const dirMode = fs.statSync(path.dirname(pathname)).mode & 0o777; - expect(fileMode).toBe(0o600); - expect(dirMode).toBe(0o700); + if (process.platform === "win32") { + expect(fileMode & 0o111).toBe(0); + } else { + expect(fileMode).toBe(0o600); + expect(dirMode).toBe(0o700); + } }); }); diff --git a/src/infra/run-node.test.ts b/src/infra/run-node.test.ts index 1007b2c6141..0b8cf1090bc 100644 --- a/src/infra/run-node.test.ts +++ b/src/infra/run-node.test.ts @@ -137,8 +137,8 @@ describe("run-node script", () => { it("returns the build exit code when the compiler step fails", async () => { await withTempDir(async (tmp) => { - const spawn = (cmd: string) => { - if (cmd === "pnpm") { + const spawn = (cmd: string, args: string[] = []) => { + if (cmd === "pnpm" || (cmd === "cmd.exe" && args.includes("pnpm"))) { return createExitedProcess(23); } return createExitedProcess(0); diff --git a/src/infra/stable-node-path.ts b/src/infra/stable-node-path.ts index 116b040eefa..9d4730f5cd7 100644 --- a/src/infra/stable-node-path.ts +++ b/src/infra/stable-node-path.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import path from "node:path"; /** * Homebrew Cellar paths (e.g. /opt/homebrew/Cellar/node/25.7.0/bin/node) @@ -8,15 +9,18 @@ import fs from "node:fs/promises"; * - Versioned formula "node@22": /opt/node@22/bin/node (keg-only) */ export async function resolveStableNodePath(nodePath: string): Promise { - const cellarMatch = nodePath.match(/^(.+?)\/Cellar\/([^/]+)\/[^/]+\/bin\/node$/); + const cellarMatch = nodePath.match( + /^(.+?)[\\/]Cellar[\\/]([^\\/]+)[\\/][^\\/]+[\\/]bin[\\/]node$/, + ); if (!cellarMatch) { return nodePath; } const prefix = cellarMatch[1]; // e.g. /opt/homebrew const formula = cellarMatch[2]; // e.g. "node" or "node@22" + const pathModule = nodePath.includes("\\") ? path.win32 : path.posix; // Try the Homebrew opt symlink first — works for both default and versioned formulas. - const optPath = `${prefix}/opt/${formula}/bin/node`; + const optPath = pathModule.join(prefix, "opt", formula, "bin", "node"); try { await fs.access(optPath); return optPath; @@ -26,7 +30,7 @@ export async function resolveStableNodePath(nodePath: string): Promise { // For the default "node" formula, also try the direct bin symlink. if (formula === "node") { - const binPath = `${prefix}/bin/node`; + const binPath = pathModule.join(prefix, "bin", "node"); try { await fs.access(binPath); return binPath; diff --git a/src/infra/update-global.test.ts b/src/infra/update-global.test.ts index b95727febbf..54cda49a407 100644 --- a/src/infra/update-global.test.ts +++ b/src/infra/update-global.test.ts @@ -56,7 +56,7 @@ describe("update global helpers", () => { path.join(".bun", "install", "global", "node_modules"), ); await expect(resolveGlobalPackageRoot("npm", runCommand, 1000)).resolves.toBe( - "/tmp/npm-root/openclaw", + path.join("/tmp/npm-root", "openclaw"), ); }); diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index 29cec3074aa..372e66f6521 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -41,6 +41,7 @@ type RuntimeFixture = { expectedArgvIndex: number; binName?: string; binNames?: string[]; + skipOnWin32?: boolean; }; type UnsafeRuntimeInvocationCase = { @@ -508,6 +509,7 @@ describe("hardenApprovedExecutionPaths", () => { scriptName: "run.ts", initialBody: 'console.log("SAFE");\n', expectedArgvIndex: 3, + skipOnWin32: true, }, { name: "pnpm exec double-dash tsx file", @@ -557,6 +559,9 @@ describe("hardenApprovedExecutionPaths", () => { for (const runtimeCase of mutableOperandCases) { it(`captures mutable ${runtimeCase.name} operands in approval plans`, () => { + if (runtimeCase.skipOnWin32 && process.platform === "win32") { + return; + } const binNames = runtimeCase.binNames ?? (runtimeCase.binName ? [runtimeCase.binName] : ["bunx", "pnpm", "npm", "npx", "tsx"]); diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index d183f9087c3..045897a5fc4 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -746,6 +746,14 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { security: "full", ask: "off", }); + if (process.platform === "win32") { + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval requires a stable executable path", + exact: true, + }); + return; + } expectCommandPinnedToCanonicalPath({ runCommand, expected: fs.realpathSync(script), @@ -779,6 +787,13 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { ask: "off", }); expect(runCommand).not.toHaveBeenCalled(); + if (process.platform === "win32") { + expectInvokeErrorMessage(sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval requires a stable executable path", + exact: true, + }); + return; + } expectInvokeErrorMessage(sendInvokeResult, { message: "SYSTEM_RUN_DENIED: approval cwd changed before execution", exact: true, diff --git a/src/shared/config-eval.test.ts b/src/shared/config-eval.test.ts index 7891c17142c..199c22a3462 100644 --- a/src/shared/config-eval.test.ts +++ b/src/shared/config-eval.test.ts @@ -86,6 +86,7 @@ describe("config-eval helpers", () => { }); it("caches binary lookups until PATH changes", () => { + setPlatform("linux"); process.env.PATH = ["/missing/bin", "/found/bin"].join(path.delimiter); const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation((candidate) => { if (String(candidate) === path.join("/found/bin", "tool")) { @@ -110,10 +111,14 @@ describe("config-eval helpers", () => { it("checks PATHEXT candidates on Windows", () => { setPlatform("win32"); - process.env.PATH = "/tools"; + const toolsDir = path.join(path.sep, "tools"); + process.env.PATH = toolsDir; process.env.PATHEXT = ".EXE;.CMD"; + const plainCandidate = path.join(toolsDir, "tool"); + const exeCandidate = path.join(toolsDir, "tool.EXE"); + const cmdCandidate = path.join(toolsDir, "tool.CMD"); const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation((candidate) => { - if (String(candidate) === "/tools/tool.CMD") { + if (String(candidate) === cmdCandidate) { return undefined; } throw new Error("missing"); @@ -121,9 +126,9 @@ describe("config-eval helpers", () => { expect(hasBinary("tool")).toBe(true); expect(accessSpy.mock.calls.map(([candidate]) => String(candidate))).toEqual([ - "/tools/tool", - "/tools/tool.EXE", - "/tools/tool.CMD", + plainCandidate, + exeCandidate, + cmdCandidate, ]); }); });