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
This commit is contained in:
Vincent Koc 2026-04-01 00:16:39 +09:00 committed by GitHub
parent fc5a2f9293
commit 1ca12ec8bf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 79 additions and 15 deletions

View File

@ -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.

View File

@ -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", () => {

View File

@ -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<string, unknown>):

View File

@ -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,

View File

@ -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 }) => {

View File

@ -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();