fix(plugins): add timeout protection to before_agent_start hook

Add configurable timeout (default 30s) to plugin lifecycle hooks to prevent
a single misbehaving plugin from permanently blocking all agent runs.

- Add hookTimeoutMs config option under plugins section
- Wrap hook execution in Promise.race with timeout
- Log warning with plugin name when hook exceeds timeout
- Agent continues gracefully after timeout instead of hanging

Fixes #48534
This commit is contained in:
Jerry-Xin 2026-03-17 13:02:45 +08:00
parent 527a1919ea
commit f7681e6fe9
7 changed files with 330 additions and 5 deletions

View File

@ -957,6 +957,8 @@ export const FIELD_HELP: Record<string, string> = {
"Plugin system controls for enabling extensions, constraining load scope, configuring entries, and tracking installs. Keep plugin policy explicit and least-privilege in production environments.",
"plugins.enabled":
"Enable or disable plugin/extension loading globally during startup and config reload (default: true). Keep enabled only when extension capabilities are required by your deployment.",
"plugins.hookTimeoutMs":
"Maximum execution time in milliseconds for each async plugin hook handler (default: 10000). If a handler exceeds this limit it is skipped with a warning and the run proceeds. Set to 0 to disable the timeout.",
"plugins.allow":
"Optional allowlist of plugin IDs; when set, only listed plugins are eligible to load. Use this to enforce approved extension inventories in controlled environments.",
"plugins.deny":

View File

@ -852,6 +852,7 @@ export const FIELD_LABELS: Record<string, string> = {
"discovery.mdns.mode": "mDNS Discovery Mode",
plugins: "Plugins",
"plugins.enabled": "Enable Plugins",
"plugins.hookTimeoutMs": "Plugin Hook Timeout (ms)",
"plugins.allow": "Plugin Allowlist",
"plugins.deny": "Plugin Denylist",
"plugins.load": "Plugin Loader",

View File

@ -29,6 +29,8 @@ export type PluginInstallRecord = Omit<InstallRecordBase, "source"> & {
export type PluginsConfig = {
/** Enable or disable plugin loading. */
enabled?: boolean;
/** Per-handler timeout for async plugin hooks (ms). Default 10 000. Set 0 to disable. */
hookTimeoutMs?: number;
/** Optional plugin allowlist (plugin ids). */
allow?: string[];
/** Optional plugin denylist (plugin ids). */

View File

@ -907,6 +907,7 @@ export const OpenClawSchema = z
plugins: z
.object({
enabled: z.boolean().optional(),
hookTimeoutMs: z.number().int().min(0).optional(),
allow: z.array(z.string()).optional(),
deny: z.array(z.string()).optional(),
load: z

View File

@ -33,7 +33,10 @@ function getHookRunnerGlobalState(): HookRunnerGlobalState {
* Initialize the global hook runner with a plugin registry.
* Called once when plugins are loaded during gateway startup.
*/
export function initializeGlobalHookRunner(registry: PluginRegistry): void {
export function initializeGlobalHookRunner(
registry: PluginRegistry,
options?: { hookTimeoutMs?: number },
): void {
const state = getHookRunnerGlobalState();
state.registry = registry;
state.hookRunner = createHookRunner(registry, {
@ -43,6 +46,7 @@ export function initializeGlobalHookRunner(registry: PluginRegistry): void {
error: (msg) => log.error(msg),
},
catchErrors: true,
hookTimeoutMs: options?.hookTimeoutMs,
});
const hookCount = registry.hooks.length;

View File

@ -0,0 +1,307 @@
/**
* Tests for per-handler hook timeout protection.
*
* Validates that a hanging plugin hook handler is terminated after the
* configured timeout so it cannot permanently block agent runs.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createHookRunner } from "./hooks.js";
import { addTestHook, TEST_PLUGIN_AGENT_CTX } from "./hooks.test-helpers.js";
import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js";
import type { PluginHookRegistration } from "./types.js";
const stubCtx = TEST_PLUGIN_AGENT_CTX;
describe("hook handler timeout", () => {
let registry: PluginRegistry;
beforeEach(() => {
registry = createEmptyPluginRegistry();
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
// -----------------------------------------------------------------------
// before_agent_start (modifying hook, sequential)
// -----------------------------------------------------------------------
it("completes normally when handler resolves within timeout", async () => {
addTestHook({
registry,
pluginId: "fast-plugin",
hookName: "before_agent_start",
handler: (() =>
Promise.resolve({ prependContext: "fast result" })) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, { hookTimeoutMs: 5000, catchErrors: true });
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(0);
const result = await promise;
expect(result?.prependContext).toBe("fast result");
});
it("skips a hanging before_agent_start handler after timeout and proceeds", async () => {
const warnings: string[] = [];
// This handler never resolves
addTestHook({
registry,
pluginId: "hanging-plugin",
hookName: "before_agent_start",
handler: (() => new Promise(() => {})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, {
hookTimeoutMs: 100,
catchErrors: true,
logger: {
debug: () => {},
warn: (msg) => warnings.push(msg),
error: (msg) => warnings.push(msg),
},
});
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(150);
const result = await promise;
// Handler timed out so no result is returned
expect(result).toBeUndefined();
// A warning/error should have been logged with the plugin name
expect(warnings.some((w) => w.includes("hanging-plugin"))).toBe(true);
expect(warnings.some((w) => w.includes("timed out"))).toBe(true);
});
it("other hooks still execute when one hangs", async () => {
const warnings: string[] = [];
// First handler (higher priority) returns normally
addTestHook({
registry,
pluginId: "good-plugin",
hookName: "before_agent_start",
handler: (() =>
Promise.resolve({
prependContext: "good context",
})) as PluginHookRegistration["handler"],
priority: 10,
});
// Second handler (lower priority) hangs
addTestHook({
registry,
pluginId: "bad-plugin",
hookName: "before_agent_start",
handler: (() => new Promise(() => {})) as PluginHookRegistration["handler"],
priority: 1,
});
const runner = createHookRunner(registry, {
hookTimeoutMs: 100,
catchErrors: true,
logger: {
debug: () => {},
warn: (msg) => warnings.push(msg),
error: (msg) => warnings.push(msg),
},
});
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(150);
const result = await promise;
// The good plugin's result should be preserved
expect(result?.prependContext).toBe("good context");
// The bad plugin should have been logged
expect(warnings.some((w) => w.includes("bad-plugin"))).toBe(true);
});
// -----------------------------------------------------------------------
// Void hooks (parallel execution)
// -----------------------------------------------------------------------
it("skips a hanging void hook handler after timeout", async () => {
const warnings: string[] = [];
let fastHandlerRan = false;
addTestHook({
registry,
pluginId: "fast-void-plugin",
hookName: "agent_end",
handler: (() => {
fastHandlerRan = true;
return Promise.resolve();
}) as PluginHookRegistration["handler"],
});
addTestHook({
registry,
pluginId: "hanging-void-plugin",
hookName: "agent_end",
handler: (() => new Promise(() => {})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, {
hookTimeoutMs: 100,
catchErrors: true,
logger: {
debug: () => {},
warn: (msg) => warnings.push(msg),
error: (msg) => warnings.push(msg),
},
});
const promise = runner.runAgentEnd({ messages: [], success: true }, stubCtx);
await vi.advanceTimersByTimeAsync(150);
await promise;
expect(fastHandlerRan).toBe(true);
expect(warnings.some((w) => w.includes("hanging-void-plugin"))).toBe(true);
});
// -----------------------------------------------------------------------
// Claiming hooks
// -----------------------------------------------------------------------
it("skips a hanging claiming hook handler after timeout", async () => {
const warnings: string[] = [];
addTestHook({
registry,
pluginId: "hanging-claim-plugin",
hookName: "inbound_claim",
handler: (() => new Promise(() => {})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, {
hookTimeoutMs: 100,
catchErrors: true,
logger: {
debug: () => {},
warn: (msg) => warnings.push(msg),
error: (msg) => warnings.push(msg),
},
});
const claimCtx = {
channelId: "test",
};
const promise = runner.runInboundClaim(
{ content: "hello", channel: "test", isGroup: false },
claimCtx,
);
await vi.advanceTimersByTimeAsync(150);
const result = await promise;
expect(result).toBeUndefined();
expect(warnings.some((w) => w.includes("hanging-claim-plugin"))).toBe(true);
});
// -----------------------------------------------------------------------
// Timeout configuration
// -----------------------------------------------------------------------
it("uses the default timeout when hookTimeoutMs is not specified", async () => {
// Handler that resolves in 5 seconds (well under the 10s default)
addTestHook({
registry,
pluginId: "slow-but-ok",
hookName: "before_agent_start",
handler: (() =>
new Promise((resolve) => {
setTimeout(() => resolve({ prependContext: "slow result" }), 5000);
})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, { catchErrors: true });
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(5000);
const result = await promise;
expect(result?.prependContext).toBe("slow result");
});
it("respects hookTimeoutMs: 0 to disable timeout", async () => {
// Handler that takes a long time but should not be timed out
addTestHook({
registry,
pluginId: "very-slow-plugin",
hookName: "before_agent_start",
handler: (() =>
new Promise((resolve) => {
setTimeout(() => resolve({ prependContext: "very slow result" }), 30_000);
})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, { hookTimeoutMs: 0, catchErrors: true });
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(30_000);
const result = await promise;
expect(result?.prependContext).toBe("very slow result");
});
it("uses custom hookTimeoutMs when specified", async () => {
const errors: string[] = [];
addTestHook({
registry,
pluginId: "medium-slow-plugin",
hookName: "before_agent_start",
handler: (() =>
new Promise((resolve) => {
setTimeout(() => resolve({ prependContext: "result" }), 3000);
})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, {
hookTimeoutMs: 2000,
catchErrors: true,
logger: {
debug: () => {},
warn: () => {},
error: (msg) => errors.push(msg),
},
});
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
await vi.advanceTimersByTimeAsync(2500);
const result = await promise;
// Should have timed out at 2000ms before the handler resolved at 3000ms
expect(result).toBeUndefined();
expect(errors.some((e) => e.includes("medium-slow-plugin"))).toBe(true);
});
// -----------------------------------------------------------------------
// Error propagation when catchErrors is false
// -----------------------------------------------------------------------
it("throws timeout error when catchErrors is false", async () => {
addTestHook({
registry,
pluginId: "hanging-throw-plugin",
hookName: "before_agent_start",
handler: (() => new Promise(() => {})) as PluginHookRegistration["handler"],
});
const runner = createHookRunner(registry, { hookTimeoutMs: 50, catchErrors: false });
const promise = runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
// Register the rejection handler before advancing timers to avoid
// unhandled rejection noise.
const assertion = expect(promise).rejects.toThrow(/timed out/);
await vi.advanceTimersByTimeAsync(100);
await assertion;
});
});

View File

@ -786,9 +786,13 @@ function warnAboutUntrackedLoadedPlugins(params: {
}
}
function activatePluginRegistry(registry: PluginRegistry, cacheKey: string): void {
function activatePluginRegistry(
registry: PluginRegistry,
cacheKey: string,
options?: { hookTimeoutMs?: number },
): void {
setActivePluginRegistry(registry, cacheKey);
initializeGlobalHookRunner(registry);
initializeGlobalHookRunner(registry, { hookTimeoutMs: options?.hookTimeoutMs });
}
export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegistry {
@ -835,7 +839,9 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
const cached = getCachedPluginRegistry(cacheKey);
if (cached) {
if (shouldActivate) {
activatePluginRegistry(cached, cacheKey);
activatePluginRegistry(cached, cacheKey, {
hookTimeoutMs: options.config?.plugins?.hookTimeoutMs,
});
}
return cached;
}
@ -1372,7 +1378,9 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
setCachedPluginRegistry(cacheKey, registry);
}
if (shouldActivate) {
activatePluginRegistry(registry, cacheKey);
activatePluginRegistry(registry, cacheKey, {
hookTimeoutMs: options.config?.plugins?.hookTimeoutMs,
});
}
return registry;
}