From f849b8de97d5c5b19fdb2a4b3b0a44b0fa7665b3 Mon Sep 17 00:00:00 2001 From: joelnishanth <140015627+joelnishanth@users.noreply.github.com> Date: Fri, 27 Mar 2026 13:08:32 -0500 Subject: [PATCH] hooks: default hooks.internal.enabled to true so bundled hooks load on fresh installs Made-with: Cursor --- src/gateway/server-startup.ts | 2 +- src/hooks/loader.test.ts | 23 ++++++++++++++++++++++- src/hooks/loader.ts | 6 +++--- src/plugins/registry.ts | 2 +- src/security/audit-extra.sync.test.ts | 11 +++++++++-- src/security/audit-extra.sync.ts | 2 +- 6 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/gateway/server-startup.ts b/src/gateway/server-startup.ts index 9cf5a5c149d..6bc7a85a9d2 100644 --- a/src/gateway/server-startup.ts +++ b/src/gateway/server-startup.ts @@ -167,7 +167,7 @@ export async function startGatewaySidecars(params: { ); } - if (params.cfg.hooks?.internal?.enabled) { + if (params.cfg.hooks?.internal?.enabled !== false) { setTimeout(() => { const hookEvent = createInternalHookEvent("gateway", "startup", "gateway:startup", { cfg: params.cfg, diff --git a/src/hooks/loader.test.ts b/src/hooks/loader.test.ts index e6cfd88ac44..3629a1c35ba 100644 --- a/src/hooks/loader.test.ts +++ b/src/hooks/loader.test.ts @@ -121,7 +121,7 @@ describe("loader", () => { expect(getRegisteredEventKeys()).not.toContain("command:new"); }; - it("should return 0 when hooks are disabled or missing", async () => { + it("should return 0 when hooks are explicitly disabled", async () => { for (const cfg of [ { hooks: { @@ -130,7 +130,28 @@ describe("loader", () => { }, }, } satisfies OpenClawConfig, + { + hooks: { + internal: { + enabled: false, + handlers: [], + }, + }, + } satisfies OpenClawConfig, + ]) { + const count = await loadInternalHooks(cfg, tmpDir); + expect(count).toBe(0); + } + }); + + it("should treat missing hooks.internal.enabled as enabled (default-on)", async () => { + // Empty config should NOT skip loading — it should attempt discovery. + // With no discoverable hooks in the temp dir (bundled dir is overridden + // to /nonexistent), this returns 0 but does NOT bail at the guard. + for (const cfg of [ {} satisfies OpenClawConfig, + { hooks: {} } satisfies OpenClawConfig, + { hooks: { internal: {} } } satisfies OpenClawConfig, ]) { const count = await loadInternalHooks(cfg, tmpDir); expect(count).toBe(0); diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 19554d1e1d8..8a19945ccc5 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -65,8 +65,8 @@ export async function loadInternalHooks( bundledHooksDir?: string; }, ): Promise { - // Check if hooks are enabled - if (!cfg.hooks?.internal?.enabled) { + // Hooks are on by default; only skip when explicitly disabled. + if (cfg.hooks?.internal?.enabled === false) { return 0; } @@ -153,7 +153,7 @@ export async function loadInternalHooks( } // 2. Load legacy config handlers (backwards compatibility) - const handlers = cfg.hooks.internal.handlers ?? []; + const handlers = cfg.hooks?.internal?.handlers ?? []; for (const handlerConfig of handlers) { try { // Legacy handler paths: keep them workspace-relative. diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index 45a21056c80..a50cdb8e043 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -375,7 +375,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { source: record.source, }); - const hookSystemEnabled = config?.hooks?.internal?.enabled === true; + const hookSystemEnabled = config?.hooks?.internal?.enabled !== false; if (!hookSystemEnabled || opts?.register === false) { return; } diff --git a/src/security/audit-extra.sync.test.ts b/src/security/audit-extra.sync.test.ts index 93f4db9fa78..0e327abc9bd 100644 --- a/src/security/audit-extra.sync.test.ts +++ b/src/security/audit-extra.sync.test.ts @@ -23,9 +23,16 @@ describe("collectAttackSurfaceSummaryFindings", () => { expectedDetail: ["hooks.webhooks: enabled", "hooks.internal: enabled"], }, { - name: "reports both hook systems as disabled when neither is configured", + name: "reports internal hooks as enabled by default and webhooks as disabled when neither is configured", cfg: {} satisfies OpenClawConfig, - expectedDetail: ["hooks.webhooks: disabled", "hooks.internal: disabled"], + expectedDetail: ["hooks.webhooks: disabled", "hooks.internal: enabled"], + }, + { + name: "reports internal hooks as disabled when explicitly set to false", + cfg: { + hooks: { internal: { enabled: false } }, + } satisfies OpenClawConfig, + expectedDetail: ["hooks.internal: disabled"], }, ])("$name", ({ cfg, expectedDetail }) => { const [finding] = collectAttackSurfaceSummaryFindings(cfg); diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index 7d54c24b835..c983a38b763 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -526,7 +526,7 @@ export function collectAttackSurfaceSummaryFindings(cfg: OpenClawConfig): Securi const group = summarizeGroupPolicy(cfg); const elevated = cfg.tools?.elevated?.enabled !== false; const webhooksEnabled = cfg.hooks?.enabled === true; - const internalHooksEnabled = cfg.hooks?.internal?.enabled === true; + const internalHooksEnabled = cfg.hooks?.internal?.enabled !== false; const browserEnabled = cfg.browser?.enabled ?? true; const detail =