mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
refactor: harden control-ui auth flow and add insecure-flag audit summary
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -72,6 +72,40 @@ type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;
|
||||
|
||||
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) {
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -114,6 +114,30 @@ function normalizeAllowFromList(list: Array<string | number> | 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) {
|
||||
|
||||
Reference in New Issue
Block a user