From 904db27019ae2b5a96c1568cba962af02f6f3aba Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 12 Mar 2026 21:35:47 +0000 Subject: [PATCH] fix(security): audit unrestricted hook agent routing --- CHANGELOG.md | 6 +++++ src/security/audit-extra.sync.ts | 24 +++++++++++++++++ src/security/audit.test.ts | 46 ++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54b3a67cc..dcce9520c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,12 @@ Docs: https://docs.openclaw.ai - Context engine/session routing: forward optional `sessionKey` through context-engine lifecycle calls so plugins can see structured routing metadata during bootstrap, assembly, post-turn ingestion, and compaction. (#44157) thanks @jalehman. - Agents/failover: classify z.ai `network_error` stop reasons as retryable timeouts so provider connectivity failures trigger fallback instead of surfacing raw unhandled-stop-reason errors. (#43884) Thanks @hougangdev. - Memory/session sync: add mode-aware post-compaction session reindexing with `agents.defaults.compaction.postIndexSync` plus `agents.defaults.memorySearch.sync.sessions.postCompactionForce`, so compacted session memory can refresh immediately without forcing every deployment into synchronous reindexing. (#25561) thanks @rodrigouroz. +- Telegram/model picker: make inline model button selections persist the chosen session model correctly, clear overrides when selecting the configured default, and include effective fallback models in `/models` button validation. (#40105) Thanks @avirweb. +- Mattermost/reply media delivery: pass agent-scoped `mediaLocalRoots` through shared reply delivery so allowed local files upload correctly from button, slash-command, and model-picker replies. (#44021) Thanks @LyleLiu666. +- Plugins/env-scoped roots: fix plugin discovery/load caches and provenance tracking so same-process `HOME`/`OPENCLAW_HOME` changes no longer reuse stale plugin state or misreport `~/...` plugins as untracked. (#44046) thanks @gumadeiras. +- Gateway/session discovery: discover disk-only and retired ACP session stores under custom templated `session.store` roots so ACP reconciliation, session-id/session-label targeting, and run-id fallback keep working after restart. (#44176) thanks @gumadeiras. +- Models/OpenRouter native ids: canonicalize native OpenRouter model keys across config writes, runtime lookups, fallback management, and `models list --plain`, and migrate legacy duplicated `openrouter/openrouter/...` config entries forward on write. +- Gateway/hooks: bucket hook auth failures by forwarded client IP behind trusted proxies and warn when `hooks.allowedAgentIds` leaves hook routing unrestricted. ## 2026.3.11 diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index cf12ac2f9..a379537bc 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -87,6 +87,13 @@ function looksLikeEnvRef(value: string): boolean { return v.startsWith("${") && v.endsWith("}"); } +function isHookAgentRoutingUnrestricted(allowedAgentIds: string[] | undefined): boolean { + if (!allowedAgentIds) { + return true; + } + return allowedAgentIds.some((agentId) => agentId === "*"); +} + function isGatewayRemotelyExposed(cfg: OpenClawConfig): boolean { const bind = typeof cfg.gateway?.bind === "string" ? cfg.gateway.bind : "loopback"; if (bind !== "loopback") { @@ -663,6 +670,11 @@ export function collectHooksHardeningFindings( const allowRequestSessionKey = cfg.hooks?.allowRequestSessionKey === true; const defaultSessionKey = typeof cfg.hooks?.defaultSessionKey === "string" ? cfg.hooks.defaultSessionKey.trim() : ""; + const allowedAgentIds = Array.isArray(cfg.hooks?.allowedAgentIds) + ? cfg.hooks.allowedAgentIds + .map((agentId) => agentId.trim()) + .filter((agentId) => agentId.length > 0) + : undefined; const allowedPrefixes = Array.isArray(cfg.hooks?.allowedSessionKeyPrefixes) ? cfg.hooks.allowedSessionKeyPrefixes .map((prefix) => prefix.trim()) @@ -681,6 +693,18 @@ export function collectHooksHardeningFindings( }); } + if (isHookAgentRoutingUnrestricted(allowedAgentIds)) { + findings.push({ + checkId: "hooks.allowed_agent_ids_unset", + severity: remoteExposure ? "critical" : "warn", + title: "Hook agent routing allows any configured agent", + detail: + "hooks.allowedAgentIds is unset or includes '*', so authenticated hook callers may route to any configured agent id.", + remediation: + 'Set hooks.allowedAgentIds to an explicit allowlist (for example, ["hooks", "main"]) or [] to deny explicit agent routing.', + }); + } + if (allowRequestSessionKey) { findings.push({ checkId: "hooks.request_session_key_enabled", diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 1c696bf6e..69bda5538 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -2656,6 +2656,52 @@ description: test skill expectFinding(res, "hooks.default_session_key_unset", "warn"); }); + it("scores hooks.allowedAgentIds unset by gateway exposure", async () => { + const baseHooks = { + enabled: true, + token: "shared-gateway-token-1234567890", + defaultSessionKey: "hook:ingress", + } satisfies NonNullable; + const cases: Array<{ + name: string; + cfg: OpenClawConfig; + expectedSeverity: "warn" | "critical"; + }> = [ + { + name: "local exposure", + cfg: { hooks: baseHooks }, + expectedSeverity: "warn", + }, + { + name: "remote exposure", + cfg: { gateway: { bind: "lan" }, hooks: baseHooks }, + expectedSeverity: "critical", + }, + ]; + await Promise.all( + cases.map(async (testCase) => { + const res = await audit(testCase.cfg); + expect( + hasFinding(res, "hooks.allowed_agent_ids_unset", testCase.expectedSeverity), + testCase.name, + ).toBe(true); + }), + ); + }); + + it("treats wildcard hooks.allowedAgentIds as unrestricted routing", async () => { + const res = await audit({ + hooks: { + enabled: true, + token: "shared-gateway-token-1234567890", + defaultSessionKey: "hook:ingress", + allowedAgentIds: ["*"], + }, + }); + + expectFinding(res, "hooks.allowed_agent_ids_unset", "warn"); + }); + it("scores hooks request sessionKey override by gateway exposure", async () => { const baseHooks = { enabled: true,