fix(plugins): enforce synchronous registration

2026-05-06 21:10:43 +00:00 · 2026-04-17 08:34:48 +05:30
parent 15b2827fc1
commit 2a283e87a7
17 changed files with 411 additions and 269 deletions
--- a/extensions/webhooks/index.test.ts
+++ b/extensions/webhooks/index.test.ts
@@ -0,0 +1,66 @@
+import { describe, expect, it, vi } from "vitest";
+import { createTestPluginApi } from "../../test/helpers/plugins/plugin-api.js";
+import type { OpenClawPluginApi } from "./api.js";
+import plugin from "./index.js";
+
+function createApi(params?: {
+  pluginConfig?: OpenClawPluginApi["pluginConfig"];
+  registerHttpRoute?: OpenClawPluginApi["registerHttpRoute"];
+  logger?: OpenClawPluginApi["logger"];
+}): OpenClawPluginApi {
+  return createTestPluginApi({
+    id: "webhooks",
+    name: "Webhooks",
+    source: "test",
+    pluginConfig: params?.pluginConfig ?? {},
+    runtime: {
+      taskFlow: {
+        bindSession: vi.fn(({ sessionKey }: { sessionKey: string }) => ({ sessionKey })),
+      },
+    } as unknown as OpenClawPluginApi["runtime"],
+    registerHttpRoute: params?.registerHttpRoute ?? vi.fn(),
+    logger:
+      params?.logger ??
+      ({
+        info: vi.fn(),
+        warn: vi.fn(),
+        error: vi.fn(),
+        debug: vi.fn(),
+      } as OpenClawPluginApi["logger"]),
+  });
+}
+
+describe("webhooks plugin registration", () => {
+  it("registers SecretRef-backed routes synchronously", () => {
+    const registerHttpRoute = vi.fn();
+
+    const result = plugin.register(
+      createApi({
+        pluginConfig: {
+          routes: {
+            zapier: {
+              sessionKey: "agent:main:main",
+              secret: {
+                source: "env",
+                provider: "default",
+                id: "OPENCLAW_WEBHOOK_SECRET",
+              },
+            },
+          },
+        },
+        registerHttpRoute,
+      }),
+    );
+
+    expect(result).toBeUndefined();
+    expect(registerHttpRoute).toHaveBeenCalledTimes(1);
+    expect(registerHttpRoute).toHaveBeenCalledWith(
+      expect.objectContaining({
+        path: "/plugins/webhooks/zapier",
+        auth: "plugin",
+        match: "exact",
+        replaceExisting: true,
+      }),
+    );
+  });
+});
--- a/extensions/webhooks/index.ts
+++ b/extensions/webhooks/index.ts
@@ -2,50 +2,52 @@ import { definePluginEntry, type OpenClawPluginApi } from "./api.js";
 import { resolveWebhooksPluginConfig } from "./src/config.js";
 import { createTaskFlowWebhookRequestHandler, type TaskFlowWebhookTarget } from "./src/http.js";

