fix: harden hook session key routing defaults

2026-05-08 20:58:26 +00:00 · 2026-02-13 02:09:01 +01:00
parent 0a7201fa84
commit 3421b2ec1e
15 changed files with 603 additions and 32 deletions
--- a/src/gateway/hooks.test.ts
+++ b/src/gateway/hooks.test.ts
@@ -7,6 +7,7 @@ import { createIMessageTestPlugin, createTestRegistry } from "../test-utils/chan
 import {
  extractHookToken,
  isHookAgentAllowed,
+  resolveHookSessionKey,
  resolveHookTargetAgentId,
  normalizeAgentPayload,
  normalizeWakePayload,
@@ -32,6 +33,7 @@ describe("gateway hooks helpers", () => {
    const resolved = resolveHooksConfig(base);
    expect(resolved?.basePath).toBe("/hooks");
    expect(resolved?.token).toBe("secret");
+    expect(resolved?.sessionPolicy.allowRequestSessionKey).toBe(false);
  });

  test("resolveHooksConfig rejects root path", () => {
@@ -71,19 +73,16 @@ describe("gateway hooks helpers", () => {
  });

  test("normalizeAgentPayload defaults + validates channel", () => {
-    const ok = normalizeAgentPayload({ message: "hello" }, { idFactory: () => "fixed" });
+    const ok = normalizeAgentPayload({ message: "hello" });
    expect(ok.ok).toBe(true);
    if (ok.ok) {
-      expect(ok.value.sessionKey).toBe("hook:fixed");
+      expect(ok.value.sessionKey).toBeUndefined();
      expect(ok.value.channel).toBe("last");
      expect(ok.value.name).toBe("Hook");
      expect(ok.value.deliver).toBe(true);
    }

-    const explicitNoDeliver = normalizeAgentPayload(
-      { message: "hello", deliver: false },
-      { idFactory: () => "fixed" },
-    );
+    const explicitNoDeliver = normalizeAgentPayload({ message: "hello", deliver: false });
    expect(explicitNoDeliver.ok).toBe(true);
    if (explicitNoDeliver.ok) {
      expect(explicitNoDeliver.value.deliver).toBe(false);
@@ -98,10 +97,7 @@ describe("gateway hooks helpers", () => {
        },
      ]),
    );
-    const imsg = normalizeAgentPayload(
-      { message: "yo", channel: "imsg" },
-      { idFactory: () => "x" },
-    );
+    const imsg = normalizeAgentPayload({ message: "yo", channel: "imsg" });
    expect(imsg.ok).toBe(true);
    if (imsg.ok) {
      expect(imsg.value.channel).toBe("imessage");
@@ -116,10 +112,7 @@ describe("gateway hooks helpers", () => {
        },
      ]),
    );
-    const teams = normalizeAgentPayload(
-      { message: "yo", channel: "teams" },
-      { idFactory: () => "x" },
-    );
+    const teams = normalizeAgentPayload({ message: "yo", channel: "teams" });
    expect(teams.ok).toBe(true);
    if (teams.ok) {
      expect(teams.value.channel).toBe("msteams");
@@ -130,16 +123,13 @@ describe("gateway hooks helpers", () => {
  });

  test("normalizeAgentPayload passes agentId", () => {
-    const ok = normalizeAgentPayload(
-      { message: "hello", agentId: "hooks" },
-      { idFactory: () => "fixed" },
-    );
+    const ok = normalizeAgentPayload({ message: "hello", agentId: "hooks" });
    expect(ok.ok).toBe(true);
    if (ok.ok) {
      expect(ok.value.agentId).toBe("hooks");
    }

-    const noAgent = normalizeAgentPayload({ message: "hello" }, { idFactory: () => "fixed" });
+    const noAgent = normalizeAgentPayload({ message: "hello" });
    expect(noAgent.ok).toBe(true);
    if (noAgent.ok) {
      expect(noAgent.value.agentId).toBeUndefined();
@@ -225,6 +215,116 @@ describe("gateway hooks helpers", () => {
    expect(isHookAgentAllowed(resolved, "hooks")).toBe(true);
    expect(isHookAgentAllowed(resolved, "missing-agent")).toBe(true);
  });
+
+  test("resolveHookSessionKey disables request sessionKey by default", () => {
+    const cfg = {
+      hooks: { enabled: true, token: "secret" },
+    } as OpenClawConfig;
+    const resolved = resolveHooksConfig(cfg);
+    expect(resolved).not.toBeNull();
+    if (!resolved) {
+      return;
+    }
+    const denied = resolveHookSessionKey({
+      hooksConfig: resolved,
+      source: "request",
+      sessionKey: "agent:main:dm:u99999",
+    });
+    expect(denied.ok).toBe(false);
+  });
+
+  test("resolveHookSessionKey allows request sessionKey when explicitly enabled", () => {
+    const cfg = {
+      hooks: { enabled: true, token: "secret", allowRequestSessionKey: true },
+    } as OpenClawConfig;
+    const resolved = resolveHooksConfig(cfg);
+    expect(resolved).not.toBeNull();
+    if (!resolved) {
+      return;
+    }
+    const allowed = resolveHookSessionKey({
+      hooksConfig: resolved,
+      source: "request",
+      sessionKey: "hook:manual",
+    });
+    expect(allowed).toEqual({ ok: true, value: "hook:manual" });
+  });
+
+  test("resolveHookSessionKey enforces allowed prefixes", () => {
+    const cfg = {
+      hooks: {
+        enabled: true,
+        token: "secret",
+        allowRequestSessionKey: true,
+        allowedSessionKeyPrefixes: ["hook:"],
+      },
+    } as OpenClawConfig;
+    const resolved = resolveHooksConfig(cfg);
+    expect(resolved).not.toBeNull();
+    if (!resolved) {
+      return;
+    }
+
+    const blocked = resolveHookSessionKey({
+      hooksConfig: resolved,
+      source: "request",
+      sessionKey: "agent:main:main",
+    });
+    expect(blocked.ok).toBe(false);
+
+    const allowed = resolveHookSessionKey({
+      hooksConfig: resolved,
+      source: "mapping",
+      sessionKey: "hook:gmail:1",
+    });
+    expect(allowed).toEqual({ ok: true, value: "hook:gmail:1" });
+  });
+
+  test("resolveHookSessionKey uses defaultSessionKey when request key is absent", () => {
+    const cfg = {
+      hooks: {
+        enabled: true,
+        token: "secret",
+        defaultSessionKey: "hook:ingress",
+      },
+    } as OpenClawConfig;
+    const resolved = resolveHooksConfig(cfg);
+    expect(resolved).not.toBeNull();
+    if (!resolved) {
+      return;
+    }
+
+    const resolvedKey = resolveHookSessionKey({
+      hooksConfig: resolved,
+      source: "request",
+    });
+    expect(resolvedKey).toEqual({ ok: true, value: "hook:ingress" });
+  });
+
+  test("resolveHooksConfig validates defaultSessionKey and generated fallback against prefixes", () => {
+    expect(() =>
+      resolveHooksConfig({
+        hooks: {
+          enabled: true,
+          token: "secret",
+          defaultSessionKey: "agent:main:main",
+          allowedSessionKeyPrefixes: ["hook:"],
+        },
+      } as OpenClawConfig),
+    ).toThrow("hooks.defaultSessionKey must match hooks.allowedSessionKeyPrefixes");
+
+    expect(() =>
+      resolveHooksConfig({
+        hooks: {
+          enabled: true,
+          token: "secret",
+          allowedSessionKeyPrefixes: ["agent:"],
+        },
+      } as OpenClawConfig),
+    ).toThrow(
+      "hooks.allowedSessionKeyPrefixes must include 'hook:' when hooks.defaultSessionKey is unset",
+    );
+  });
 });

 const emptyRegistry = createTestRegistry([]);
--- a/src/gateway/hooks.ts
+++ b/src/gateway/hooks.ts
@@ -17,6 +17,7 @@ export type HooksConfigResolved = {
  maxBodyBytes: number;
  mappings: HookMappingResolved[];
  agentPolicy: HookAgentPolicyResolved;
+  sessionPolicy: HookSessionPolicyResolved;
 };

 export type HookAgentPolicyResolved = {
@@ -25,6 +26,12 @@ export type HookAgentPolicyResolved = {
  allowedAgentIds?: Set<string>;
 };

+export type HookSessionPolicyResolved = {
+  defaultSessionKey?: string;
+  allowRequestSessionKey: boolean;
+  allowedSessionKeyPrefixes?: string[];
+};
+
 export function resolveHooksConfig(cfg: OpenClawConfig): HooksConfigResolved | null {
  if (cfg.hooks?.enabled !== true) {
    return null;
@@ -47,6 +54,26 @@ export function resolveHooksConfig(cfg: OpenClawConfig): HooksConfigResolved | n
  const defaultAgentId = resolveDefaultAgentId(cfg);
  const knownAgentIds = resolveKnownAgentIds(cfg, defaultAgentId);
  const allowedAgentIds = resolveAllowedAgentIds(cfg.hooks?.allowedAgentIds);
+  const defaultSessionKey = resolveSessionKey(cfg.hooks?.defaultSessionKey);
+  const allowedSessionKeyPrefixes = resolveAllowedSessionKeyPrefixes(
+    cfg.hooks?.allowedSessionKeyPrefixes,
+  );
+  if (
+    defaultSessionKey &&
+    allowedSessionKeyPrefixes &&
+    !isSessionKeyAllowedByPrefix(defaultSessionKey, allowedSessionKeyPrefixes)
+  ) {
+    throw new Error("hooks.defaultSessionKey must match hooks.allowedSessionKeyPrefixes");
+  }
+  if (
+    !defaultSessionKey &&
+    allowedSessionKeyPrefixes &&
+    !isSessionKeyAllowedByPrefix("hook:example", allowedSessionKeyPrefixes)
+  ) {
+    throw new Error(
+      "hooks.allowedSessionKeyPrefixes must include 'hook:' when hooks.defaultSessionKey is unset",
+    );
+  }
  return {
    basePath: trimmed,
    token,
@@ -57,6 +84,11 @@ export function resolveHooksConfig(cfg: OpenClawConfig): HooksConfigResolved | n
      knownAgentIds,
      allowedAgentIds,
    },
+    sessionPolicy: {
+      defaultSessionKey,
+      allowRequestSessionKey: cfg.hooks?.allowRequestSessionKey === true,
+      allowedSessionKeyPrefixes,
+    },
  };
 }

@@ -89,6 +121,39 @@ function resolveAllowedAgentIds(raw: string[] | undefined): Set<string> | undefi
  return allowed;
 }

+function resolveSessionKey(raw: string | undefined): string | undefined {
+  const value = raw?.trim();
+  return value ? value : undefined;
+}
+
+function normalizeSessionKeyPrefix(raw: string): string | undefined {
+  const value = raw.trim().toLowerCase();
+  return value ? value : undefined;
+}
+
+function resolveAllowedSessionKeyPrefixes(raw: string[] | undefined): string[] | undefined {
+  if (!Array.isArray(raw)) {
+    return undefined;
+  }
+  const set = new Set<string>();
+  for (const prefix of raw) {
+    const normalized = normalizeSessionKeyPrefix(prefix);
+    if (!normalized) {
+      continue;
+    }
+    set.add(normalized);
+  }
+  return set.size > 0 ? Array.from(set) : undefined;
+}
+
+function isSessionKeyAllowedByPrefix(sessionKey: string, prefixes: string[]): boolean {
+  const normalized = sessionKey.trim().toLowerCase();
+  if (!normalized) {
+    return false;
+  }
+  return prefixes.some((prefix) => normalized.startsWith(prefix));
+}
+
 export function extractHookToken(req: IncomingMessage): string | undefined {
  const auth =
    typeof req.headers.authorization === "string" ? req.headers.authorization.trim() : "";
@@ -186,7 +251,7 @@ export type HookAgentPayload = {
  name: string;
  agentId?: string;
  wakeMode: "now" | "next-heartbeat";
-  sessionKey: string;
+  sessionKey?: string;
  deliver: boolean;
  channel: HookMessageChannel;
  to?: string;
@@ -253,11 +318,43 @@ export function isHookAgentAllowed(
 }

 export const getHookAgentPolicyError = () => "agentId is not allowed by hooks.allowedAgentIds";
+export const getHookSessionKeyRequestPolicyError = () =>
+  "sessionKey is disabled for external /hooks/agent payloads; set hooks.allowRequestSessionKey=true to enable";
+export const getHookSessionKeyPrefixError = (prefixes: string[]) =>
+  `sessionKey must start with one of: ${prefixes.join(", ")}`;

-export function normalizeAgentPayload(
-  payload: Record<string, unknown>,
-  opts?: { idFactory?: () => string },
-):
+export function resolveHookSessionKey(params: {
+  hooksConfig: HooksConfigResolved;
+  source: "request" | "mapping";
+  sessionKey?: string;
+  idFactory?: () => string;
+}): { ok: true; value: string } | { ok: false; error: string } {
+  const requested = resolveSessionKey(params.sessionKey);
+  if (requested) {
+    if (params.source === "request" && !params.hooksConfig.sessionPolicy.allowRequestSessionKey) {
+      return { ok: false, error: getHookSessionKeyRequestPolicyError() };
+    }
+    const allowedPrefixes = params.hooksConfig.sessionPolicy.allowedSessionKeyPrefixes;
+    if (allowedPrefixes && !isSessionKeyAllowedByPrefix(requested, allowedPrefixes)) {
+      return { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) };
+    }
+    return { ok: true, value: requested };
+  }
+
+  const defaultSessionKey = params.hooksConfig.sessionPolicy.defaultSessionKey;
+  if (defaultSessionKey) {
+    return { ok: true, value: defaultSessionKey };
+  }
+
+  const generated = `hook:${(params.idFactory ?? randomUUID)()}`;
+  const allowedPrefixes = params.hooksConfig.sessionPolicy.allowedSessionKeyPrefixes;
+  if (allowedPrefixes && !isSessionKeyAllowedByPrefix(generated, allowedPrefixes)) {
+    return { ok: false, error: getHookSessionKeyPrefixError(allowedPrefixes) };
+  }
+  return { ok: true, value: generated };
+}
+
+export function normalizeAgentPayload(payload: Record<string, unknown>):
  | {
      ok: true;
      value: HookAgentPayload;
@@ -274,11 +371,8 @@ export function normalizeAgentPayload(
    typeof agentIdRaw === "string" && agentIdRaw.trim() ? agentIdRaw.trim() : undefined;
  const wakeMode = payload.wakeMode === "next-heartbeat" ? "next-heartbeat" : "now";
  const sessionKeyRaw = payload.sessionKey;
-  const idFactory = opts?.idFactory ?? randomUUID;
  const sessionKey =
-    typeof sessionKeyRaw === "string" && sessionKeyRaw.trim()
-      ? sessionKeyRaw.trim()
-      : `hook:${idFactory()}`;
+    typeof sessionKeyRaw === "string" && sessionKeyRaw.trim() ? sessionKeyRaw.trim() : undefined;
  const channel = resolveHookChannel(payload.channel);
  if (!channel) {
    return { ok: false, error: getHookChannelError() };
--- a/src/gateway/server-http.ts
+++ b/src/gateway/server-http.ts
@@ -38,6 +38,7 @@ import {
  normalizeHookHeaders,
  normalizeWakePayload,
  readJsonBody,
+  resolveHookSessionKey,
  resolveHookTargetAgentId,
  resolveHookChannel,
  resolveHookDeliver,
@@ -266,8 +267,18 @@ export function createHooksRequestHandler(
        sendJson(res, 400, { ok: false, error: getHookAgentPolicyError() });
        return true;
      }
+      const sessionKey = resolveHookSessionKey({
+        hooksConfig,
+        source: "request",
+        sessionKey: normalized.value.sessionKey,
+      });
+      if (!sessionKey.ok) {
+        sendJson(res, 400, { ok: false, error: sessionKey.error });
+        return true;
+      }
      const runId = dispatchAgentHook({
        ...normalized.value,
+        sessionKey: sessionKey.value,
        agentId: resolveHookTargetAgentId(hooksConfig, normalized.value.agentId),
      });
      sendJson(res, 202, { ok: true, runId });
@@ -309,12 +320,21 @@ export function createHooksRequestHandler(
            sendJson(res, 400, { ok: false, error: getHookAgentPolicyError() });
            return true;
          }
+          const sessionKey = resolveHookSessionKey({
+            hooksConfig,
+            source: "mapping",
+            sessionKey: mapped.action.sessionKey,
+          });
+          if (!sessionKey.ok) {
+            sendJson(res, 400, { ok: false, error: sessionKey.error });
+            return true;
+          }
          const runId = dispatchAgentHook({
            message: mapped.action.message,
            name: mapped.action.name ?? "Hook",
            agentId: resolveHookTargetAgentId(hooksConfig, mapped.action.agentId),
            wakeMode: mapped.action.wakeMode,
-            sessionKey: mapped.action.sessionKey ?? "",
+            sessionKey: sessionKey.value,
            deliver: resolveHookDeliver(mapped.action.deliver),
            channel,
            to: mapped.action.to,
--- a/src/gateway/server.hooks.e2e.test.ts
+++ b/src/gateway/server.hooks.e2e.test.ts
@@ -199,6 +199,115 @@ describe("gateway server hooks", () => {
    }
  });

+  test("rejects request sessionKey unless hooks.allowRequestSessionKey is enabled", async () => {
+    testState.hooksConfig = { enabled: true, token: "hook-secret" };
+    const port = await getFreePort();
+    const server = await startGatewayServer(port);
+    try {
+      const denied = await fetch(`http://127.0.0.1:${port}/hooks/agent`, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: "Bearer hook-secret",
+        },
+        body: JSON.stringify({
+          message: "Do it",
+          sessionKey: "agent:main:dm:u99999",
+        }),
+      });
+      expect(denied.status).toBe(400);
+      const deniedBody = (await denied.json()) as { error?: string };
+      expect(deniedBody.error).toContain("hooks.allowRequestSessionKey");
+    } finally {
+      await server.close();
+    }
+  });
+
+  test("respects hooks session policy for request + mapping session keys", async () => {
+    testState.hooksConfig = {
+      enabled: true,
+      token: "hook-secret",
+      allowRequestSessionKey: true,
+      allowedSessionKeyPrefixes: ["hook:"],
+      defaultSessionKey: "hook:ingress",
+      mappings: [
+        {
+          match: { path: "mapped-ok" },
+          action: "agent",
+          messageTemplate: "Mapped: {{payload.subject}}",
+          sessionKey: "hook:mapped:{{payload.id}}",
+        },
+        {
+          match: { path: "mapped-bad" },
+          action: "agent",
+          messageTemplate: "Mapped: {{payload.subject}}",
+          sessionKey: "agent:main:main",
+        },
+      ],
+    };
+    const port = await getFreePort();
+    const server = await startGatewayServer(port);
+    try {
+      cronIsolatedRun.mockReset();
+      cronIsolatedRun.mockResolvedValue({ status: "ok", summary: "done" });
+
+      const defaultRoute = await fetch(`http://127.0.0.1:${port}/hooks/agent`, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: "Bearer hook-secret",
+        },
+        body: JSON.stringify({ message: "No key" }),
+      });
+      expect(defaultRoute.status).toBe(202);
+      await waitForSystemEvent();
+      const defaultCall = cronIsolatedRun.mock.calls[0]?.[0] as { sessionKey?: string } | undefined;
+      expect(defaultCall?.sessionKey).toBe("hook:ingress");
+      drainSystemEvents(resolveMainKey());
+
+      cronIsolatedRun.mockReset();
+      cronIsolatedRun.mockResolvedValue({ status: "ok", summary: "done" });
+      const mappedOk = await fetch(`http://127.0.0.1:${port}/hooks/mapped-ok`, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: "Bearer hook-secret",
+        },
+        body: JSON.stringify({ subject: "hello", id: "42" }),
+      });
+      expect(mappedOk.status).toBe(202);
+      await waitForSystemEvent();
+      const mappedCall = cronIsolatedRun.mock.calls[0]?.[0] as { sessionKey?: string } | undefined;
+      expect(mappedCall?.sessionKey).toBe("hook:mapped:42");
+      drainSystemEvents(resolveMainKey());
+
+      const requestBadPrefix = await fetch(`http://127.0.0.1:${port}/hooks/agent`, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: "Bearer hook-secret",
+        },
+        body: JSON.stringify({
+          message: "Bad key",
+          sessionKey: "agent:main:main",
+        }),
+      });
+      expect(requestBadPrefix.status).toBe(400);
+
+      const mappedBadPrefix = await fetch(`http://127.0.0.1:${port}/hooks/mapped-bad`, {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json",
+          Authorization: "Bearer hook-secret",
+        },
+        body: JSON.stringify({ subject: "hello" }),
+      });
+      expect(mappedBadPrefix.status).toBe(400);
+    } finally {
+      await server.close();
+    }
+  });
+
  test("enforces hooks.allowedAgentIds for explicit agent routing", async () => {
    testState.hooksConfig = {
      enabled: true,
--- a/src/gateway/server/hooks.ts
+++ b/src/gateway/server/hooks.ts
@@ -43,7 +43,7 @@ export function createGatewayHooksRequestHandler(params: {
    timeoutSeconds?: number;
    allowUnsafeExternalContent?: boolean;
  }) => {
-    const sessionKey = value.sessionKey.trim() ? value.sessionKey.trim() : `hook:${randomUUID()}`;
+    const sessionKey = value.sessionKey.trim();
    const mainSessionKey = resolveMainSessionKeyFromConfig();
    const jobId = randomUUID();
    const now = Date.now();