Skip to content

Commit 1ca12ec

Browse files
vincentkocvincentkoc
authored
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
1 parent fc5a2f9 commit 1ca12ec

6 files changed

Lines changed: 79 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai
3838

3939
### Fixes
4040

41+
- 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.
4142
- 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.
4243
- 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.
4344
- 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.

‎src/gateway/hooks.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,22 +297,22 @@ describe("gateway hooks helpers", () => {
297297
expect(resolvedKey).toEqual({ ok: true, value: "hook:ingress" });
298298
});
299299

300-
test("normalizeHookDispatchSessionKey strips duplicate target agent prefix", () => {
300+
test("normalizeHookDispatchSessionKey preserves target agent scope", () => {
301301
expect(
302302
normalizeHookDispatchSessionKey({
303303
sessionKey: "agent:hooks:slack:channel:c123",
304304
targetAgentId: "hooks",
305305
}),
306-
).toBe("slack:channel:c123");
306+
).toBe("agent:hooks:slack:channel:c123");
307307
});
308308

309-
test("normalizeHookDispatchSessionKey preserves non-target agent scoped keys", () => {
309+
test("normalizeHookDispatchSessionKey rebinds non-target agent scoped keys to the target agent", () => {
310310
expect(
311311
normalizeHookDispatchSessionKey({
312312
sessionKey: "agent:main:slack:channel:c123",
313313
targetAgentId: "hooks",
314314
}),
315-
).toBe("agent:main:slack:channel:c123");
315+
).toBe("agent:hooks:slack:channel:c123");
316316
});
317317

318318
test("resolveHooksConfig validates defaultSessionKey and generated fallback against prefixes", () => {

‎src/gateway/hooks.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ function resolveAllowedSessionKeyPrefixes(raw: string[] | undefined): string[] |
127127
return set.size > 0 ? Array.from(set) : undefined;
128128
}
129129

130-
function isSessionKeyAllowedByPrefix(sessionKey: string, prefixes: string[]): boolean {
130+
export function isSessionKeyAllowedByPrefix(sessionKey: string, prefixes: string[]): boolean {
131131
const normalized = sessionKey.trim().toLowerCase();
132132
if (!normalized) {
133133
return false;
@@ -349,10 +349,7 @@ export function normalizeHookDispatchSessionKey(params: {
349349
return trimmed;
350350
}
351351
const targetAgentId = normalizeAgentId(params.targetAgentId);
352-
if (parsed.agentId !== targetAgentId) {
353-
return `agent:${parsed.agentId}:${parsed.rest}`;
354-
}
355-
return parsed.rest;
352+
return `agent:${targetAgentId}:${parsed.rest}`;
356353
}
357354

358355
export function normalizeAgentPayload(payload: Record<string, unknown>):

‎src/gateway/server-http.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ import {
4040
extractHookToken,
4141
getHookAgentPolicyError,
4242
getHookChannelError,
43+
getHookSessionKeyPrefixError,
4344
type HookAgentDispatchPayload,
4445
type HooksConfigResolved,
4546
isHookAgentAllowed,
47+
isSessionKeyAllowedByPrefix,
4648
normalizeAgentPayload,
4749
normalizeHookHeaders,
4850
resolveHookIdempotencyKey,
@@ -615,6 +617,14 @@ export function createHooksRequestHandler(
615617
sessionKey: sessionKey.value,
616618
targetAgentId,
617619
});
620+
const allowedPrefixes = hooksConfig.sessionPolicy.allowedSessionKeyPrefixes;
621+
if (
622+
allowedPrefixes &&
623+
!isSessionKeyAllowedByPrefix(normalizedDispatchSessionKey, allowedPrefixes)
624+
) {
625+
sendJson(res, 400, { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) });
626+
return true;
627+
}
618628
const runId = dispatchAgentHook({
619629
...normalized.value,
620630
idempotencyKey,
@@ -676,6 +686,14 @@ export function createHooksRequestHandler(
676686
sessionKey: sessionKey.value,
677687
targetAgentId,
678688
});
689+
const allowedPrefixes = hooksConfig.sessionPolicy.allowedSessionKeyPrefixes;
690+
if (
691+
allowedPrefixes &&
692+
!isSessionKeyAllowedByPrefix(normalizedDispatchSessionKey, allowedPrefixes)
693+
) {
694+
sendJson(res, 400, { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) });
695+
return true;
696+
}
679697
const replayKey = buildHookReplayCacheKey({
680698
pathKey: subPath || "mapping",
681699
token,

‎src/gateway/server.hooks.test.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ describe("gateway server hooks", () => {
328328
});
329329
});
330330

331-
test("normalizes duplicate target-agent prefixes before isolated dispatch", async () => {
331+
test("preserves target-agent prefixes before isolated dispatch", async () => {
332332
testState.hooksConfig = {
333333
enabled: true,
334334
token: HOOK_TOKEN,
@@ -352,11 +352,62 @@ describe("gateway server hooks", () => {
352352
| { sessionKey?: string; job?: { agentId?: string } }
353353
| undefined;
354354
expect(routedCall?.job?.agentId).toBe("hooks");
355-
expect(routedCall?.sessionKey).toBe("slack:channel:c123");
355+
expect(routedCall?.sessionKey).toBe("agent:hooks:slack:channel:c123");
356356
drainSystemEvents(resolveMainKey());
357357
});
358358
});
359359

360+
test("rebinds mismatched agent prefixes to the hook target before isolated dispatch", async () => {
361+
testState.hooksConfig = {
362+
enabled: true,
363+
token: HOOK_TOKEN,
364+
allowRequestSessionKey: true,
365+
allowedSessionKeyPrefixes: ["hook:", "agent:"],
366+
};
367+
setMainAndHooksAgents();
368+
await withGatewayServer(async ({ port }) => {
369+
mockIsolatedRunOkOnce();
370+
371+
const resAgent = await postHook(port, "/hooks/agent", {
372+
message: "Do it",
373+
name: "Email",
374+
agentId: "hooks",
375+
sessionKey: "agent:main:slack:channel:c123",
376+
});
377+
expect(resAgent.status).toBe(200);
378+
await waitForSystemEvent();
379+
380+
const routedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as
381+
| { sessionKey?: string; job?: { agentId?: string } }
382+
| undefined;
383+
expect(routedCall?.job?.agentId).toBe("hooks");
384+
expect(routedCall?.sessionKey).toBe("agent:hooks:slack:channel:c123");
385+
drainSystemEvents(resolveMainKey());
386+
});
387+
});
388+
389+
test("rejects rebinding into a session namespace that is not allowlisted", async () => {
390+
testState.hooksConfig = {
391+
enabled: true,
392+
token: HOOK_TOKEN,
393+
allowRequestSessionKey: true,
394+
allowedSessionKeyPrefixes: ["hook:", "agent:main:"],
395+
};
396+
setMainAndHooksAgents();
397+
await withGatewayServer(async ({ port }) => {
398+
const denied = await postHook(port, "/hooks/agent", {
399+
message: "Do it",
400+
name: "Email",
401+
agentId: "hooks",
402+
sessionKey: "agent:main:slack:channel:c123",
403+
});
404+
expect(denied.status).toBe(400);
405+
const body = (await denied.json()) as { error?: string };
406+
expect(body.error).toContain("sessionKey must start with one of");
407+
expect(cronIsolatedRun).not.toHaveBeenCalled();
408+
});
409+
});
410+
360411
test("dedupes repeated /hooks/agent deliveries by idempotency key", async () => {
361412
testState.hooksConfig = { enabled: true, token: HOOK_TOKEN };
362413
await withGatewayServer(async ({ port }) => {

‎src/gateway/server/hooks.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ export function createGatewayHooksRequestHandler(params: {
4242
};
4343

4444
const dispatchAgentHook = (value: HookAgentDispatchPayload) => {
45-
const sessionKey = normalizeHookDispatchSessionKey({
46-
sessionKey: value.sessionKey,
47-
targetAgentId: value.agentId,
48-
});
45+
const sessionKey = value.sessionKey;
4946
const mainSessionKey = resolveMainSessionKeyFromConfig();
5047
const jobId = randomUUID();
5148
const now = Date.now();

0 commit comments

Comments
 (0)