From 1ca12ec8bf763a9994a439fb4d6fcacfbe28a96b Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 1 Apr 2026 00:16:39 +0900 Subject: [PATCH] fix(hooks): rebind hook agent session keys to the target agent (#58225) * fix(hooks): rebind hook agent session keys * fix(hooks): preserve scoped hook session keys * fix(hooks): validate normalized dispatch keys --- CHANGELOG.md | 1 + src/gateway/hooks.test.ts | 8 ++--- src/gateway/hooks.ts | 7 ++-- src/gateway/server-http.ts | 18 +++++++++++ src/gateway/server.hooks.test.ts | 55 ++++++++++++++++++++++++++++++-- src/gateway/server/hooks.ts | 5 +-- 6 files changed, 79 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc6adddacd4..25d942ed35b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Hooks/session routing: rebind hook-triggered `agent:` session keys to the actual target agent before isolated dispatch so dedicated hook agents keep their own session-scoped tool and plugin identity. Thanks @kexinoh and @vincentkoc. - Outbound media/local files: piggyback host-local `MEDIA:` reads on the configured fs policy instead of a separate media-root check, so generated files outside the workspace can send when `tools.fs.workspaceOnly=false` while plaintext-like host files stay blocked by the outbound media allowlist. - Gateway/auth: reject mismatched browser `Origin` headers on trusted-proxy HTTP operator requests while keeping origin-less headless proxy clients working. Thanks @AntAISecurityLab and @vincentkoc. - Plugins/startup: block workspace `.env` from overriding `OPENCLAW_BUNDLED_PLUGINS_DIR`, so bundled plugin trust roots only come from inherited runtime env or package resolution instead of repo-local dotenv files. Thanks @nexrin and @vincentkoc. diff --git a/src/gateway/hooks.test.ts b/src/gateway/hooks.test.ts index 068ea69bc99..3c998b969cd 100644 --- a/src/gateway/hooks.test.ts +++ b/src/gateway/hooks.test.ts @@ -297,22 +297,22 @@ describe("gateway hooks helpers", () => { expect(resolvedKey).toEqual({ ok: true, value: "hook:ingress" }); }); - test("normalizeHookDispatchSessionKey strips duplicate target agent prefix", () => { + test("normalizeHookDispatchSessionKey preserves target agent scope", () => { expect( normalizeHookDispatchSessionKey({ sessionKey: "agent:hooks:slack:channel:c123", targetAgentId: "hooks", }), - ).toBe("slack:channel:c123"); + ).toBe("agent:hooks:slack:channel:c123"); }); - test("normalizeHookDispatchSessionKey preserves non-target agent scoped keys", () => { + test("normalizeHookDispatchSessionKey rebinds non-target agent scoped keys to the target agent", () => { expect( normalizeHookDispatchSessionKey({ sessionKey: "agent:main:slack:channel:c123", targetAgentId: "hooks", }), - ).toBe("agent:main:slack:channel:c123"); + ).toBe("agent:hooks:slack:channel:c123"); }); test("resolveHooksConfig validates defaultSessionKey and generated fallback against prefixes", () => { diff --git a/src/gateway/hooks.ts b/src/gateway/hooks.ts index 0733de195ca..f41f3fa819b 100644 --- a/src/gateway/hooks.ts +++ b/src/gateway/hooks.ts @@ -127,7 +127,7 @@ function resolveAllowedSessionKeyPrefixes(raw: string[] | undefined): string[] | return set.size > 0 ? Array.from(set) : undefined; } -function isSessionKeyAllowedByPrefix(sessionKey: string, prefixes: string[]): boolean { +export function isSessionKeyAllowedByPrefix(sessionKey: string, prefixes: string[]): boolean { const normalized = sessionKey.trim().toLowerCase(); if (!normalized) { return false; @@ -349,10 +349,7 @@ export function normalizeHookDispatchSessionKey(params: { return trimmed; } const targetAgentId = normalizeAgentId(params.targetAgentId); - if (parsed.agentId !== targetAgentId) { - return `agent:${parsed.agentId}:${parsed.rest}`; - } - return parsed.rest; + return `agent:${targetAgentId}:${parsed.rest}`; } export function normalizeAgentPayload(payload: Record): diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index 4bde6aed8fa..3dd587eccd5 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -40,9 +40,11 @@ import { extractHookToken, getHookAgentPolicyError, getHookChannelError, + getHookSessionKeyPrefixError, type HookAgentDispatchPayload, type HooksConfigResolved, isHookAgentAllowed, + isSessionKeyAllowedByPrefix, normalizeAgentPayload, normalizeHookHeaders, resolveHookIdempotencyKey, @@ -615,6 +617,14 @@ export function createHooksRequestHandler( sessionKey: sessionKey.value, targetAgentId, }); + const allowedPrefixes = hooksConfig.sessionPolicy.allowedSessionKeyPrefixes; + if ( + allowedPrefixes && + !isSessionKeyAllowedByPrefix(normalizedDispatchSessionKey, allowedPrefixes) + ) { + sendJson(res, 400, { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) }); + return true; + } const runId = dispatchAgentHook({ ...normalized.value, idempotencyKey, @@ -676,6 +686,14 @@ export function createHooksRequestHandler( sessionKey: sessionKey.value, targetAgentId, }); + const allowedPrefixes = hooksConfig.sessionPolicy.allowedSessionKeyPrefixes; + if ( + allowedPrefixes && + !isSessionKeyAllowedByPrefix(normalizedDispatchSessionKey, allowedPrefixes) + ) { + sendJson(res, 400, { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) }); + return true; + } const replayKey = buildHookReplayCacheKey({ pathKey: subPath || "mapping", token, diff --git a/src/gateway/server.hooks.test.ts b/src/gateway/server.hooks.test.ts index 9904ff16414..8fd25d9c791 100644 --- a/src/gateway/server.hooks.test.ts +++ b/src/gateway/server.hooks.test.ts @@ -328,7 +328,7 @@ describe("gateway server hooks", () => { }); }); - test("normalizes duplicate target-agent prefixes before isolated dispatch", async () => { + test("preserves target-agent prefixes before isolated dispatch", async () => { testState.hooksConfig = { enabled: true, token: HOOK_TOKEN, @@ -352,11 +352,62 @@ describe("gateway server hooks", () => { | { sessionKey?: string; job?: { agentId?: string } } | undefined; expect(routedCall?.job?.agentId).toBe("hooks"); - expect(routedCall?.sessionKey).toBe("slack:channel:c123"); + expect(routedCall?.sessionKey).toBe("agent:hooks:slack:channel:c123"); drainSystemEvents(resolveMainKey()); }); }); + test("rebinds mismatched agent prefixes to the hook target before isolated dispatch", async () => { + testState.hooksConfig = { + enabled: true, + token: HOOK_TOKEN, + allowRequestSessionKey: true, + allowedSessionKeyPrefixes: ["hook:", "agent:"], + }; + setMainAndHooksAgents(); + await withGatewayServer(async ({ port }) => { + mockIsolatedRunOkOnce(); + + const resAgent = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "hooks", + sessionKey: "agent:main:slack:channel:c123", + }); + expect(resAgent.status).toBe(200); + await waitForSystemEvent(); + + 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("agent:hooks:slack:channel:c123"); + drainSystemEvents(resolveMainKey()); + }); + }); + + test("rejects rebinding into a session namespace that is not allowlisted", async () => { + testState.hooksConfig = { + enabled: true, + token: HOOK_TOKEN, + allowRequestSessionKey: true, + allowedSessionKeyPrefixes: ["hook:", "agent:main:"], + }; + setMainAndHooksAgents(); + await withGatewayServer(async ({ port }) => { + const denied = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "hooks", + sessionKey: "agent:main:slack:channel:c123", + }); + expect(denied.status).toBe(400); + const body = (await denied.json()) as { error?: string }; + expect(body.error).toContain("sessionKey must start with one of"); + expect(cronIsolatedRun).not.toHaveBeenCalled(); + }); + }); + test("dedupes repeated /hooks/agent deliveries by idempotency key", async () => { testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; await withGatewayServer(async ({ port }) => { diff --git a/src/gateway/server/hooks.ts b/src/gateway/server/hooks.ts index 330addbc03c..fc9ecae6f42 100644 --- a/src/gateway/server/hooks.ts +++ b/src/gateway/server/hooks.ts @@ -42,10 +42,7 @@ export function createGatewayHooksRequestHandler(params: { }; const dispatchAgentHook = (value: HookAgentDispatchPayload) => { - const sessionKey = normalizeHookDispatchSessionKey({ - sessionKey: value.sessionKey, - targetAgentId: value.agentId, - }); + const sessionKey = value.sessionKey; const mainSessionKey = resolveMainSessionKeyFromConfig(); const jobId = randomUUID(); const now = Date.now();