refactor(security): unify hook rate-limit and hook module loading

src/gateway/auth-rate-limit.ts

@@ -31,11 +31,14 @@ export interface RateLimitConfig {
   lockoutMs?: number;
   /** Exempt loopback (localhost) addresses from rate limiting.  @default true */
   exemptLoopback?: boolean;
+  /** Background prune interval in milliseconds; set <= 0 to disable auto-prune.  @default 60_000 */
+  pruneIntervalMs?: number;
 }
 
 export const AUTH_RATE_LIMIT_SCOPE_DEFAULT = "default";
 export const AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET = "shared-secret";
 export const AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN = "device-token";
+export const AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH = "hook-auth";
 
 export interface RateLimitEntry {
   /** Timestamps (epoch ms) of recent failed attempts inside the window. */
@@ -94,13 +97,14 @@ export function createAuthRateLimiter(config?: RateLimitConfig): AuthRateLimiter
   const windowMs = config?.windowMs ?? DEFAULT_WINDOW_MS;
   const lockoutMs = config?.lockoutMs ?? DEFAULT_LOCKOUT_MS;
   const exemptLoopback = config?.exemptLoopback ?? true;
+  const pruneIntervalMs = config?.pruneIntervalMs ?? PRUNE_INTERVAL_MS;
 
   const entries = new Map<string, RateLimitEntry>();
 
   // Periodic cleanup to avoid unbounded map growth.
-  const pruneTimer = setInterval(() => prune(), PRUNE_INTERVAL_MS);
+  const pruneTimer = pruneIntervalMs > 0 ? setInterval(() => prune(), pruneIntervalMs) : null;
   // Allow the Node.js process to exit even if the timer is still active.
-  if (pruneTimer.unref) {
+  if (pruneTimer?.unref) {
     pruneTimer.unref();
   }
 
@@ -218,7 +222,9 @@ export function createAuthRateLimiter(config?: RateLimitConfig): AuthRateLimiter
   }
 
   function dispose(): void {
-    clearInterval(pruneTimer);
+    if (pruneTimer) {
+      clearInterval(pruneTimer);
+    }
     entries.clear();
   }
 

src/gateway/hooks-mapping.ts

@@ -1,6 +1,6 @@
 import path from "node:path";
-import { pathToFileURL } from "node:url";
 import { CONFIG_PATH, type HookMappingConfig, type HooksConfig } from "../config/config.js";
+import { importFileModule, resolveFunctionModuleExport } from "../hooks/module-loader.js";
 import type { HookMessageChannel } from "./hooks.js";
 
 export type HookMappingResolved = {
@@ -330,19 +330,22 @@ async function loadTransform(transform: HookMappingTransformResolved): Promise<H
   if (cached) {
     return cached;
   }
-  const url = pathToFileURL(transform.modulePath).href;
-  const mod = (await import(url)) as Record<string, unknown>;
+  const mod = await importFileModule({ modulePath: transform.modulePath });
   const fn = resolveTransformFn(mod, transform.exportName);
   transformCache.set(cacheKey, fn);
   return fn;
 }
 
 function resolveTransformFn(mod: Record<string, unknown>, exportName?: string): HookTransformFn {
-  const candidate = exportName ? mod[exportName] : (mod.default ?? mod.transform);
-  if (typeof candidate !== "function") {
+  const candidate = resolveFunctionModuleExport<HookTransformFn>({
+    mod,
+    exportName,
+    fallbackExportNames: ["default", "transform"],
+  });
+  if (!candidate) {
     throw new Error("hook transform module must export a function");
   }
-  return candidate as HookTransformFn;
+  return candidate;
 }
 
 function resolvePath(baseDir: string, target: string): string {

src/gateway/server-http.ts

@@ -19,7 +19,12 @@ import { loadConfig } from "../config/config.js";
 import type { createSubsystemLogger } from "../logging/subsystem.js";
 import { safeEqualSecret } from "../security/secret-equal.js";
 import { handleSlackHttpRequest } from "../slack/http/index.js";
-import { normalizeRateLimitClientIp, type AuthRateLimiter } from "./auth-rate-limit.js";
+import {
+  AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH,
+  createAuthRateLimiter,
+  normalizeRateLimitClientIp,
+  type AuthRateLimiter,
+} from "./auth-rate-limit.js";
 import {
   authorizeHttpGatewayConnect,
   isLocalDirectRequest,
@@ -58,11 +63,9 @@ import type { GatewayWsClient } from "./server/ws-types.js";
 import { handleToolsInvokeHttpRequest } from "./tools-invoke-http.js";
 
 type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;
-type HookAuthFailure = { count: number; windowStartedAtMs: number };
 
 const HOOK_AUTH_FAILURE_LIMIT = 20;
 const HOOK_AUTH_FAILURE_WINDOW_MS = 60_000;
-const HOOK_AUTH_FAILURE_TRACK_MAX = 2048;
 
 type HookDispatchers = {
   dispatchWakeHook: (value: { text: string; mode: "now" | "next-heartbeat" }) => void;
@@ -219,60 +222,19 @@ export function createHooksRequestHandler(
   } & HookDispatchers,
 ): HooksRequestHandler {
   const { getHooksConfig, bindHost, port, logHooks, dispatchAgentHook, dispatchWakeHook } = opts;
-  const hookAuthFailures = new Map<string, HookAuthFailure>();
+  const hookAuthLimiter = createAuthRateLimiter({
+    maxAttempts: HOOK_AUTH_FAILURE_LIMIT,
+    windowMs: HOOK_AUTH_FAILURE_WINDOW_MS,
+    lockoutMs: HOOK_AUTH_FAILURE_WINDOW_MS,
+    exemptLoopback: false,
+    // Handler lifetimes are tied to gateway runtime/tests; skip background timer fanout.
+    pruneIntervalMs: 0,
+  });
 
   const resolveHookClientKey = (req: IncomingMessage): string => {
     return normalizeRateLimitClientIp(req.socket?.remoteAddress);
   };
 
-  const recordHookAuthFailure = (
-    clientKey: string,
-    nowMs: number,
-  ): { throttled: boolean; retryAfterSeconds?: number } => {
-    if (!hookAuthFailures.has(clientKey) && hookAuthFailures.size >= HOOK_AUTH_FAILURE_TRACK_MAX) {
-      // Prune expired entries instead of clearing all state.
-      for (const [key, entry] of hookAuthFailures) {
-        if (nowMs - entry.windowStartedAtMs >= HOOK_AUTH_FAILURE_WINDOW_MS) {
-          hookAuthFailures.delete(key);
-        }
-      }
-      // If still at capacity after pruning, drop the oldest half.
-      if (hookAuthFailures.size >= HOOK_AUTH_FAILURE_TRACK_MAX) {
-        let toRemove = Math.floor(hookAuthFailures.size / 2);
-        for (const key of hookAuthFailures.keys()) {
-          if (toRemove <= 0) {
-            break;
-          }
-          hookAuthFailures.delete(key);
-          toRemove--;
-        }
-      }
-    }
-    const current = hookAuthFailures.get(clientKey);
-    const expired = !current || nowMs - current.windowStartedAtMs >= HOOK_AUTH_FAILURE_WINDOW_MS;
-    const next: HookAuthFailure = expired
-      ? { count: 1, windowStartedAtMs: nowMs }
-      : { count: current.count + 1, windowStartedAtMs: current.windowStartedAtMs };
-    // Delete-before-set refreshes Map insertion order so recently-active
-    // clients are not evicted before dormant ones during oldest-half eviction.
-    if (hookAuthFailures.has(clientKey)) {
-      hookAuthFailures.delete(clientKey);
-    }
-    hookAuthFailures.set(clientKey, next);
-    if (next.count <= HOOK_AUTH_FAILURE_LIMIT) {
-      return { throttled: false };
-    }
-    const retryAfterMs = Math.max(1, next.windowStartedAtMs + HOOK_AUTH_FAILURE_WINDOW_MS - nowMs);
-    return {
-      throttled: true,
-      retryAfterSeconds: Math.ceil(retryAfterMs / 1000),
-    };
-  };
-
-  const clearHookAuthFailure = (clientKey: string) => {
-    hookAuthFailures.delete(clientKey);
-  };
-
   return async (req, res) => {
     const hooksConfig = getHooksConfig();
     if (!hooksConfig) {
@@ -296,9 +258,9 @@ export function createHooksRequestHandler(
     const token = extractHookToken(req);
     const clientKey = resolveHookClientKey(req);
     if (!safeEqualSecret(token, hooksConfig.token)) {
-      const throttle = recordHookAuthFailure(clientKey, Date.now());
-      if (throttle.throttled) {
-        const retryAfter = throttle.retryAfterSeconds ?? 1;
+      const throttle = hookAuthLimiter.check(clientKey, AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH);
+      if (!throttle.allowed) {
+        const retryAfter = throttle.retryAfterMs > 0 ? Math.ceil(throttle.retryAfterMs / 1000) : 1;
         res.statusCode = 429;
         res.setHeader("Retry-After", String(retryAfter));
         res.setHeader("Content-Type", "text/plain; charset=utf-8");
@@ -306,12 +268,13 @@ export function createHooksRequestHandler(
         logHooks.warn(`hook auth throttled for ${clientKey}; retry-after=${retryAfter}s`);
         return true;
       }
+      hookAuthLimiter.recordFailure(clientKey, AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH);
       res.statusCode = 401;
       res.setHeader("Content-Type", "text/plain; charset=utf-8");
       res.end("Unauthorized");
       return true;
     }
-    clearHookAuthFailure(clientKey);
+    hookAuthLimiter.reset(clientKey, AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH);
 
     if (req.method !== "POST") {
       res.statusCode = 405;

src/hooks/loader.ts

@@ -6,7 +6,6 @@
  */
 
 import path from "node:path";
-import { pathToFileURL } from "node:url";
 import type { OpenClawConfig } from "../config/config.js";
 import { createSubsystemLogger } from "../logging/subsystem.js";
 import { isPathInsideWithRealpath } from "../security/scan-paths.js";
@@ -14,6 +13,7 @@ import { resolveHookConfig } from "./config.js";
 import { shouldIncludeHook } from "./config.js";
 import type { InternalHookHandler } from "./internal-hooks.js";
 import { registerInternalHook } from "./internal-hooks.js";
+import { importFileModule, resolveFunctionModuleExport } from "./module-loader.js";
 import { loadWorkspaceHookEntries } from "./workspace.js";
 
 const log = createSubsystemLogger("hooks:loader");
@@ -82,16 +82,18 @@ export async function loadInternalHooks(
           );
           continue;
         }
-        // Import handler module with cache-busting
-        const url = pathToFileURL(entry.hook.handlerPath).href;
-        const cacheBustedUrl = `${url}?t=${Date.now()}`;
-        const mod = (await import(cacheBustedUrl)) as Record<string, unknown>;
-
         // Get handler function (default or named export)
         const exportName = entry.metadata?.export ?? "default";
-        const handler = mod[exportName];
+        const mod = await importFileModule({
+          modulePath: entry.hook.handlerPath,
+          cacheBust: true,
+        });
+        const handler = resolveFunctionModuleExport<InternalHookHandler>({
+          mod,
+          exportName,
+        });
 
-        if (typeof handler !== "function") {
+        if (!handler) {
           log.error(`Handler '${exportName}' from ${entry.hook.name} is not a function`);
           continue;
         }
@@ -104,7 +106,7 @@ export async function loadInternalHooks(
         }
 
         for (const event of events) {
-          registerInternalHook(event, handler as InternalHookHandler);
+          registerInternalHook(event, handler);
         }
 
         log.info(
@@ -157,21 +159,23 @@ export async function loadInternalHooks(
         continue;
       }
 
-      // Import the module with cache-busting to ensure fresh reload
-      const url = pathToFileURL(modulePath).href;
-      const cacheBustedUrl = `${url}?t=${Date.now()}`;
-      const mod = (await import(cacheBustedUrl)) as Record<string, unknown>;
-
       // Get the handler function
       const exportName = handlerConfig.export ?? "default";
-      const handler = mod[exportName];
+      const mod = await importFileModule({
+        modulePath,
+        cacheBust: true,
+      });
+      const handler = resolveFunctionModuleExport<InternalHookHandler>({
+        mod,
+        exportName,
+      });
 
-      if (typeof handler !== "function") {
+      if (!handler) {
         log.error(`Handler '${exportName}' from ${modulePath} is not a function`);
         continue;
       }
 
-      registerInternalHook(handlerConfig.event, handler as InternalHookHandler);
+      registerInternalHook(handlerConfig.event, handler);
       log.info(
         `Registered hook (legacy): ${handlerConfig.event} -> ${modulePath}${exportName !== "default" ? `#${exportName}` : ""}`,
       );

src/hooks/module-loader.test.ts

@@ -0,0 +1,48 @@
+import path from "node:path";
+import { pathToFileURL } from "node:url";
+import { describe, expect, it } from "vitest";
+import { resolveFileModuleUrl, resolveFunctionModuleExport } from "./module-loader.js";
+
+describe("hooks module loader helpers", () => {
+  it("builds a file URL without cache-busting by default", () => {
+    const modulePath = path.resolve("/tmp/hook-handler.js");
+    expect(resolveFileModuleUrl({ modulePath })).toBe(pathToFileURL(modulePath).href);
+  });
+
+  it("adds a cache-busting query when requested", () => {
+    const modulePath = path.resolve("/tmp/hook-handler.js");
+    expect(
+      resolveFileModuleUrl({
+        modulePath,
+        cacheBust: true,
+        nowMs: 123,
+      }),
+    ).toBe(`${pathToFileURL(modulePath).href}?t=123`);
+  });
+
+  it("resolves explicit function exports", () => {
+    const fn = () => "ok";
+    const resolved = resolveFunctionModuleExport({
+      mod: { run: fn },
+      exportName: "run",
+    });
+    expect(resolved).toBe(fn);
+  });
+
+  it("falls back through named exports when no explicit export is provided", () => {
+    const fallback = () => "ok";
+    const resolved = resolveFunctionModuleExport({
+      mod: { transform: fallback },
+      fallbackExportNames: ["default", "transform"],
+    });
+    expect(resolved).toBe(fallback);
+  });
+
+  it("returns undefined when export exists but is not callable", () => {
+    const resolved = resolveFunctionModuleExport({
+      mod: { run: "nope" },
+      exportName: "run",
+    });
+    expect(resolved).toBeUndefined();
+  });
+});

src/hooks/module-loader.ts

@@ -0,0 +1,46 @@
+import { pathToFileURL } from "node:url";
+
+type ModuleNamespace = Record<string, unknown>;
+type GenericFunction = (...args: never[]) => unknown;
+
+export function resolveFileModuleUrl(params: {
+  modulePath: string;
+  cacheBust?: boolean;
+  nowMs?: number;
+}): string {
+  const url = pathToFileURL(params.modulePath).href;
+  if (!params.cacheBust) {
+    return url;
+  }
+  const ts = params.nowMs ?? Date.now();
+  return `${url}?t=${ts}`;
+}
+
+export async function importFileModule(params: {
+  modulePath: string;
+  cacheBust?: boolean;
+  nowMs?: number;
+}): Promise<ModuleNamespace> {
+  const specifier = resolveFileModuleUrl(params);
+  return (await import(specifier)) as ModuleNamespace;
+}
+
+export function resolveFunctionModuleExport<T extends GenericFunction>(params: {
+  mod: ModuleNamespace;
+  exportName?: string;
+  fallbackExportNames?: string[];
+}): T | undefined {
+  const explicitExport = params.exportName?.trim();
+  if (explicitExport) {
+    const candidate = params.mod[explicitExport];
+    return typeof candidate === "function" ? (candidate as T) : undefined;
+  }
+  const fallbacks = params.fallbackExportNames ?? ["default"];
+  for (const exportName of fallbacks) {
+    const candidate = params.mod[exportName];
+    if (typeof candidate === "function") {
+      return candidate as T;
+    }
+  }
+  return undefined;
+}