From e355f6e093df0fecc9a659ce9f92bf83784d5519 Mon Sep 17 00:00:00 2001 From: Marcus Castro Date: Fri, 13 Feb 2026 00:46:27 -0300 Subject: [PATCH] fix(security): distinguish webhooks from internal hooks in audit summary (#13474) * fix(security): distinguish webhooks from internal hooks in audit summary The attack surface summary reported a single 'hooks: disabled/enabled' line that only checked the external webhook endpoint (hooks.enabled), ignoring internal hooks (hooks.internal.enabled). Users who enabled internal hooks (session-memory, command-logger, etc.) saw 'hooks: disabled' and thought something was broken. Split into two separate lines: - hooks.webhooks: disabled/enabled - hooks.internal: disabled/enabled Fixes #13466 * test(security): move attack surface tests to focused test file Move the 3 new hook-distinction tests from the monolithic audit.test.ts (1,511 lines) into a dedicated audit-extra.sync.test.ts that tests collectAttackSurfaceSummaryFindings directly. Avoids growing the already-large test file and keeps tests focused on the changed unit. * fix: add changelog entry for security audit hook split (#13474) (thanks @mcaxtr) --------- Co-authored-by: Peter Steinberger --- CHANGELOG.md | 1 + src/security/audit-extra.sync.test.ts | 34 +++++++++++++++++++++++++++ src/security/audit-extra.sync.ts | 7 ++++-- 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 src/security/audit-extra.sync.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d4465124d..2b72fd19b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Audit: distinguish external webhooks (`hooks.enabled`) from internal hooks (`hooks.internal.enabled`) in attack-surface summaries to avoid false exposure signals when only internal hooks are enabled. (#13474) Thanks @mcaxtr. - Sandbox: pass configured `sandbox.docker.env` variables to sandbox containers at `docker create` time. (#15138) Thanks @stevebot-alive. - Onboarding/CLI: restore terminal state without resuming paused `stdin`, so onboarding exits cleanly after choosing Web UI and the installer returns instead of appearing stuck. - macOS Voice Wake: fix a crash in trigger trimming for CJK/Unicode transcripts by matching and slicing on original-string ranges instead of transformed-string indices. (#11052) Thanks @Flash-LHR. diff --git a/src/security/audit-extra.sync.test.ts b/src/security/audit-extra.sync.test.ts new file mode 100644 index 0000000000..88d374f2f3 --- /dev/null +++ b/src/security/audit-extra.sync.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { collectAttackSurfaceSummaryFindings } from "./audit-extra.sync.js"; + +describe("collectAttackSurfaceSummaryFindings", () => { + it("distinguishes external webhooks from internal hooks when only internal hooks are enabled", () => { + const cfg: OpenClawConfig = { + hooks: { internal: { enabled: true } }, + }; + + const [finding] = collectAttackSurfaceSummaryFindings(cfg); + expect(finding.checkId).toBe("summary.attack_surface"); + expect(finding.detail).toContain("hooks.webhooks: disabled"); + expect(finding.detail).toContain("hooks.internal: enabled"); + }); + + it("reports both hook systems as enabled when both are configured", () => { + const cfg: OpenClawConfig = { + hooks: { enabled: true, internal: { enabled: true } }, + }; + + const [finding] = collectAttackSurfaceSummaryFindings(cfg); + expect(finding.detail).toContain("hooks.webhooks: enabled"); + expect(finding.detail).toContain("hooks.internal: enabled"); + }); + + it("reports both hook systems as disabled when neither is configured", () => { + const cfg: OpenClawConfig = {}; + + const [finding] = collectAttackSurfaceSummaryFindings(cfg); + expect(finding.detail).toContain("hooks.webhooks: disabled"); + expect(finding.detail).toContain("hooks.internal: disabled"); + }); +}); diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index c2e9a635bb..45330dbfd2 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -303,7 +303,8 @@ function listGroupPolicyOpen(cfg: OpenClawConfig): string[] { export function collectAttackSurfaceSummaryFindings(cfg: OpenClawConfig): SecurityAuditFinding[] { const group = summarizeGroupPolicy(cfg); const elevated = cfg.tools?.elevated?.enabled !== false; - const hooksEnabled = cfg.hooks?.enabled === true; + const webhooksEnabled = cfg.hooks?.enabled === true; + const internalHooksEnabled = cfg.hooks?.internal?.enabled === true; const browserEnabled = cfg.browser?.enabled ?? true; const detail = @@ -311,7 +312,9 @@ export function collectAttackSurfaceSummaryFindings(cfg: OpenClawConfig): Securi `\n` + `tools.elevated: ${elevated ? "enabled" : "disabled"}` + `\n` + - `hooks: ${hooksEnabled ? "enabled" : "disabled"}` + + `hooks.webhooks: ${webhooksEnabled ? "enabled" : "disabled"}` + + `\n` + + `hooks.internal: ${internalHooksEnabled ? "enabled" : "disabled"}` + `\n` + `browser control: ${browserEnabled ? "enabled" : "disabled"}`;