diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index 467cdb371a9..d30f5f8c708 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -127,8 +127,9 @@ High-signal `checkId` values you will most likely see in real deployments (not e | `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` | critical | Insecure-auth toggle enabled | `gateway.controlUi.allowInsecureAuth` | 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 | @@ -153,6 +154,16 @@ keep it off unless you are actively debugging and can revert quickly. `openclaw security audit` warns when this setting is enabled. +## Insecure or dangerous flags summary + +`openclaw security audit` includes `config.insecure_or_dangerous_flags` when any +insecure/dangerous debug switches are enabled. This warning aggregates the exact +keys so you can review them in one place (for example +`gateway.controlUi.allowInsecureAuth=true`, +`gateway.controlUi.dangerouslyDisableDeviceAuth=true`, +`hooks.gmail.allowUnsafeExternalContent=true`, or +`tools.exec.applyPatch.workspaceOnly=false`). + ## Reverse Proxy Configuration If you run the Gateway behind a reverse proxy (nginx, Caddy, Traefik, etc.), you should configure `gateway.trustedProxies` for proper client IP detection. diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index 61be61fafdd..92a3d1e5c40 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -72,6 +72,40 @@ type SubsystemLogger = ReturnType; const DEVICE_SIGNATURE_SKEW_MS = 10 * 60 * 1000; +type ControlUiAuthPolicy = { + allowInsecureAuthConfigured: boolean; + dangerouslyDisableDeviceAuth: boolean; + allowBypass: boolean; + device: ConnectParams["device"] | null | undefined; +}; + +function resolveControlUiAuthPolicy(params: { + isControlUi: boolean; + controlUiConfig: + | { + allowInsecureAuth?: boolean; + dangerouslyDisableDeviceAuth?: boolean; + } + | undefined; + deviceRaw: ConnectParams["device"] | null | undefined; +}): ControlUiAuthPolicy { + const allowInsecureAuthConfigured = + params.isControlUi && params.controlUiConfig?.allowInsecureAuth === true; + const dangerouslyDisableDeviceAuth = + params.isControlUi && params.controlUiConfig?.dangerouslyDisableDeviceAuth === true; + return { + allowInsecureAuthConfigured, + dangerouslyDisableDeviceAuth, + // `allowInsecureAuth` must not bypass secure-context/device-auth requirements. + allowBypass: dangerouslyDisableDeviceAuth, + device: dangerouslyDisableDeviceAuth ? null : params.deviceRaw, + }; +} + +function shouldSkipControlUiPairing(policy: ControlUiAuthPolicy, sharedAuthOk: boolean): boolean { + return policy.allowBypass && sharedAuthOk; +} + export function attachGatewayWsMessageHandler(params: { socket: WebSocket; upgradeReq: IncomingMessage; @@ -337,62 +371,74 @@ export function attachGatewayWsMessageHandler(params: { const hasTokenAuth = Boolean(connectParams.auth?.token); const hasPasswordAuth = Boolean(connectParams.auth?.password); const hasSharedAuth = hasTokenAuth || hasPasswordAuth; - const allowInsecureControlUi = - isControlUi && configSnapshot.gateway?.controlUi?.allowInsecureAuth === true; - const disableControlUiDeviceAuth = - isControlUi && configSnapshot.gateway?.controlUi?.dangerouslyDisableDeviceAuth === true; - // `allowInsecureAuth` must not bypass secure-context/device-auth requirements. - const allowControlUiBypass = disableControlUiDeviceAuth; - const device = disableControlUiDeviceAuth ? null : deviceRaw; - - const hasDeviceTokenCandidate = Boolean(connectParams.auth?.token && device); - let authResult: GatewayAuthResult = await authorizeGatewayConnect({ - auth: resolvedAuth, - connectAuth: connectParams.auth, - req: upgradeReq, - trustedProxies, - allowTailscaleHeaderAuth: true, - rateLimiter: hasDeviceTokenCandidate ? undefined : rateLimiter, - clientIp, - rateLimitScope: AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, + const controlUiAuthPolicy = resolveControlUiAuthPolicy({ + isControlUi, + controlUiConfig: configSnapshot.gateway?.controlUi, + deviceRaw, }); + const device = controlUiAuthPolicy.device; - if ( - hasDeviceTokenCandidate && - authResult.ok && - rateLimiter && - (authResult.method === "token" || authResult.method === "password") - ) { - const sharedRateCheck = rateLimiter.check(clientIp, AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET); - if (!sharedRateCheck.allowed) { - authResult = { - ok: false, - reason: "rate_limited", - rateLimited: true, - retryAfterMs: sharedRateCheck.retryAfterMs, - }; - } else { - rateLimiter.reset(clientIp, AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET); + const resolveAuthState = async () => { + const hasDeviceTokenCandidate = Boolean(connectParams.auth?.token && device); + let nextAuthResult: GatewayAuthResult = await authorizeGatewayConnect({ + auth: resolvedAuth, + connectAuth: connectParams.auth, + req: upgradeReq, + trustedProxies, + allowTailscaleHeaderAuth: true, + rateLimiter: hasDeviceTokenCandidate ? undefined : rateLimiter, + clientIp, + rateLimitScope: AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, + }); + + if ( + hasDeviceTokenCandidate && + nextAuthResult.ok && + rateLimiter && + (nextAuthResult.method === "token" || nextAuthResult.method === "password") + ) { + const sharedRateCheck = rateLimiter.check( + clientIp, + AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, + ); + if (!sharedRateCheck.allowed) { + nextAuthResult = { + ok: false, + reason: "rate_limited", + rateLimited: true, + retryAfterMs: sharedRateCheck.retryAfterMs, + }; + } else { + rateLimiter.reset(clientIp, AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET); + } } - } - let authOk = authResult.ok; - let authMethod = - authResult.method ?? (resolvedAuth.mode === "password" ? "password" : "token"); - const sharedAuthResult = hasSharedAuth - ? await authorizeGatewayConnect({ - auth: { ...resolvedAuth, allowTailscale: false }, - connectAuth: connectParams.auth, - req: upgradeReq, - trustedProxies, - // Shared-auth probe only; rate-limit side effects are handled in - // the primary auth flow (or deferred for device-token candidates). - rateLimitScope: AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, - }) - : null; - const sharedAuthOk = - sharedAuthResult?.ok === true && - (sharedAuthResult.method === "token" || sharedAuthResult.method === "password"); + const nextAuthMethod = + nextAuthResult.method ?? (resolvedAuth.mode === "password" ? "password" : "token"); + const sharedAuthResult = hasSharedAuth + ? await authorizeGatewayConnect({ + auth: { ...resolvedAuth, allowTailscale: false }, + connectAuth: connectParams.auth, + req: upgradeReq, + trustedProxies, + // Shared-auth probe only; rate-limit side effects are handled in + // the primary auth flow (or deferred for device-token candidates). + rateLimitScope: AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, + }) + : null; + const nextSharedAuthOk = + sharedAuthResult?.ok === true && + (sharedAuthResult.method === "token" || sharedAuthResult.method === "password"); + + return { + authResult: nextAuthResult, + authOk: nextAuthResult.ok, + authMethod: nextAuthMethod, + sharedAuthOk: nextSharedAuthOk, + }; + }; + + let { authResult, authOk, authMethod, sharedAuthOk } = await resolveAuthState(); const rejectUnauthorized = (failedAuth: GatewayAuthResult) => { markHandshakeFailure("unauthorized", { authMode: resolvedAuth.mode, @@ -421,35 +467,46 @@ export function attachGatewayWsMessageHandler(params: { sendHandshakeErrorResponse(ErrorCodes.INVALID_REQUEST, authMessage); close(1008, truncateCloseReason(authMessage)); }; - if (!device) { - if (scopes.length > 0 && !allowControlUiBypass) { + const clearUnboundScopes = () => { + if (scopes.length > 0 && !controlUiAuthPolicy.allowBypass) { scopes = []; connectParams.scopes = scopes; } + }; + const handleMissingDeviceIdentity = (): boolean => { + if (device) { + return true; + } + clearUnboundScopes(); const canSkipDevice = sharedAuthOk; - if (isControlUi && !allowControlUiBypass) { + if (isControlUi && !controlUiAuthPolicy.allowBypass) { const errorMessage = "control ui requires device identity (use HTTPS or localhost secure context)"; markHandshakeFailure("control-ui-insecure-auth", { - insecureAuthConfigured: allowInsecureControlUi, + insecureAuthConfigured: controlUiAuthPolicy.allowInsecureAuthConfigured, }); sendHandshakeErrorResponse(ErrorCodes.INVALID_REQUEST, errorMessage); close(1008, errorMessage); - return; + return false; } - // Allow shared-secret authenticated connections (e.g., control-ui) to skip device identity + // Allow shared-secret authenticated connections (e.g., control-ui) to skip device identity. if (!canSkipDevice) { if (!authOk && hasSharedAuth) { rejectUnauthorized(authResult); - return; + return false; } markHandshakeFailure("device-required"); sendHandshakeErrorResponse(ErrorCodes.NOT_PAIRED, "device identity required"); close(1008, "device identity required"); - return; + return false; } + + return true; + }; + if (!handleMissingDeviceIdentity()) { + return; } if (device) { const derivedId = deriveDeviceIdFromPublicKey(device.publicKey); @@ -625,7 +682,7 @@ export function attachGatewayWsMessageHandler(params: { return; } - const skipPairing = allowControlUiBypass && sharedAuthOk; + const skipPairing = shouldSkipControlUiPairing(controlUiAuthPolicy, sharedAuthOk); if (device && devicePublicKey && !skipPairing) { const formatAuditList = (items: string[] | undefined): string => { if (!items || items.length === 0) { diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 3585a7b69a3..8e6a04725a9 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -707,7 +707,12 @@ describe("security audit", () => { expect.arrayContaining([ expect.objectContaining({ checkId: "gateway.control_ui.insecure_auth", - severity: "critical", + severity: "warn", + }), + expect.objectContaining({ + checkId: "config.insecure_or_dangerous_flags", + severity: "warn", + detail: expect.stringContaining("gateway.controlUi.allowInsecureAuth=true"), }), ]), ); @@ -728,10 +733,40 @@ describe("security audit", () => { checkId: "gateway.control_ui.device_auth_disabled", severity: "critical", }), + expect.objectContaining({ + checkId: "config.insecure_or_dangerous_flags", + severity: "warn", + detail: expect.stringContaining("gateway.controlUi.dangerouslyDisableDeviceAuth=true"), + }), ]), ); }); + it("warns when insecure/dangerous debug flags are enabled", async () => { + const cfg: OpenClawConfig = { + hooks: { + gmail: { allowUnsafeExternalContent: true }, + mappings: [{ allowUnsafeExternalContent: true }], + }, + tools: { + exec: { + applyPatch: { + workspaceOnly: false, + }, + }, + }, + }; + + const res = await audit(cfg); + const finding = res.findings.find((f) => f.checkId === "config.insecure_or_dangerous_flags"); + + expect(finding).toBeTruthy(); + expect(finding?.severity).toBe("warn"); + expect(finding?.detail).toContain("hooks.gmail.allowUnsafeExternalContent=true"); + expect(finding?.detail).toContain("hooks.mappings[0].allowUnsafeExternalContent=true"); + expect(finding?.detail).toContain("tools.exec.applyPatch.workspaceOnly=false"); + }); + it("flags trusted-proxy auth mode without generic shared-secret findings", async () => { const cfg: OpenClawConfig = { gateway: { diff --git a/src/security/audit.ts b/src/security/audit.ts index 294110158f4..8d4b3427f13 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -114,6 +114,30 @@ function normalizeAllowFromList(list: Array | undefined | null) return list.map((v) => String(v).trim()).filter(Boolean); } +function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] { + const enabledFlags: string[] = []; + if (cfg.gateway?.controlUi?.allowInsecureAuth === true) { + enabledFlags.push("gateway.controlUi.allowInsecureAuth=true"); + } + if (cfg.gateway?.controlUi?.dangerouslyDisableDeviceAuth === true) { + enabledFlags.push("gateway.controlUi.dangerouslyDisableDeviceAuth=true"); + } + if (cfg.hooks?.gmail?.allowUnsafeExternalContent === true) { + enabledFlags.push("hooks.gmail.allowUnsafeExternalContent=true"); + } + if (Array.isArray(cfg.hooks?.mappings)) { + for (const [index, mapping] of cfg.hooks.mappings.entries()) { + if (mapping?.allowUnsafeExternalContent === true) { + enabledFlags.push(`hooks.mappings[${index}].allowUnsafeExternalContent=true`); + } + } + } + if (cfg.tools?.exec?.applyPatch?.workspaceOnly === false) { + enabledFlags.push("tools.exec.applyPatch.workspaceOnly=false"); + } + return enabledFlags; +} + async function collectFilesystemFindings(params: { stateDir: string; configPath: string; @@ -348,7 +372,7 @@ function collectGatewayConfigFindings( if (cfg.gateway?.controlUi?.allowInsecureAuth === true) { findings.push({ checkId: "gateway.control_ui.insecure_auth", - severity: "critical", + severity: "warn", title: "Control UI insecure auth toggle enabled", detail: "gateway.controlUi.allowInsecureAuth=true does not bypass secure context or device identity checks; only dangerouslyDisableDeviceAuth disables Control UI device identity checks.", @@ -367,6 +391,18 @@ function collectGatewayConfigFindings( }); } + const enabledDangerousFlags = collectEnabledInsecureOrDangerousFlags(cfg); + if (enabledDangerousFlags.length > 0) { + findings.push({ + checkId: "config.insecure_or_dangerous_flags", + severity: "warn", + title: "Insecure or dangerous config flags enabled", + detail: `Detected ${enabledDangerousFlags.length} enabled flag(s): ${enabledDangerousFlags.join(", ")}.`, + remediation: + "Disable these flags when not actively debugging, or keep deployment scoped to trusted/local-only networks.", + }); + } + const token = typeof auth.token === "string" && auth.token.trim().length > 0 ? auth.token.trim() : null; if (auth.mode === "token" && token && token.length < 24) {