+function registerWebhookRoutes(api: OpenClawPluginApi): void {
+  const routes = resolveWebhooksPluginConfig({
+    pluginConfig: api.pluginConfig,
+  });
+  if (routes.length === 0) {
+    return;
+  }
+
+  const targetsByPath = new Map<string, TaskFlowWebhookTarget[]>();
+  const handler = createTaskFlowWebhookRequestHandler({
+    cfg: api.config,
+    targetsByPath,
+  });
+
+  for (const route of routes) {
+    const taskFlow = api.runtime.taskFlow.bindSession({
+      sessionKey: route.sessionKey,
+    });
+    const target: TaskFlowWebhookTarget = {
+      routeId: route.routeId,
+      path: route.path,
+      secretInput: route.secret,
+      secretConfigPath: `plugins.entries.webhooks.routes.${route.routeId}.secret`,
+      defaultControllerId: route.controllerId,
+      taskFlow,
+    };
+    targetsByPath.set(target.path, [...(targetsByPath.get(target.path) ?? []), target]);
+    api.registerHttpRoute({
+      path: target.path,
+      auth: "plugin",
+      match: "exact",
+      replaceExisting: true,
+      handler,
+    });
+    api.logger.info?.(
+      `[webhooks] registered route ${route.routeId} on ${route.path} for session ${route.sessionKey}`,
+    );
+  }
+}
+
 export default definePluginEntry({
  id: "webhooks",
  name: "Webhooks",
  description:
    "Authenticated inbound webhooks that bind external automation to OpenClaw TaskFlows.",
-  async register(api: OpenClawPluginApi) {
-    const routes = await resolveWebhooksPluginConfig({
-      pluginConfig: api.pluginConfig,
-      cfg: api.config,
-      env: process.env,
-      logger: api.logger,
-    });
-    if (routes.length === 0) {
-      return;
-    }
-
-    const targetsByPath = new Map<string, TaskFlowWebhookTarget[]>();
-    const handler = createTaskFlowWebhookRequestHandler({
-      cfg: api.config,
-      targetsByPath,
-    });
-
-    for (const route of routes) {
-      const taskFlow = api.runtime.taskFlow.bindSession({
-        sessionKey: route.sessionKey,
-      });
-      const target: TaskFlowWebhookTarget = {
-        routeId: route.routeId,
-        path: route.path,
-        secret: route.secret,
-        defaultControllerId: route.controllerId,
-        taskFlow,
-      };
-      targetsByPath.set(target.path, [...(targetsByPath.get(target.path) ?? []), target]);
-      api.registerHttpRoute({
-        path: target.path,
-        auth: "plugin",
-        match: "exact",
-        replaceExisting: true,
-        handler,
-      });
-      api.logger.info?.(
-        `[webhooks] registered route ${route.routeId} on ${route.path} for session ${route.sessionKey}`,
-      );
-    }
+  register(api: OpenClawPluginApi) {
+    registerWebhookRoutes(api);
  },
 });
--- a/extensions/webhooks/runtime-api.ts
+++ b/extensions/webhooks/runtime-api.ts
@@ -4,6 +4,7 @@ export {
  normalizeWebhookPath,
  readJsonWebhookBodyOrReject,
  resolveRequestClientIp,
+  resolveWebhookTargetWithAuthOrReject,
  resolveWebhookTargetWithAuthOrRejectSync,
  withResolvedWebhookRequestPipeline,
  WEBHOOK_IN_FLIGHT_DEFAULTS,
--- a/extensions/webhooks/src/config.test.ts
+++ b/extensions/webhooks/src/config.test.ts
@@ -1,10 +1,9 @@
-import { describe, expect, it, vi } from "vitest";
-import type { OpenClawConfig } from "../runtime-api.js";
+import { describe, expect, it } from "vitest";
 import { resolveWebhooksPluginConfig } from "./config.js";

 describe("resolveWebhooksPluginConfig", () => {
-  it("resolves default paths and SecretRef-backed secrets", async () => {
-    const routes = await resolveWebhooksPluginConfig({
+  it("keeps SecretRef-backed secrets on the route config", () => {
+    const routes = resolveWebhooksPluginConfig({
      pluginConfig: {
        routes: {
          zapier: {
@@ -17,10 +16,6 @@ describe("resolveWebhooksPluginConfig", () => {
          },
        },
      },
-      cfg: {} as OpenClawConfig,
-      env: {
-        OPENCLAW_WEBHOOK_SECRET: "shared-secret",
-      },
    });

    expect(routes).toEqual([
@@ -28,16 +23,18 @@ describe("resolveWebhooksPluginConfig", () => {
        routeId: "zapier",
        path: "/plugins/webhooks/zapier",
        sessionKey: "agent:main:main",
-        secret: "shared-secret",
+        secret: {
+          source: "env",
+          provider: "default",
+          id: "OPENCLAW_WEBHOOK_SECRET",
+        },
        controllerId: "webhooks/zapier",
      },
    ]);
  });

-  it("skips routes whose secret cannot be resolved", async () => {
-    const warn = vi.fn();
-
-    const routes = await resolveWebhooksPluginConfig({
+  it("keeps routes whose secret needs runtime resolution", () => {
+    const routes = resolveWebhooksPluginConfig({
      pluginConfig: {
        routes: {
          missing: {
@@ -50,19 +47,25 @@ describe("resolveWebhooksPluginConfig", () => {
          },
        },
      },
-      cfg: {} as OpenClawConfig,
-      env: {},
-      logger: { warn } as never,
    });

-    expect(routes).toEqual([]);
-    expect(warn).toHaveBeenCalledWith(
-      expect.stringContaining("[webhooks] skipping route missing:"),
-    );
+    expect(routes).toEqual([
+      {
+        routeId: "missing",
+        path: "/plugins/webhooks/missing",
+        sessionKey: "agent:main:main",
+        secret: {
+          source: "env",
+          provider: "default",
+          id: "MISSING_SECRET",
+        },
+        controllerId: "webhooks/missing",
+      },
+    ]);
  });

-  it("rejects duplicate normalized paths", async () => {
-    await expect(
+  it("rejects duplicate normalized paths", () => {
+    expect(() =>
      resolveWebhooksPluginConfig({
        pluginConfig: {
          routes: {
@@ -78,9 +81,7 @@ describe("resolveWebhooksPluginConfig", () => {
            },
          },
        },
-        cfg: {} as OpenClawConfig,
-        env: {},
      }),
-    ).rejects.toThrow(/conflicts with routes\.first\.path/i);
+    ).toThrow(/conflicts with routes\.first\.path/i);
  });
 });
--- a/extensions/webhooks/src/config.ts
+++ b/extensions/webhooks/src/config.ts
@@ -1,10 +1,5 @@
 import { z } from "zod";
-import type { PluginLogger } from "../api.js";
-import {
-  normalizeWebhookPath,
-  resolveConfiguredSecretInputString,
-  type OpenClawConfig,
-} from "../runtime-api.js";
+import { normalizeWebhookPath } from "../runtime-api.js";

 const secretRefSchema = z
  .object({
@@ -33,23 +28,22 @@ const webhooksPluginConfigSchema = z
  })
  .strict();

-export type ResolvedWebhookRouteConfig = {
+export type WebhookSecretInput = z.infer<typeof secretInputSchema>;
+
+export type ConfiguredWebhookRouteConfig = {
  routeId: string;
  path: string;
  sessionKey: string;
-  secret: string;
+  secret: WebhookSecretInput;
  controllerId: string;
  description?: string;
 };

-export async function resolveWebhooksPluginConfig(params: {
+export function resolveWebhooksPluginConfig(params: {
  pluginConfig: unknown;
-  cfg: OpenClawConfig;
-  env: NodeJS.ProcessEnv;
-  logger?: PluginLogger;
-}): Promise<ResolvedWebhookRouteConfig[]> {
+}): ConfiguredWebhookRouteConfig[] {
  const parsed = webhooksPluginConfigSchema.parse(params.pluginConfig ?? {});
-  const resolvedRoutes: ResolvedWebhookRouteConfig[] = [];
+  const configuredRoutes: ConfiguredWebhookRouteConfig[] = [];
  const seenPaths = new Map<string, string>();

  for (const [routeId, route] of Object.entries(parsed.routes)) {
@@ -64,32 +58,16 @@ export async function resolveWebhooksPluginConfig(params: {
      );
    }

-    const secretResolution = await resolveConfiguredSecretInputString({
-      config: params.cfg,
-      env: params.env,
-      value: route.secret,
-      path: `plugins.entries.webhooks.routes.${routeId}.secret`,
-    });
-    const secret = secretResolution.value?.trim();
-    if (!secret) {
-      params.logger?.warn?.(
-        `[webhooks] skipping route ${routeId}: ${
-          secretResolution.unresolvedRefReason ?? "secret is empty or unresolved"
-        }`,
-      );
-      continue;
-    }
-
    seenPaths.set(path, routeId);
-    resolvedRoutes.push({
+    configuredRoutes.push({
      routeId,
      path,
      sessionKey: route.sessionKey,
-      secret,
+      secret: route.secret,
      controllerId: route.controllerId ?? `webhooks/${routeId}`,
      ...(route.description ? { description: route.description } : {}),
    });
  }

-  return resolvedRoutes;
+  return configuredRoutes;
 }
--- a/extensions/webhooks/src/http.test.ts
+++ b/extensions/webhooks/src/http.test.ts
@@ -69,13 +69,16 @@ function createJsonRequest(params: {
 function createHandler(): {
  handler: ReturnType<typeof createTaskFlowWebhookRequestHandler>;
  target: TaskFlowWebhookTarget;
+  secret: string;
 } {
  const runtime = createRuntimeTaskFlow();
  nextSessionId += 1;
+  const secret = "shared-secret";
  const target: TaskFlowWebhookTarget = {
    routeId: "zapier",
    path: "/plugins/webhooks/zapier",
-    secret: "shared-secret",
+    secretInput: secret,
+    secretConfigPath: "plugins.entries.webhooks.routes.zapier.secret",
    defaultControllerId: "webhooks/zapier",
    taskFlow: runtime.bindSession({
      sessionKey: `agent:main:webhook-test-${String(nextSessionId)}`,
@@ -88,6 +91,7 @@ function createHandler(): {
      targetsByPath,
    }),
    target,
+    secret,
  };
 }

@@ -133,11 +137,11 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("creates flows through the bound session and scrubs owner metadata from responses", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const res = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "create_flow",
        goal: "Review inbound queue",
@@ -158,7 +162,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("runs child tasks and scrubs task ownership fields from responses", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const flow = target.taskFlow.createManaged({
      controllerId: "webhooks/zapier",
      goal: "Triage inbox",
@@ -166,7 +170,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const res = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "run_task",
        flowId: flow.flowId,
@@ -193,11 +197,11 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("returns 404 for missing flow mutations", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const res = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "set_waiting",
        flowId: "flow-missing",
@@ -219,7 +223,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("returns 409 for revision conflicts", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const flow = target.taskFlow.createManaged({
      controllerId: "webhooks/zapier",
      goal: "Review inbox",
@@ -227,7 +231,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const res = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "set_waiting",
        flowId: flow.flowId,
@@ -252,7 +256,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("rejects internal runtimes and running-only metadata from external callers", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const flow = target.taskFlow.createManaged({
      controllerId: "webhooks/zapier",
      goal: "Review inbox",
@@ -261,7 +265,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const runtimeRes = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "run_task",
        flowId: flow.flowId,
@@ -278,7 +282,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const queuedMetadataRes = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "run_task",
        flowId: flow.flowId,
@@ -297,7 +301,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("reuses the same task record when retried with the same runId", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const flow = target.taskFlow.createManaged({
      controllerId: "webhooks/zapier",
      goal: "Triage inbox",
@@ -306,7 +310,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const first = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "run_task",
        flowId: flow.flowId,
@@ -319,7 +323,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const second = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "run_task",
        flowId: flow.flowId,
@@ -339,7 +343,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
  });

  it("returns 409 when cancellation targets a terminal flow", async () => {
-    const { handler, target } = createHandler();
+    const { handler, target, secret } = createHandler();
    const flow = target.taskFlow.createManaged({
      controllerId: "webhooks/zapier",
      goal: "Review inbox",
@@ -353,7 +357,7 @@ describe("createTaskFlowWebhookRequestHandler", () => {
    const res = await dispatchJsonRequest({
      handler,
      path: target.path,
-      secret: target.secret,
+      secret,
      body: {
        action: "cancel_flow",
        flowId: flow.flowId,
--- a/extensions/webhooks/src/http.ts
+++ b/extensions/webhooks/src/http.ts
@@ -8,13 +8,15 @@ import {
  createWebhookInFlightLimiter,
  readJsonWebhookBodyOrReject,
  resolveRequestClientIp,
-  resolveWebhookTargetWithAuthOrRejectSync,
+  resolveConfiguredSecretInputString,
+  resolveWebhookTargetWithAuthOrReject,
  withResolvedWebhookRequestPipeline,
  WEBHOOK_IN_FLIGHT_DEFAULTS,
  WEBHOOK_RATE_LIMIT_DEFAULTS,
  type OpenClawConfig,
  type WebhookInFlightLimiter,
 } from "../runtime-api.js";
+import type { WebhookSecretInput } from "./config.js";

 type BoundTaskFlowRuntime = ReturnType<PluginRuntime["taskFlow"]["bindSession"]>;

@@ -174,7 +176,8 @@ type WebhookAction = z.infer<typeof webhookActionSchema>;
 export type TaskFlowWebhookTarget = {
  routeId: string;
  path: string;
-  secret: string;
+  secretInput: WebhookSecretInput;
+  secretConfigPath: string;
  defaultControllerId: string;
  taskFlow: BoundTaskFlowRuntime;
 };
@@ -698,11 +701,23 @@ export function createTaskFlowWebhookRequestHandler(params: {
      inFlightLimiter,
      handle: async ({ targets }) => {
        const presentedSecret = extractSharedSecret(req);
-        const target = resolveWebhookTargetWithAuthOrRejectSync({
+        const target = await resolveWebhookTargetWithAuthOrReject({
          targets,
          res,
-          isMatch: (candidate) =>
-            presentedSecret.length > 0 && timingSafeEquals(candidate.secret, presentedSecret),
+          isMatch: async (candidate) => {
+            if (presentedSecret.length === 0) {
+              return false;
+            }
+            const resolvedSecret = await resolveConfiguredSecretInputString({
+              config: params.cfg,
+              env: process.env,
+              value: candidate.secretInput,
+              path: candidate.secretConfigPath,
+            });
+            return Boolean(
+              resolvedSecret.value && timingSafeEquals(resolvedSecret.value, presentedSecret),
+            );
+          },
        });
        if (!target) {
          return true;