mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 09:58:38 +00:00
fix(security): audit unrestricted hook agent routing
This commit is contained in:
@@ -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.
|
- 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.
|
- 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.
|
- 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
|
## 2026.3.11
|
||||||
|
|
||||||
|
|||||||
@@ -87,6 +87,13 @@ function looksLikeEnvRef(value: string): boolean {
|
|||||||
return v.startsWith("${") && v.endsWith("}");
|
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 {
|
function isGatewayRemotelyExposed(cfg: OpenClawConfig): boolean {
|
||||||
const bind = typeof cfg.gateway?.bind === "string" ? cfg.gateway.bind : "loopback";
|
const bind = typeof cfg.gateway?.bind === "string" ? cfg.gateway.bind : "loopback";
|
||||||
if (bind !== "loopback") {
|
if (bind !== "loopback") {
|
||||||
@@ -663,6 +670,11 @@ export function collectHooksHardeningFindings(
|
|||||||
const allowRequestSessionKey = cfg.hooks?.allowRequestSessionKey === true;
|
const allowRequestSessionKey = cfg.hooks?.allowRequestSessionKey === true;
|
||||||
const defaultSessionKey =
|
const defaultSessionKey =
|
||||||
typeof cfg.hooks?.defaultSessionKey === "string" ? cfg.hooks.defaultSessionKey.trim() : "";
|
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)
|
const allowedPrefixes = Array.isArray(cfg.hooks?.allowedSessionKeyPrefixes)
|
||||||
? cfg.hooks.allowedSessionKeyPrefixes
|
? cfg.hooks.allowedSessionKeyPrefixes
|
||||||
.map((prefix) => prefix.trim())
|
.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) {
|
if (allowRequestSessionKey) {
|
||||||
findings.push({
|
findings.push({
|
||||||
checkId: "hooks.request_session_key_enabled",
|
checkId: "hooks.request_session_key_enabled",
|
||||||
|
|||||||
@@ -2656,6 +2656,52 @@ description: test skill
|
|||||||
expectFinding(res, "hooks.default_session_key_unset", "warn");
|
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<OpenClawConfig["hooks"]>;
|
||||||
|
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 () => {
|
it("scores hooks request sessionKey override by gateway exposure", async () => {
|
||||||
const baseHooks = {
|
const baseHooks = {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
|
|||||||
Reference in New Issue
Block a user