refactor(security): reuse hook agent routing normalization
This commit is contained in:
@@ -99,7 +99,7 @@ function resolveKnownAgentIds(cfg: OpenClawConfig, defaultAgentId: string): Set<
|
||||
return known;
|
||||
}
|
||||
|
||||
function resolveAllowedAgentIds(raw: string[] | undefined): Set<string> | undefined {
|
||||
export function resolveAllowedAgentIds(raw: string[] | undefined): Set<string> | undefined {
|
||||
if (!Array.isArray(raw)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user