diff --git a/CHANGELOG.md b/CHANGELOG.md index b6f3357b6e1..9ea94ae6790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Validate node exec event provenance [AI]. (#81071) Thanks @pgondhi987. - Limit hook CLI tool authority [AI]. (#81065) Thanks @pgondhi987. - Require admin scope for node device token management [AI]. (#81067) Thanks @pgondhi987. - Restrict chat sender allowlist matching [AI]. (#80898) Thanks @pgondhi987. diff --git a/apps/macos/Sources/OpenClaw/NodeMode/MacNodeRuntime.swift b/apps/macos/Sources/OpenClaw/NodeMode/MacNodeRuntime.swift index 3e1eb5756d2..a955c7a1b86 100644 --- a/apps/macos/Sources/OpenClaw/NodeMode/MacNodeRuntime.swift +++ b/apps/macos/Sources/OpenClaw/NodeMode/MacNodeRuntime.swift @@ -521,7 +521,8 @@ actor MacNodeRuntime { let sessionKey = (params.sessionKey?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false) ? params.sessionKey!.trimmingCharacters(in: .whitespacesAndNewlines) : self.mainSessionKey - let runId = UUID().uuidString + let providedRunId = params.runId?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + let runId = providedRunId.isEmpty ? UUID().uuidString : providedRunId let envOverrideDiagnostics = HostEnvSanitizer.inspectOverrides( overrides: params.env, blockPathOverrides: true) diff --git a/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift b/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift index f24e288b2a7..a69139f210e 100644 --- a/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift @@ -14,6 +14,18 @@ struct MacNodeRuntimeTests { } } + actor ExecEventProbe { + private var captured: [(event: String, json: String)] = [] + + func append(event: String, json: String?) { + self.captured.append((event: event, json: json ?? "")) + } + + func events() -> [(event: String, json: String)] { + self.captured + } + } + @Test func `handle invoke rejects unknown command`() async { let runtime = MacNodeRuntime() let response = await runtime.handleInvoke( @@ -45,6 +57,40 @@ struct MacNodeRuntimeTests { #expect(response.ok == false) } + @Test func `system run denied event preserves gateway run id`() async throws { + let stateDir = FileManager().temporaryDirectory + .appendingPathComponent("openclaw-state-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager().removeItem(at: stateDir) } + + try await TestIsolation.withEnvValues(["OPENCLAW_STATE_DIR": stateDir.path]) { + let probe = ExecEventProbe() + let runtime = MacNodeRuntime() + await runtime.setEventSender { event, json in + await probe.append(event: event, json: json) + } + let params = OpenClawSystemRunParams( + command: ["/bin/sh", "-lc", "printf ok"], + sessionKey: "agent:main:main", + runId: "gateway-run-1") + let json = try String(data: JSONEncoder().encode(params), encoding: .utf8) + let response = await runtime.handleInvoke( + BridgeInvokeRequest( + id: "req-run-id", + command: OpenClawSystemCommand.run.rawValue, + paramsJSON: json)) + + #expect(response.ok == false) + let denied = try #require((await probe.events()).first { $0.event == "exec.denied" }) + struct Payload: Decodable { + var sessionKey: String + var runId: String + } + let payload = try JSONDecoder().decode(Payload.self, from: Data(denied.json.utf8)) + #expect(payload.sessionKey == "agent:main:main") + #expect(payload.runId == "gateway-run-1") + } + } + @Test func `handle invoke rejects blocked system run env override before execution`() async throws { let runtime = MacNodeRuntime() let params = OpenClawSystemRunParams( diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/SystemCommands.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/SystemCommands.swift index a2c8349058b..27bf598cefd 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/SystemCommands.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/SystemCommands.swift @@ -29,6 +29,7 @@ public struct OpenClawSystemRunParams: Codable, Sendable, Equatable { public var needsScreenRecording: Bool? public var agentId: String? public var sessionKey: String? + public var runId: String? public var approved: Bool? public var approvalDecision: String? @@ -41,6 +42,7 @@ public struct OpenClawSystemRunParams: Codable, Sendable, Equatable { needsScreenRecording: Bool? = nil, agentId: String? = nil, sessionKey: String? = nil, + runId: String? = nil, approved: Bool? = nil, approvalDecision: String? = nil) { @@ -52,6 +54,7 @@ public struct OpenClawSystemRunParams: Codable, Sendable, Equatable { self.needsScreenRecording = needsScreenRecording self.agentId = agentId self.sessionKey = sessionKey + self.runId = runId self.approved = approved self.approvalDecision = approvalDecision } diff --git a/src/agents/bash-tools.exec-host-node-phases.ts b/src/agents/bash-tools.exec-host-node-phases.ts index 3fb5b80aab7..31119993ce2 100644 --- a/src/agents/bash-tools.exec-host-node-phases.ts +++ b/src/agents/bash-tools.exec-host-node-phases.ts @@ -166,6 +166,7 @@ export function buildNodeSystemRunInvoke(params: { }): Record { const timeoutMs = params.target.runTimeoutSec > 0 ? Math.floor(params.target.runTimeoutSec * 1000) : 0; + const runId = params.runId ?? crypto.randomUUID(); return { nodeId: params.target.nodeId, command: "system.run", @@ -188,7 +189,7 @@ export function buildNodeSystemRunInvoke(params: { : {}), approved: params.approved, approvalDecision: params.approvalDecision ?? undefined, - runId: params.runId ?? undefined, + runId, suppressNotifyOnExit: params.suppressNotifyOnExit === true || params.notifyOnExit === false ? true : undefined, }, diff --git a/src/agents/bash-tools.exec-host-node.test.ts b/src/agents/bash-tools.exec-host-node.test.ts index 6e5b34ec7cf..a3aad68557e 100644 --- a/src/agents/bash-tools.exec-host-node.test.ts +++ b/src/agents/bash-tools.exec-host-node.test.ts @@ -422,6 +422,7 @@ describe("executeNodeHostCommand", () => { const runParams = requireRunParams(call); expect(runParams.command).toEqual(["/bin/sh", "-lc", "bun ./script.ts"]); expect(runParams.rawCommand).toBe("bun ./script.ts"); + expect(typeof runParams.runId).toBe("string"); expect(runParams.suppressNotifyOnExit).toBe(true); expect(runParams.timeoutMs).toBe(30_000); expect(Object.hasOwn(runParams, "systemRunPlan")).toBe(false); diff --git a/src/gateway/node-registry.test.ts b/src/gateway/node-registry.test.ts index 06e3cf56d0d..913ccde6dd1 100644 --- a/src/gateway/node-registry.test.ts +++ b/src/gateway/node-registry.test.ts @@ -1,8 +1,13 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { NodeRegistry, serializeEventPayload } from "./node-registry.js"; import type { GatewayWsClient } from "./server/ws-types.js"; -function makeClient(connId: string, nodeId: string, sent: string[] = []): GatewayWsClient { +function makeClient( + connId: string, + nodeId: string, + sent: string[] = [], + opts: { clientId?: string; platform?: string; version?: string } = {}, +): GatewayWsClient { return { connId, usesSharedGatewayAuth: false, @@ -14,7 +19,12 @@ function makeClient(connId: string, nodeId: string, sent: string[] = []): Gatewa }, } as unknown as GatewayWsClient["socket"], connect: { - client: { id: "openclaw-macos", version: "1.0.0", platform: "darwin", mode: "node" }, + client: { + id: opts.clientId ?? "openclaw-macos", + version: opts.version ?? "1.0.0", + platform: opts.platform ?? "darwin", + mode: "node", + }, device: { id: nodeId, publicKey: "public-key", @@ -55,6 +65,345 @@ describe("gateway/node-registry", () => { await expect(oldDisconnected).resolves.toBeInstanceOf(Error); }); + it("matches pending system.run events to the issuing connection", async () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register( + makeClient("conn-1", "node-1", frames, { + clientId: "openclaw-node-host", + platform: "linux", + }), + {}, + ); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-1", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + const request = JSON.parse(frames[0] ?? "{}") as { payload?: { id?: string } }; + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-1", + sessionKey: "agent:main:main", + terminal: false, + }), + ).toBe(true); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-other", + runId: "run-1", + sessionKey: "agent:main:main", + terminal: false, + }), + ).toBe(false); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-other", + sessionKey: "agent:main:main", + terminal: false, + }), + ).toBe(false); + + registry.handleInvokeResult({ + id: request.payload?.id ?? "", + nodeId: "node-1", + connId: "conn-1", + ok: true, + }); + await expect(invoke).resolves.toEqual({ + ok: true, + payload: undefined, + payloadJSON: null, + error: null, + }); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-1", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-1", + sessionKey: "agent:main:main", + terminal: false, + }), + ).toBe(false); + }); + + it("keeps no-timeout system.run event authorization after invoke timeout", async () => { + vi.useFakeTimers(); + const registry = new NodeRegistry(); + const frames: string[] = []; + try { + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-timeout", sessionKey: "agent:main:main", timeoutMs: 0 }, + timeoutMs: 1, + }); + + await vi.advanceTimersByTimeAsync(1); + await expect(invoke).resolves.toEqual({ + ok: false, + error: { code: "TIMEOUT", message: "node invoke timed out" }, + }); + + await vi.advanceTimersByTimeAsync(2 * 60 * 60 * 1000); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-timeout", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + it("matches a single system.run event when legacy payload omits runId", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-legacy", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("rejects runId-less system.run events for non-legacy nodes", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register( + makeClient("conn-1", "node-1", frames, { + clientId: "openclaw-node-host", + platform: "linux", + }), + {}, + ); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-required", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(false); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("generates and forwards a runId when system.run params omit it", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { command: ["/bin/sh", "-lc", "printf ok"], sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + const request = JSON.parse(frames[0] ?? "{}") as { + payload?: { paramsJSON?: string | null }; + }; + const forwarded = JSON.parse(request.payload?.paramsJSON ?? "{}") as { runId?: unknown }; + + expect(typeof forwarded.runId).toBe("string"); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: forwarded.runId as string, + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("clears system.run event authorization when invoke result fails", async () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-failed", sessionKey: "agent:main:main", timeoutMs: 0 }, + timeoutMs: 1_000, + }); + const request = JSON.parse(frames[0] ?? "{}") as { payload?: { id?: string } }; + + expect( + registry.handleInvokeResult({ + id: request.payload?.id ?? "", + nodeId: "node-1", + connId: "conn-1", + ok: false, + error: { code: "INVALID_REQUEST", message: "invalid params" }, + }), + ).toBe(true); + await expect(invoke).resolves.toEqual({ + ok: false, + payload: undefined, + payloadJSON: null, + error: { code: "INVALID_REQUEST", message: "invalid params" }, + }); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-failed", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(false); + }); + + it("matches legacy macOS exec events with runtime-generated runId when single pending run matches", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "gateway-run", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "legacy-runtime-run", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("rejects mismatched runId fallback for non-macOS nodes", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register( + makeClient("conn-1", "node-1", frames, { + clientId: "openclaw-node-host", + platform: "linux", + }), + {}, + ); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "gateway-run", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "runtime-run", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(false); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("matches system.run events with emitted session key when invoke omitted sessionKey", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-without-session" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-without-session", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(true); + registry.unregister("conn-1"); + void invoke.catch(() => {}); + }); + + it("rejects runId-less system.run events when the connection has multiple matches", () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeClient("conn-1", "node-1", frames), {}); + const first = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-a", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + const second = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-b", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + sessionKey: "agent:main:main", + terminal: true, + }), + ).toBe(false); + registry.unregister("conn-1"); + void first.catch(() => {}); + void second.catch(() => {}); + }); + it("sends raw event payload JSON without changing the envelope shape", () => { const registry = new NodeRegistry(); const frames: string[] = []; diff --git a/src/gateway/node-registry.ts b/src/gateway/node-registry.ts index 6a5e5278bfa..46707594968 100644 --- a/src/gateway/node-registry.ts +++ b/src/gateway/node-registry.ts @@ -26,11 +26,24 @@ type PendingInvoke = { nodeId: string; connId: string; command: string; + systemRunEvent?: PendingSystemRunEvent; resolve: (value: NodeInvokeResult) => void; reject: (err: Error) => void; timer: ReturnType; }; +type PendingSystemRunEvent = { + runId: string; + sessionKey?: string; + timeoutMs?: number | null; +}; + +type AuthorizedSystemRunEvent = PendingSystemRunEvent & { + nodeId: string; + connId: string; + expiresAtMs: number | null; +}; + type NodeInvokeResult = { ok: boolean; payload?: unknown; @@ -39,6 +52,7 @@ type NodeInvokeResult = { }; const SERIALIZED_EVENT_PAYLOAD = Symbol("openclaw.serializedEventPayload"); +const AUTHORIZED_SYSTEM_RUN_EVENT_GRACE_MS = 5 * 60 * 1000; export type SerializedEventPayload = { readonly json: string; @@ -62,10 +76,63 @@ function isSerializedEventPayload(value: unknown): value is SerializedEventPaylo ); } +function normalizeString(value: unknown): string { + return typeof value === "string" ? value.trim() : ""; +} + +function normalizeSystemRunTimeoutMs(value: unknown): number | null | undefined { + if (value === undefined) { + return undefined; + } + if (typeof value !== "number" || !Number.isFinite(value)) { + return undefined; + } + const timeoutMs = Math.trunc(value); + return timeoutMs > 0 ? timeoutMs : null; +} + +function resolvePendingSystemRunEvent(params: { + command: string; + params?: unknown; +}): PendingSystemRunEvent | undefined { + if (params.command !== "system.run" || !params.params || typeof params.params !== "object") { + return undefined; + } + const obj = params.params as Record; + const runId = normalizeString(obj.runId); + if (!runId) { + return undefined; + } + const timeoutMs = normalizeSystemRunTimeoutMs(obj.timeoutMs); + const sessionKey = normalizeString(obj.sessionKey); + return { + runId, + ...(sessionKey ? { sessionKey } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; +} + +function withSystemRunEventRunId(params: { command: string; params?: unknown }): unknown { + if ( + params.command !== "system.run" || + !params.params || + typeof params.params !== "object" || + Array.isArray(params.params) + ) { + return params.params; + } + const obj = params.params as Record; + if (normalizeString(obj.runId)) { + return params.params; + } + return { ...obj, runId: randomUUID() }; +} + export class NodeRegistry { private nodesById = new Map(); private nodesByConn = new Map(); private pendingInvokes = new Map(); + private authorizedSystemRunEvents = new Map(); register(client: GatewayWsClient, opts: { remoteIp?: string | undefined }) { const connect = client.connect; @@ -125,6 +192,11 @@ export class NodeRegistry { pending.reject(new Error(`node disconnected (${pending.command})`)); this.pendingInvokes.delete(id); } + for (const [key, event] of this.authorizedSystemRunEvents) { + if (event.connId === connId) { + this.authorizedSystemRunEvents.delete(key); + } + } return unregistersCurrentNode ? nodeId : null; } @@ -151,12 +223,16 @@ export class NodeRegistry { }; } const requestId = randomUUID(); + const invokeParams = withSystemRunEventRunId({ + command: params.command, + params: params.params, + }); const payload = { id: requestId, nodeId: params.nodeId, command: params.command, paramsJSON: - "params" in params && params.params !== undefined ? JSON.stringify(params.params) : null, + "params" in params && invokeParams !== undefined ? JSON.stringify(invokeParams) : null, timeoutMs: params.timeoutMs, idempotencyKey: params.idempotencyKey, }; @@ -167,6 +243,17 @@ export class NodeRegistry { error: { code: "UNAVAILABLE", message: "failed to send invoke to node" }, }; } + const systemRunEvent = resolvePendingSystemRunEvent({ + command: params.command, + params: invokeParams, + }); + if (systemRunEvent) { + this.rememberAuthorizedSystemRunEvent({ + nodeId: params.nodeId, + connId: node.connId, + ...systemRunEvent, + }); + } const timeoutMs = typeof params.timeoutMs === "number" ? params.timeoutMs : 30_000; return await new Promise((resolve, reject) => { const timer = setTimeout(() => { @@ -180,6 +267,7 @@ export class NodeRegistry { nodeId: params.nodeId, connId: node.connId, command: params.command, + systemRunEvent, resolve, reject, timer, @@ -187,6 +275,150 @@ export class NodeRegistry { }); } + authorizeSystemRunEvent(params: { + nodeId: string; + connId?: string; + runId?: string; + sessionKey: string; + terminal: boolean; + }): boolean { + if (!params.connId || !params.sessionKey) { + return false; + } + const connId = params.connId; + this.pruneAuthorizedSystemRunEvents(); + let match: { key: string; event: AuthorizedSystemRunEvent } | null = null; + if (params.runId) { + match = this.matchAuthorizedSystemRunEvent({ + nodeId: params.nodeId, + connId, + runId: params.runId, + sessionKey: params.sessionKey, + }); + if (!match && this.allowsLegacyMacRunIdFallback({ nodeId: params.nodeId, connId })) { + match = this.matchSingleAuthorizedSystemRunEvent({ + nodeId: params.nodeId, + connId, + sessionKey: params.sessionKey, + }); + } + } else { + if (!this.allowsLegacyMacRunIdFallback({ nodeId: params.nodeId, connId })) { + return false; + } + match = this.matchSingleAuthorizedSystemRunEvent({ + nodeId: params.nodeId, + connId, + sessionKey: params.sessionKey, + }); + } + if (!match) { + return false; + } + if (params.terminal) { + this.authorizedSystemRunEvents.delete(match.key); + } + return true; + } + + private rememberAuthorizedSystemRunEvent( + event: Omit, + ): void { + this.pruneAuthorizedSystemRunEvents(); + const authorized: AuthorizedSystemRunEvent = { + ...event, + expiresAtMs: this.authorizedSystemRunEventExpiresAt(event.timeoutMs), + }; + this.authorizedSystemRunEvents.set(this.authorizedSystemRunEventKey(authorized), authorized); + } + + private forgetAuthorizedSystemRunEvent( + event: Omit, + ): void { + this.authorizedSystemRunEvents.delete(this.authorizedSystemRunEventKey(event)); + } + + private authorizedSystemRunEventExpiresAt(timeoutMs: number | null | undefined): number | null { + if (typeof timeoutMs !== "number") { + return null; + } + return Date.now() + timeoutMs + AUTHORIZED_SYSTEM_RUN_EVENT_GRACE_MS; + } + + private matchAuthorizedSystemRunEvent(params: { + nodeId: string; + connId: string; + runId: string; + sessionKey: string; + }): { key: string; event: AuthorizedSystemRunEvent } | null { + for (const [key, event] of this.authorizedSystemRunEvents) { + if ( + event.nodeId === params.nodeId && + event.connId === params.connId && + event.runId === params.runId && + this.authorizedSystemRunSessionMatches(event, params.sessionKey) + ) { + return { key, event }; + } + } + return null; + } + + private matchSingleAuthorizedSystemRunEvent(params: { + nodeId: string; + connId: string; + sessionKey: string; + }): { key: string; event: AuthorizedSystemRunEvent } | null { + let match: { key: string; event: AuthorizedSystemRunEvent } | null = null; + for (const [key, event] of this.authorizedSystemRunEvents) { + if ( + event.nodeId !== params.nodeId || + event.connId !== params.connId || + !this.authorizedSystemRunSessionMatches(event, params.sessionKey) + ) { + continue; + } + if (match) { + return null; + } + match = { key, event }; + } + return match; + } + + private authorizedSystemRunSessionMatches( + event: AuthorizedSystemRunEvent, + sessionKey: string, + ): boolean { + return !event.sessionKey || event.sessionKey === sessionKey; + } + + private allowsLegacyMacRunIdFallback(params: { nodeId: string; connId: string }): boolean { + const node = this.nodesById.get(params.nodeId); + return ( + node?.connId === params.connId && + node.clientId === "openclaw-macos" && + node.platform === "darwin" + ); + } + + private pruneAuthorizedSystemRunEvents(now = Date.now()): void { + for (const [key, event] of this.authorizedSystemRunEvents) { + if (event.expiresAtMs !== null && event.expiresAtMs <= now) { + this.authorizedSystemRunEvents.delete(key); + } + } + } + + private authorizedSystemRunEventKey(params: { + nodeId: string; + connId: string; + runId: string; + sessionKey?: string; + }): string { + return `${params.nodeId}\0${params.connId}\0${params.sessionKey ?? ""}\0${params.runId}`; + } + handleInvokeResult(params: { id: string; nodeId: string; @@ -205,6 +437,13 @@ export class NodeRegistry { } clearTimeout(pending.timer); this.pendingInvokes.delete(params.id); + if (!params.ok && pending.systemRunEvent) { + this.forgetAuthorizedSystemRunEvent({ + nodeId: pending.nodeId, + connId: pending.connId, + ...pending.systemRunEvent, + }); + } pending.resolve({ ok: params.ok, payload: params.payload, diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 3ca697560af..ddd1774c493 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -51,6 +51,7 @@ import { validateNodePairVerifyParams, validateNodeRenameParams, } from "../protocol/index.js"; +import type { NodeEventContext } from "../server-node-events-types.js"; import { NODE_WAKE_RECONNECT_POLL_MS, NODE_WAKE_RECONNECT_RETRY_WAIT_MS, @@ -1299,7 +1300,7 @@ export const nodeHandlers: GatewayRequestHandlers = { await respondUnavailableOnThrow(respond, async () => { const { handleNodeEvent } = await import("../server-node-events.js"); const nodeId = client?.connect?.device?.id ?? client?.connect?.client?.id ?? "node"; - const nodeContext = { + const nodeContext: NodeEventContext = { deps: context.deps, broadcast: context.broadcast, nodeSendToSession: context.nodeSendToSession, @@ -1317,6 +1318,14 @@ export const nodeHandlers: GatewayRequestHandlers = { getHealthCache: context.getHealthCache, refreshHealthSnapshot: context.refreshHealthSnapshot, loadGatewayModelCatalog: context.loadGatewayModelCatalog, + authorizeNodeSystemRunEvent: (eventParams) => + context.nodeRegistry.authorizeSystemRunEvent({ + nodeId: eventParams.nodeId, + connId: eventParams.connId, + runId: eventParams.runId, + sessionKey: eventParams.sessionKey, + terminal: eventParams.terminal, + }), logGateway: { warn: context.logGateway.warn }, }; const result = await handleNodeEvent( @@ -1326,7 +1335,7 @@ export const nodeHandlers: GatewayRequestHandlers = { event: p.event, payloadJSON, }, - { deviceId: client?.connect?.device?.id }, + { connId: client?.connId, deviceId: client?.connect?.device?.id }, ); respond(true, result ?? { ok: true }, undefined); }); diff --git a/src/gateway/server-node-events-types.ts b/src/gateway/server-node-events-types.ts index bacc3c14239..77f8161dc64 100644 --- a/src/gateway/server-node-events-types.ts +++ b/src/gateway/server-node-events-types.ts @@ -30,6 +30,13 @@ export type NodeEventContext = { includeSensitive?: boolean; }) => Promise; loadGatewayModelCatalog: () => Promise; + authorizeNodeSystemRunEvent: (params: { + nodeId: string; + connId?: string; + runId?: string; + sessionKey: string; + terminal: boolean; + }) => boolean; logGateway: { warn: (msg: string) => void }; }; diff --git a/src/gateway/server-node-events.test.ts b/src/gateway/server-node-events.test.ts index 3d337b8a385..9954c9f7bdd 100644 --- a/src/gateway/server-node-events.test.ts +++ b/src/gateway/server-node-events.test.ts @@ -1,5 +1,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { NodeRegistry } from "./node-registry.js"; +import { PROTOCOL_VERSION } from "./protocol/index.js"; +import type { GatewayWsClient } from "./server/ws-types.js"; import type { loadSessionEntry as loadSessionEntryType } from "./session-utils.js"; const buildSessionLookup = ( @@ -159,7 +162,9 @@ const loadSessionEntryMock = runtimeMocks.loadSessionEntry; const registerApnsRegistrationVi = runtimeMocks.registerApnsRegistration; const normalizeChannelIdVi = runtimeMocks.normalizeChannelId; -function buildCtx(): NodeEventContext { +function buildCtx( + opts: { authorizeNodeSystemRunEvent?: NodeEventContext["authorizeNodeSystemRunEvent"] } = {}, +): NodeEventContext { return { deps: {} as CliDeps, broadcast: () => {}, @@ -178,10 +183,46 @@ function buildCtx(): NodeEventContext { getHealthCache: () => null, refreshHealthSnapshot: async () => ({}) as HealthSummary, loadGatewayModelCatalog: async () => [], + authorizeNodeSystemRunEvent: opts.authorizeNodeSystemRunEvent ?? (() => false), logGateway: { warn: () => {} }, }; } +function buildExecCtx() { + return buildCtx({ authorizeNodeSystemRunEvent: () => true }); +} + +function makeNodeClient(connId: string, nodeId: string, sent: string[] = []): GatewayWsClient { + return { + connId, + usesSharedGatewayAuth: false, + socket: { + send(frame: unknown) { + if (typeof frame === "string") { + sent.push(frame); + } + }, + } as unknown as GatewayWsClient["socket"], + connect: { + minProtocol: PROTOCOL_VERSION, + maxProtocol: PROTOCOL_VERSION, + client: { + id: "node-host", + version: "1.0.0", + platform: "linux", + mode: "node", + }, + device: { + id: nodeId, + publicKey: "public-key", + signature: "signature", + signedAt: 1, + nonce: "nonce", + }, + } as GatewayWsClient["connect"], + }; +} + function expectFields(value: unknown, expected: Record): void { if (!value || typeof value !== "object") { throw new Error("expected fields object"); @@ -231,7 +272,7 @@ describe("node exec events", () => { }); it("enqueues exec.started events", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-1", { event: "exec.started", payloadJSON: JSON.stringify({ @@ -251,8 +292,112 @@ describe("node exec events", () => { }); }); - it("enqueues exec.finished events with output", async () => { + it("rejects exec lifecycle events without a pending node run", async () => { const ctx = buildCtx(); + const result = await handleNodeEvent( + ctx, + "node-1", + { + event: "exec.finished", + payloadJSON: JSON.stringify({ + sessionKey: "agent:main:main", + runId: "forged-run", + exitCode: 0, + output: "done", + }), + }, + { connId: "conn-1" }, + ); + + expect(result).toEqual({ + ok: true, + event: "exec.finished", + handled: false, + reason: "unmatched_exec_event", + }); + expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + expect(requestHeartbeatMock).not.toHaveBeenCalled(); + }); + + it("keeps a node run authorized from exec.started through exec.finished", async () => { + const registry = new NodeRegistry(); + const frames: string[] = []; + registry.register(makeNodeClient("conn-1", "node-1", frames), {}); + const invoke = registry.invoke({ + nodeId: "node-1", + command: "system.run", + params: { runId: "run-seq", sessionKey: "agent:main:main" }, + timeoutMs: 1_000, + }); + const invokeSettled = invoke.catch(() => {}); + const ctx = buildCtx({ + authorizeNodeSystemRunEvent: (params) => registry.authorizeSystemRunEvent(params), + }); + + await handleNodeEvent( + ctx, + "node-1", + { + event: "exec.started", + payloadJSON: JSON.stringify({ + sessionKey: "agent:main:main", + runId: "run-seq", + command: "printf ok", + }), + }, + { connId: "conn-1" }, + ); + await handleNodeEvent( + ctx, + "node-1", + { + event: "exec.finished", + payloadJSON: JSON.stringify({ + sessionKey: "agent:main:main", + runId: "run-seq", + command: "printf ok", + exitCode: 0, + timedOut: false, + output: "done", + }), + }, + { connId: "conn-1" }, + ); + + expect(enqueueSystemEventMock).toHaveBeenNthCalledWith( + 1, + "Exec started (node=node-1 id=run-seq): printf ok", + { sessionKey: "agent:main:main", contextKey: "exec:run-seq", trusted: false }, + ); + expect(enqueueSystemEventMock).toHaveBeenNthCalledWith( + 2, + "Exec finished (node=node-1 id=run-seq, code 0)\ndone", + { sessionKey: "agent:main:main", contextKey: "exec:run-seq", trusted: false }, + ); + expect(requestHeartbeatMock).toHaveBeenNthCalledWith(1, { + reason: "exec-event", + sessionKey: "agent:main:main", + }); + expect(requestHeartbeatMock).toHaveBeenNthCalledWith(2, { + reason: "exec-event", + sessionKey: "agent:main:main", + }); + expect( + registry.authorizeSystemRunEvent({ + nodeId: "node-1", + connId: "conn-1", + runId: "run-seq", + sessionKey: "agent:main:main", + terminal: false, + }), + ).toBe(false); + + registry.unregister("conn-1"); + await invokeSettled; + }); + + it("enqueues exec.finished events with output", async () => { + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-2", { event: "exec.finished", payloadJSON: JSON.stringify({ @@ -270,8 +415,42 @@ describe("node exec events", () => { expect(requestHeartbeatMock).toHaveBeenCalledWith({ reason: "exec-event" }); }); + it("accepts legacy exec.finished events when authorization matches without runId", async () => { + const authorizeNodeSystemRunEvent = vi.fn(() => true); + const ctx = buildCtx({ authorizeNodeSystemRunEvent }); + await handleNodeEvent( + ctx, + "node-2", + { + event: "exec.finished", + payloadJSON: JSON.stringify({ + sessionKey: "agent:main:main", + exitCode: 0, + timedOut: false, + output: "done", + }), + }, + { connId: "conn-1" }, + ); + + expect(authorizeNodeSystemRunEvent).toHaveBeenCalledWith({ + nodeId: "node-2", + connId: "conn-1", + sessionKey: "agent:main:main", + terminal: true, + }); + expect(enqueueSystemEventMock).toHaveBeenCalledWith( + "Exec finished (node=node-2, code 0)\ndone", + { sessionKey: "agent:main:main", contextKey: "exec", trusted: false }, + ); + expect(requestHeartbeatMock).toHaveBeenCalledWith({ + reason: "exec-event", + sessionKey: "agent:main:main", + }); + }); + it("dedupes duplicate exec.finished events for the same runId on the same session", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); const payloadJSON = JSON.stringify({ sessionKey: "agent:main:main", runId: "run-dup-finished", @@ -306,7 +485,7 @@ describe("node exec events", () => { ...buildSessionLookup("node-node-2"), canonicalKey: "agent:main:node-node-2", }); - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-2", { event: "exec.finished", payloadJSON: JSON.stringify({ @@ -329,7 +508,7 @@ describe("node exec events", () => { }); it("suppresses noisy exec.finished success events with empty output", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-2", { event: "exec.finished", payloadJSON: JSON.stringify({ @@ -345,7 +524,7 @@ describe("node exec events", () => { }); it("truncates long exec.finished output in system events", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-2", { event: "exec.finished", payloadJSON: JSON.stringify({ @@ -365,7 +544,7 @@ describe("node exec events", () => { }); it("enqueues exec.denied events with reason", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-3", { event: "exec.denied", payloadJSON: JSON.stringify({ @@ -394,7 +573,7 @@ describe("node exec events", () => { session: { mainKey: string }; tools: { exec: { notifyOnExit: boolean } }; }); - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-1", { event: "exec.started", payloadJSON: JSON.stringify({ @@ -416,7 +595,7 @@ describe("node exec events", () => { session: { mainKey: string }; tools: { exec: { notifyOnExit: boolean } }; }); - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-2", { event: "exec.finished", payloadJSON: JSON.stringify({ @@ -439,7 +618,7 @@ describe("node exec events", () => { session: { mainKey: string }; tools: { exec: { notifyOnExit: boolean } }; }); - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-3", { event: "exec.denied", payloadJSON: JSON.stringify({ @@ -455,7 +634,7 @@ describe("node exec events", () => { }); it("sanitizes remote exec event content before enqueue", async () => { - const ctx = buildCtx(); + const ctx = buildExecCtx(); await handleNodeEvent(ctx, "node-4", { event: "exec.denied", payloadJSON: JSON.stringify({ diff --git a/src/gateway/server-node-events.ts b/src/gateway/server-node-events.ts index 48ed7f488ef..b9096b88769 100644 --- a/src/gateway/server-node-events.ts +++ b/src/gateway/server-node-events.ts @@ -365,7 +365,7 @@ export const handleNodeEvent = async ( ctx: NodeEventContext, nodeId: string, evt: NodeEvent, - opts?: { deviceId?: string }, + opts?: { connId?: string; deviceId?: string }, ): Promise => { switch (evt.event) { case "voice.transcript": { @@ -695,9 +695,27 @@ export const handleNodeEvent = async ( } const { canonicalKey: sessionKey } = loadSessionEntry(sessionKeyRaw); + const cfg = getRuntimeConfig(); + const runId = normalizeOptionalString(obj.runId) ?? ""; + if ( + !ctx.authorizeNodeSystemRunEvent({ + nodeId, + connId: opts?.connId, + ...(runId ? { runId } : {}), + // Match the key sent in system.run params; canonicalization below is for routing. + sessionKey: sessionKeyRaw, + terminal: evt.event === "exec.finished" || evt.event === "exec.denied", + }) + ) { + return { + ok: true, + event: evt.event, + handled: false, + reason: "unmatched_exec_event", + }; + } // Respect tools.exec.notifyOnExit setting (default: true) // When false, skip system event notifications for node exec events. - const cfg = getRuntimeConfig(); const notifyOnExit = cfg.tools?.exec?.notifyOnExit !== false; if (!notifyOnExit) { return undefined; @@ -705,8 +723,6 @@ export const handleNodeEvent = async ( if (obj.suppressNotifyOnExit === true) { return undefined; } - - const runId = normalizeOptionalString(obj.runId) ?? ""; const command = sanitizeInboundSystemTags(normalizeOptionalString(obj.command) ?? ""); const exitCode = typeof obj.exitCode === "number" && Number.isFinite(obj.exitCode)