mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(security): harden gateway command/audit guardrails
This commit is contained in:
@@ -43,6 +43,8 @@ Docs: https://docs.openclaw.ai
|
||||
- Chat/UI: strip inline reply/audio directive tags (`[[reply_to_current]]`, `[[reply_to:<id>]]`, `[[audio_as_voice]]`) from displayed chat history, live chat event output, and session preview snippets so control tags no longer leak into user-visible surfaces.
|
||||
- BlueBubbles/DM history: restore DM backfill context with account-scoped rolling history, bounded backfill retries, and safer history payload limits. (#20302) Thanks @Ryan-Haines.
|
||||
- BlueBubbles/Webhooks: accept inbound/reaction webhook payloads when BlueBubbles omits `handle` but provides DM `chatGuid`, and harden payload extraction for array/string-wrapped message bodies so valid webhook events no longer get rejected as unparseable. (#23275) Thanks @toph31.
|
||||
- Security/Audit: add `openclaw security audit` finding `gateway.nodes.allow_commands_dangerous` for risky `gateway.nodes.allowCommands` overrides, with severity upgraded to critical on remote gateway exposure.
|
||||
- Gateway/Control plane: reduce cross-client write limiter contention by adding `connId` fallback keying when device ID and client IP are both unavailable.
|
||||
- Security/Config: block prototype-key traversal during config merge patch and legacy migration merge helpers (`__proto__`, `constructor`, `prototype`) to prevent prototype pollution during config mutation flows. (#22968) Thanks @Clawborn.
|
||||
- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting.
|
||||
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
|
||||
|
||||
@@ -27,7 +27,7 @@ The audit warns when multiple DM senders share the main session and recommends *
|
||||
This is for cooperative/shared inbox hardening. A single Gateway shared by mutually untrusted/adversarial operators is not a recommended setup; split trust boundaries with separate gateways (or separate OS users/hosts).
|
||||
It also warns when small models (`<=300B`) are used without sandboxing and with web/browser tools enabled.
|
||||
For webhook ingress, it warns when `hooks.defaultSessionKey` is unset, when request `sessionKey` overrides are enabled, and when overrides are enabled without `hooks.allowedSessionKeyPrefixes`.
|
||||
It also warns when sandbox Docker settings are configured while sandbox mode is off, when `gateway.nodes.denyCommands` uses ineffective pattern-like/unknown entries, when global `tools.profile="minimal"` is overridden by agent tool profiles, when open groups expose runtime/filesystem tools without sandbox/workspace guards, and when installed extension plugin tools may be reachable under permissive tool policy.
|
||||
It also warns when sandbox Docker settings are configured while sandbox mode is off, when `gateway.nodes.denyCommands` uses ineffective pattern-like/unknown entries, when `gateway.nodes.allowCommands` explicitly enables dangerous node commands, when global `tools.profile="minimal"` is overridden by agent tool profiles, when open groups expose runtime/filesystem tools without sandbox/workspace guards, and when installed extension plugin tools may be reachable under permissive tool policy.
|
||||
It also warns when sandbox browser uses Docker `bridge` network without `sandbox.browser.cdpSourceRange`.
|
||||
It also warns when existing sandbox browser Docker containers have missing/stale hash labels (for example pre-migration containers missing `openclaw.browserConfigEpoch`) and recommends `openclaw sandbox recreate --browser --all`.
|
||||
It also warns when npm-based plugin/hook install records are unpinned, missing integrity metadata, or drift from currently installed package versions.
|
||||
|
||||
@@ -84,7 +84,7 @@ If more than one person can DM your bot:
|
||||
- **Browser control exposure** (remote nodes, relay ports, remote CDP endpoints).
|
||||
- **Local disk hygiene** (permissions, symlinks, config includes, “synced folder” paths).
|
||||
- **Plugins** (extensions exist without an explicit allowlist).
|
||||
- **Policy drift/misconfig** (sandbox docker settings configured but sandbox mode off; ineffective `gateway.nodes.denyCommands` patterns; global `tools.profile="minimal"` overridden by per-agent profiles; extension plugin tools reachable under permissive tool policy).
|
||||
- **Policy drift/misconfig** (sandbox docker settings configured but sandbox mode off; ineffective `gateway.nodes.denyCommands` patterns; dangerous `gateway.nodes.allowCommands` entries; global `tools.profile="minimal"` overridden by per-agent profiles; extension plugin tools reachable under permissive tool policy).
|
||||
- **Runtime expectation drift** (for example `tools.exec.host="sandbox"` while sandbox mode is off, which runs directly on the gateway host).
|
||||
- **Model hygiene** (warn when configured models look legacy; not a hard block).
|
||||
|
||||
@@ -117,30 +117,31 @@ When the audit prints findings, treat this as a priority order:
|
||||
|
||||
High-signal `checkId` values you will most likely see in real deployments (not exhaustive):
|
||||
|
||||
| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix |
|
||||
| -------------------------------------------------- | ------------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | -------- |
|
||||
| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes |
|
||||
| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes |
|
||||
| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes |
|
||||
| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no |
|
||||
| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no |
|
||||
| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*` | no |
|
||||
| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API | `gateway.tools.allow` | no |
|
||||
| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no |
|
||||
| `gateway.control_ui.insecure_auth` | warn | Insecure-auth compatibility toggle enabled | `gateway.controlUi.allowInsecureAuth` | no |
|
||||
| `gateway.control_ui.device_auth_disabled` | critical | Disables device identity check | `gateway.controlUi.dangerouslyDisableDeviceAuth` | no |
|
||||
| `config.insecure_or_dangerous_flags` | warn | Any insecure/dangerous debug flags enabled | multiple keys (see finding detail) | no |
|
||||
| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no |
|
||||
| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no |
|
||||
| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no |
|
||||
| `logging.redact_off` | warn | Sensitive values leak to logs/status | `logging.redactSensitive` | yes |
|
||||
| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no |
|
||||
| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` resolves to host exec when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no |
|
||||
| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` resolves to host exec when sandbox is off | `agents.list[].tools.exec.host`, `agents.list[].sandbox.mode` | no |
|
||||
| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix |
|
||||
| -------------------------------------------------- | ------------- | ------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | -------- |
|
||||
| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes |
|
||||
| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes |
|
||||
| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes |
|
||||
| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no |
|
||||
| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no |
|
||||
| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*` | no |
|
||||
| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API | `gateway.tools.allow` | no |
|
||||
| `gateway.nodes.allow_commands_dangerous` | warn/critical | Enables high-impact node commands (camera/screen/contacts/calendar/SMS) | `gateway.nodes.allowCommands` | no |
|
||||
| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no |
|
||||
| `gateway.control_ui.insecure_auth` | warn | Insecure-auth compatibility toggle enabled | `gateway.controlUi.allowInsecureAuth` | no |
|
||||
| `gateway.control_ui.device_auth_disabled` | critical | Disables device identity check | `gateway.controlUi.dangerouslyDisableDeviceAuth` | no |
|
||||
| `config.insecure_or_dangerous_flags` | warn | Any insecure/dangerous debug flags enabled | multiple keys (see finding detail) | no |
|
||||
| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no |
|
||||
| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no |
|
||||
| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no |
|
||||
| `logging.redact_off` | warn | Sensitive values leak to logs/status | `logging.redactSensitive` | yes |
|
||||
| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no |
|
||||
| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` resolves to host exec when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no |
|
||||
| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` resolves to host exec when sandbox is off | `agents.list[].tools.exec.host`, `agents.list[].sandbox.mode` | no |
|
||||
| `security.exposure.open_groups_with_runtime_or_fs` | critical/warn | Open groups can reach command/file tools without sandbox/workspace guards | `channels.*.groupPolicy`, `tools.profile/deny`, `tools.fs.workspaceOnly`, `agents.*.sandbox.mode` | no |
|
||||
| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.list[].tools.profile` | no |
|
||||
| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no |
|
||||
| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no |
|
||||
| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.list[].tools.profile` | no |
|
||||
| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no |
|
||||
| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no |
|
||||
|
||||
## Control UI over HTTP
|
||||
|
||||
|
||||
@@ -51,7 +51,7 @@ export const FIELD_HELP: Record<string, string> = {
|
||||
'Node browser routing ("auto" = pick single connected browser node, "manual" = require node param, "off" = disable).',
|
||||
"gateway.nodes.browser.node": "Pin browser routing to a specific node id or name (optional).",
|
||||
"gateway.nodes.allowCommands":
|
||||
"Extra node.invoke commands to allow beyond the gateway defaults (array of command strings).",
|
||||
"Extra node.invoke commands to allow beyond the gateway defaults (array of command strings). Enabling dangerous commands here is a security-sensitive override and is flagged by `openclaw security audit`.",
|
||||
"gateway.nodes.denyCommands":
|
||||
"Commands to block even if present in node claims or default allowlist.",
|
||||
"nodeHost.browserProxy.enabled": "Expose the local browser control server via node proxy.",
|
||||
|
||||
@@ -21,6 +21,13 @@ function normalizePart(value: unknown, fallback: string): string {
|
||||
export function resolveControlPlaneRateLimitKey(client: GatewayClient | null): string {
|
||||
const deviceId = normalizePart(client?.connect?.device?.id, "unknown-device");
|
||||
const clientIp = normalizePart(client?.clientIp, "unknown-ip");
|
||||
if (deviceId === "unknown-device" && clientIp === "unknown-ip") {
|
||||
// Last-resort fallback: avoid cross-client contention when upstream identity is missing.
|
||||
const connId = normalizePart(client?.connId, "");
|
||||
if (connId) {
|
||||
return `${deviceId}|${clientIp}|conn=${connId}`;
|
||||
}
|
||||
}
|
||||
return `${deviceId}|${clientIp}`;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { __testing as controlPlaneRateLimitTesting } from "./control-plane-rate-limit.js";
|
||||
import {
|
||||
__testing as controlPlaneRateLimitTesting,
|
||||
resolveControlPlaneRateLimitKey,
|
||||
} from "./control-plane-rate-limit.js";
|
||||
import { handleGatewayRequest } from "./server-methods.js";
|
||||
import type { GatewayRequestHandler } from "./server-methods/types.js";
|
||||
|
||||
@@ -121,4 +124,43 @@ describe("gateway control-plane write rate limit", () => {
|
||||
expect(allowed).toHaveBeenCalledWith(true, undefined, undefined);
|
||||
expect(handlerCalls).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
it("uses connId fallback when both device and client IP are unknown", () => {
|
||||
const key = resolveControlPlaneRateLimitKey({
|
||||
connect: {
|
||||
role: "operator",
|
||||
scopes: ["operator.admin"],
|
||||
client: {
|
||||
id: "openclaw-control-ui",
|
||||
version: "1.0.0",
|
||||
platform: "darwin",
|
||||
mode: "ui",
|
||||
},
|
||||
minProtocol: 1,
|
||||
maxProtocol: 1,
|
||||
},
|
||||
connId: "conn-fallback",
|
||||
});
|
||||
expect(key).toBe("unknown-device|unknown-ip|conn=conn-fallback");
|
||||
});
|
||||
|
||||
it("keeps device/IP-based key when identity is present", () => {
|
||||
const key = resolveControlPlaneRateLimitKey({
|
||||
connect: {
|
||||
role: "operator",
|
||||
scopes: ["operator.admin"],
|
||||
client: {
|
||||
id: "openclaw-control-ui",
|
||||
version: "1.0.0",
|
||||
platform: "darwin",
|
||||
mode: "ui",
|
||||
},
|
||||
minProtocol: 1,
|
||||
maxProtocol: 1,
|
||||
},
|
||||
connId: "conn-fallback",
|
||||
clientIp: "10.0.0.10",
|
||||
});
|
||||
expect(key).toBe("unknown-device|10.0.0.10");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,7 +16,10 @@ import { formatCliCommand } from "../cli/command-format.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import type { AgentToolsConfig } from "../config/types.tools.js";
|
||||
import { resolveGatewayAuth } from "../gateway/auth.js";
|
||||
import { resolveNodeCommandAllowlist } from "../gateway/node-command-policy.js";
|
||||
import {
|
||||
DEFAULT_DANGEROUS_NODE_COMMANDS,
|
||||
resolveNodeCommandAllowlist,
|
||||
} from "../gateway/node-command-policy.js";
|
||||
import { inferParamBFromIdOrName } from "../shared/model-param-b.js";
|
||||
import { pickSandboxToolPolicy } from "./audit-tool-policy.js";
|
||||
|
||||
@@ -805,6 +808,43 @@ export function collectNodeDenyCommandPatternFindings(cfg: OpenClawConfig): Secu
|
||||
return findings;
|
||||
}
|
||||
|
||||
export function collectNodeDangerousAllowCommandFindings(
|
||||
cfg: OpenClawConfig,
|
||||
): SecurityAuditFinding[] {
|
||||
const findings: SecurityAuditFinding[] = [];
|
||||
const allowRaw = cfg.gateway?.nodes?.allowCommands;
|
||||
if (!Array.isArray(allowRaw) || allowRaw.length === 0) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
const allow = new Set(allowRaw.map(normalizeNodeCommand).filter(Boolean));
|
||||
if (allow.size === 0) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
const deny = new Set((cfg.gateway?.nodes?.denyCommands ?? []).map(normalizeNodeCommand));
|
||||
const dangerousAllowed = DEFAULT_DANGEROUS_NODE_COMMANDS.filter(
|
||||
(cmd) => allow.has(cmd) && !deny.has(cmd),
|
||||
);
|
||||
if (dangerousAllowed.length === 0) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
findings.push({
|
||||
checkId: "gateway.nodes.allow_commands_dangerous",
|
||||
severity: isGatewayRemotelyExposed(cfg) ? "critical" : "warn",
|
||||
title: "Dangerous node commands explicitly enabled",
|
||||
detail:
|
||||
`gateway.nodes.allowCommands includes: ${dangerousAllowed.join(", ")}. ` +
|
||||
"These commands can trigger high-impact device actions (camera/screen/contacts/calendar/reminders/SMS).",
|
||||
remediation:
|
||||
"Remove these entries from gateway.nodes.allowCommands (recommended). " +
|
||||
"If you keep them, treat gateway auth as full operator access and keep gateway exposure local/tailnet-only.",
|
||||
});
|
||||
|
||||
return findings;
|
||||
}
|
||||
|
||||
export function collectMinimalProfileOverrideFindings(cfg: OpenClawConfig): SecurityAuditFinding[] {
|
||||
const findings: SecurityAuditFinding[] = [];
|
||||
if (cfg.tools?.profile !== "minimal") {
|
||||
|
||||
@@ -16,6 +16,7 @@ export {
|
||||
collectHooksHardeningFindings,
|
||||
collectMinimalProfileOverrideFindings,
|
||||
collectModelHygieneFindings,
|
||||
collectNodeDangerousAllowCommandFindings,
|
||||
collectNodeDenyCommandPatternFindings,
|
||||
collectSandboxDangerousConfigFindings,
|
||||
collectSandboxDockerNoopFindings,
|
||||
|
||||
@@ -767,6 +767,59 @@ describe("security audit", () => {
|
||||
expect(finding?.detail).toContain("system.runx");
|
||||
});
|
||||
|
||||
it("scores dangerous gateway.nodes.allowCommands by exposure", async () => {
|
||||
const cases: Array<{
|
||||
name: string;
|
||||
cfg: OpenClawConfig;
|
||||
expectedSeverity: "warn" | "critical";
|
||||
}> = [
|
||||
{
|
||||
name: "loopback gateway",
|
||||
cfg: {
|
||||
gateway: {
|
||||
bind: "loopback",
|
||||
nodes: { allowCommands: ["camera.snap", "screen.record"] },
|
||||
},
|
||||
},
|
||||
expectedSeverity: "warn",
|
||||
},
|
||||
{
|
||||
name: "lan-exposed gateway",
|
||||
cfg: {
|
||||
gateway: {
|
||||
bind: "lan",
|
||||
nodes: { allowCommands: ["camera.snap", "screen.record"] },
|
||||
},
|
||||
},
|
||||
expectedSeverity: "critical",
|
||||
},
|
||||
];
|
||||
|
||||
for (const testCase of cases) {
|
||||
const res = await audit(testCase.cfg);
|
||||
const finding = res.findings.find(
|
||||
(f) => f.checkId === "gateway.nodes.allow_commands_dangerous",
|
||||
);
|
||||
expect(finding?.severity, testCase.name).toBe(testCase.expectedSeverity);
|
||||
expect(finding?.detail, testCase.name).toContain("camera.snap");
|
||||
expect(finding?.detail, testCase.name).toContain("screen.record");
|
||||
}
|
||||
});
|
||||
|
||||
it("does not flag dangerous allowCommands entries when denied again", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
nodes: {
|
||||
allowCommands: ["camera.snap", "screen.record"],
|
||||
denyCommands: ["camera.snap", "screen.record"],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await audit(cfg);
|
||||
expectNoFinding(res, "gateway.nodes.allow_commands_dangerous");
|
||||
});
|
||||
|
||||
it("flags agent profile overrides when global tools.profile is minimal", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
tools: {
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
collectSandboxBrowserHashLabelFindings,
|
||||
collectMinimalProfileOverrideFindings,
|
||||
collectModelHygieneFindings,
|
||||
collectNodeDangerousAllowCommandFindings,
|
||||
collectNodeDenyCommandPatternFindings,
|
||||
collectSmallModelRiskFindings,
|
||||
collectSandboxDangerousConfigFindings,
|
||||
@@ -717,6 +718,7 @@ export async function runSecurityAudit(opts: SecurityAuditOptions): Promise<Secu
|
||||
findings.push(...collectSandboxDockerNoopFindings(cfg));
|
||||
findings.push(...collectSandboxDangerousConfigFindings(cfg));
|
||||
findings.push(...collectNodeDenyCommandPatternFindings(cfg));
|
||||
findings.push(...collectNodeDangerousAllowCommandFindings(cfg));
|
||||
findings.push(...collectMinimalProfileOverrideFindings(cfg));
|
||||
findings.push(...collectSecretsInConfigFindings(cfg));
|
||||
findings.push(...collectModelHygieneFindings(cfg));
|
||||
|
||||
Reference in New Issue
Block a user