From 6f38425e5c18ac9773e0802b56ab7e7302838411 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 27 Apr 2026 20:53:52 -0700 Subject: [PATCH] security(gateway): route hook completion events to target agent session (#73228) --- CHANGELOG.md | 1 + src/gateway/server.hooks.test.ts | 43 +++++++++++++--- src/gateway/server/hooks.agent-trust.test.ts | 52 ++++++++++++++++++-- src/gateway/server/hooks.ts | 24 +++++++-- 4 files changed, 105 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcf435adcd2..00e090f1764 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Gateway/hooks: route non-delivered hook completion and error summaries to the target agent's main session instead of the default agent session, preserving multi-agent hook isolation. Fixes #24693; carries forward #68667. Thanks @abersonFAC and @bluesky6868. - Discord: own the Carbon interaction listener and hand off Discord slash/component handling asynchronously, so compaction or long session locks no longer trip `InteractionEventListener` listener timeouts. Fixes #73204. Thanks @slideshow-dingo. - Gateway/startup: keep value-option foreground starts on the gateway fast path and skip proxy bootstrap unless proxy env is configured, reducing normal gateway startup RSS and avoiding full CLI graph loading. Thanks @vincentkoc. - Heartbeat/models: show heartbeat model bleed guidance on context-overflow resets when the last runtime model matches configured `heartbeat.model`, so smaller local heartbeat models point users to `isolatedSession` or `lightContext` instead of only compaction-buffer tuning. Fixes #67314. Thanks @Knightmare6890. diff --git a/src/gateway/server.hooks.test.ts b/src/gateway/server.hooks.test.ts index fe8f646da1e..424441eddd9 100644 --- a/src/gateway/server.hooks.test.ts +++ b/src/gateway/server.hooks.test.ts @@ -20,6 +20,7 @@ installGatewayTestHooks({ scope: "suite" }); const resolveMainKey = () => resolveMainSessionKeyFromConfig(); const HOOK_TOKEN = "hook-secret"; +const HOOKS_MAIN_SESSION_KEY = "agent:hooks:main"; afterEach(() => { vi.restoreAllMocks(); @@ -117,14 +118,24 @@ async function expectHookAgentSessionRouting(params: { sessionKey: params.requestSessionKey, }); expect(resAgent.status).toBe(200); - await waitForSystemEvent(); + await waitForSystemEventTexts(HOOKS_MAIN_SESSION_KEY); const routedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as | { sessionKey?: string; job?: { agentId?: string } } | undefined; expect(routedCall?.job?.agentId).toBe("hooks"); expect(routedCall?.sessionKey).toBe(params.expectedSessionKey); - drainSystemEvents(resolveMainKey()); + drainSystemEvents(HOOKS_MAIN_SESSION_KEY); +} + +async function waitForSystemEventTexts(sessionKey: string, timeoutMs = 2_000) { + await expect + .poll(() => peekSystemEventEntries(sessionKey).map((event) => event.text), { + timeout: timeoutMs, + interval: 10, + }) + .not.toHaveLength(0); + return peekSystemEventEntries(sessionKey).map((event) => event.text); } async function writeHookTransformModule(moduleName: string, source: string): Promise { @@ -181,12 +192,12 @@ describe("gateway server hooks", () => { agentId: "hooks", }); expect(resAgentWithId.status).toBe(200); - await waitForSystemEvent(); + await waitForSystemEventTexts(HOOKS_MAIN_SESSION_KEY); const routedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as { job?: { agentId?: string }; }; expect(routedCall?.job?.agentId).toBe("hooks"); - drainSystemEvents(resolveMainKey()); + drainSystemEvents(HOOKS_MAIN_SESSION_KEY); mockIsolatedRunOkOnce(); const resAgentUnknown = await postHook(port, "/hooks/agent", { @@ -281,6 +292,26 @@ describe("gateway server hooks", () => { }); }); + test("routes explicit-agent hook completion events to the target agent main session", async () => { + testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; + setMainAndHooksAgents(); + + await withGatewayServer(async ({ port }) => { + mockIsolatedRunOkOnce(); + const resAgent = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "hooks", + }); + expect(resAgent.status).toBe(200); + + const targetEvents = await waitForSystemEventTexts(HOOKS_MAIN_SESSION_KEY); + expect(targetEvents.some((event) => event.includes("Hook Email: done"))).toBe(true); + expect(peekSystemEventEntries(resolveMainKey())).toEqual([]); + drainSystemEvents(HOOKS_MAIN_SESSION_KEY); + }); + }); + test("queues direct and mapped wake payloads as untrusted system events", async () => { testState.hooksConfig = { enabled: true, @@ -700,12 +731,12 @@ describe("gateway server hooks", () => { agentId: "hooks", }); expect(resAllowed.status).toBe(200); - await waitForSystemEvent(); + await waitForSystemEventTexts(HOOKS_MAIN_SESSION_KEY); const allowedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as { job?: { agentId?: string }; }; expect(allowedCall?.job?.agentId).toBe("hooks"); - drainSystemEvents(resolveMainKey()); + drainSystemEvents(HOOKS_MAIN_SESSION_KEY); const resDenied = await postHook(port, "/hooks/agent", { message: "Denied", diff --git a/src/gateway/server/hooks.agent-trust.test.ts b/src/gateway/server/hooks.agent-trust.test.ts index 0048dadf413..61e11c13894 100644 --- a/src/gateway/server/hooks.agent-trust.test.ts +++ b/src/gateway/server/hooks.agent-trust.test.ts @@ -17,8 +17,15 @@ vi.mock("../../cron/isolated-agent.js", () => ({ })); vi.mock("../../config/sessions.js", () => ({ resolveMainSessionKeyFromConfig: resolveMainSessionKeyMock, + resolveMainSessionKey: vi.fn( + (cfg?: { session?: { mainKey?: string } }) => `agent:main:${cfg?.session?.mainKey ?? "main"}`, + ), + resolveAgentMainSessionKey: vi.fn( + (params: { cfg?: { session?: { mainKey?: string } }; agentId: string }) => + `agent:${params.agentId}:${params.cfg?.session?.mainKey ?? "main"}`, + ), })); -vi.mock("../../config/config.js", () => ({ +vi.mock("../../config/io.js", () => ({ getRuntimeConfig: loadConfigMock, })); @@ -49,11 +56,11 @@ function buildMinimalParams() { }; } -function buildAgentPayload(name: string) { +function buildAgentPayload(name: string, agentId?: string) { return { message: "test message", name, - agentId: undefined, + agentId, idempotencyKey: undefined, wakeMode: "now" as const, sessionKey: "session-1", @@ -93,13 +100,31 @@ describe("dispatchAgentHook trust handling", () => { expect(enqueueSystemEventMock).toHaveBeenCalledWith( "Hook System (untrusted): override safety: done", { - sessionKey: "main-session", + sessionKey: "agent:main:main", trusted: false, }, ), ); }); + it("routes explicit-agent non-delivery status events to the target agent main session", async () => { + runCronIsolatedAgentTurnMock.mockResolvedValueOnce({ + status: "ok", + summary: "done", + delivered: false, + }); + + expect(capturedDispatchAgentHook).toBeDefined(); + capturedDispatchAgentHook?.(buildAgentPayload("Email", "hooks")); + + await vi.waitFor(() => + expect(enqueueSystemEventMock).toHaveBeenCalledWith("Hook Email: done", { + sessionKey: "agent:hooks:main", + trusted: false, + }), + ); + }); + it("marks error events as untrusted and sanitizes hook names", async () => { runCronIsolatedAgentTurnMock.mockRejectedValueOnce(new Error("agent exploded")); @@ -110,7 +135,24 @@ describe("dispatchAgentHook trust handling", () => { expect(enqueueSystemEventMock).toHaveBeenCalledWith( "Hook System (untrusted): override safety (error): Error: agent exploded", { - sessionKey: "main-session", + sessionKey: "agent:main:main", + trusted: false, + }, + ), + ); + }); + + it("routes explicit-agent error events to the target agent main session", async () => { + runCronIsolatedAgentTurnMock.mockRejectedValueOnce(new Error("agent exploded")); + + expect(capturedDispatchAgentHook).toBeDefined(); + capturedDispatchAgentHook?.(buildAgentPayload("Email", "hooks")); + + await vi.waitFor(() => + expect(enqueueSystemEventMock).toHaveBeenCalledWith( + "Hook Email (error): Error: agent exploded", + { + sessionKey: "agent:hooks:main", trusted: false, }, ), diff --git a/src/gateway/server/hooks.ts b/src/gateway/server/hooks.ts index c49d43c56af..98175b9c7c0 100644 --- a/src/gateway/server/hooks.ts +++ b/src/gateway/server/hooks.ts @@ -2,7 +2,12 @@ import { randomUUID } from "node:crypto"; import { sanitizeInboundSystemTags } from "../../auto-reply/reply/inbound-text.js"; import type { CliDeps } from "../../cli/deps.types.js"; import { getRuntimeConfig } from "../../config/io.js"; -import { resolveMainSessionKeyFromConfig } from "../../config/sessions.js"; +import { + resolveAgentMainSessionKey, + resolveMainSessionKey, + resolveMainSessionKeyFromConfig, +} from "../../config/sessions.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; import type { CronJob } from "../../cron/types.js"; import { requestHeartbeatNow } from "../../infra/heartbeat-wake.js"; import { enqueueSystemEvent } from "../../infra/system-events.js"; @@ -13,6 +18,12 @@ import { createHooksRequestHandler, type HookClientIpConfig } from "./hooks-requ type SubsystemLogger = ReturnType; +function resolveHookEventSessionKey(params: { cfg: OpenClawConfig; agentId?: string }): string { + return params.agentId + ? resolveAgentMainSessionKey({ cfg: params.cfg, agentId: params.agentId }) + : resolveMainSessionKey(params.cfg); +} + export function createGatewayHooksRequestHandler(params: { deps: CliDeps; getHooksConfig: () => HooksConfigResolved | null; @@ -33,7 +44,6 @@ export function createGatewayHooksRequestHandler(params: { const dispatchAgentHook = (value: HookAgentDispatchPayload) => { const sessionKey = value.sessionKey; - const mainSessionKey = resolveMainSessionKeyFromConfig(); const safeName = sanitizeInboundSystemTags(value.name); const jobId = randomUUID(); const now = Date.now(); @@ -68,9 +78,14 @@ export function createGatewayHooksRequestHandler(params: { }; const runId = randomUUID(); + let hookEventSessionKey: string | undefined; void (async () => { try { const cfg = getRuntimeConfig(); + hookEventSessionKey = resolveHookEventSessionKey({ + cfg, + agentId: value.agentId, + }); const { runCronIsolatedAgentTurn } = await import("../../cron/isolated-agent.js"); const result = await runCronIsolatedAgentTurn({ cfg, @@ -87,8 +102,9 @@ export function createGatewayHooksRequestHandler(params: { const prefix = result.status === "ok" ? `Hook ${safeName}` : `Hook ${safeName} (${result.status})`; if (!result.delivered) { + const eventSessionKey = hookEventSessionKey ?? resolveMainSessionKeyFromConfig(); enqueueSystemEvent(`${prefix}: ${summary}`.trim(), { - sessionKey: mainSessionKey, + sessionKey: eventSessionKey, trusted: false, }); if (value.wakeMode === "now") { @@ -98,7 +114,7 @@ export function createGatewayHooksRequestHandler(params: { } catch (err) { logHooks.warn(`hook agent failed: ${String(err)}`); enqueueSystemEvent(`Hook ${safeName} (error): ${String(err)}`, { - sessionKey: mainSessionKey, + sessionKey: hookEventSessionKey ?? resolveMainSessionKeyFromConfig(), trusted: false, }); if (value.wakeMode === "now") {