diff --git a/src/gateway/hooks.ts b/src/gateway/hooks.ts index 957056bab..32751369f 100644 --- a/src/gateway/hooks.ts +++ b/src/gateway/hooks.ts @@ -99,7 +99,7 @@ function resolveKnownAgentIds(cfg: OpenClawConfig, defaultAgentId: string): Set< return known; } -function resolveAllowedAgentIds(raw: string[] | undefined): Set | undefined { +export function resolveAllowedAgentIds(raw: string[] | undefined): Set | undefined { if (!Array.isArray(raw)) { return undefined; } diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index a379537bc..79a701c54 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -21,6 +21,7 @@ import { } from "../config/model-input.js"; import type { AgentToolsConfig } from "../config/types.tools.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; +import { resolveAllowedAgentIds } from "../gateway/hooks.js"; import { DEFAULT_DANGEROUS_NODE_COMMANDS, resolveNodeCommandAllowlist, @@ -87,13 +88,6 @@ 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") { @@ -670,11 +664,7 @@ 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 allowedAgentIds = resolveAllowedAgentIds(cfg.hooks?.allowedAgentIds); const allowedPrefixes = Array.isArray(cfg.hooks?.allowedSessionKeyPrefixes) ? cfg.hooks.allowedSessionKeyPrefixes .map((prefix) => prefix.trim()) @@ -693,9 +683,9 @@ export function collectHooksHardeningFindings( }); } - if (isHookAgentRoutingUnrestricted(allowedAgentIds)) { + if (allowedAgentIds === undefined) { findings.push({ - checkId: "hooks.allowed_agent_ids_unset", + checkId: "hooks.allowed_agent_ids_unrestricted", severity: remoteExposure ? "critical" : "warn", title: "Hook agent routing allows any configured agent", detail: diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 69bda5538..2546feae9 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -2656,7 +2656,7 @@ description: test skill expectFinding(res, "hooks.default_session_key_unset", "warn"); }); - it("scores hooks.allowedAgentIds unset by gateway exposure", async () => { + it("scores unrestricted hooks.allowedAgentIds by gateway exposure", async () => { const baseHooks = { enabled: true, token: "shared-gateway-token-1234567890", @@ -2682,7 +2682,7 @@ description: test skill cases.map(async (testCase) => { const res = await audit(testCase.cfg); expect( - hasFinding(res, "hooks.allowed_agent_ids_unset", testCase.expectedSeverity), + hasFinding(res, "hooks.allowed_agent_ids_unrestricted", testCase.expectedSeverity), testCase.name, ).toBe(true); }), @@ -2699,7 +2699,7 @@ description: test skill }, }); - expectFinding(res, "hooks.allowed_agent_ids_unset", "warn"); + expectFinding(res, "hooks.allowed_agent_ids_unrestricted", "warn"); }); it("scores hooks request sessionKey override by gateway exposure", async () => {