mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-06 06:41:08 +00:00
fix(gateway): skip device pairing for authenticated CLI connections in Docker
CLI connections with valid shared auth (token/password) now bypass device pairing, fixing the chicken-and-egg problem where Docker CLI commands fail with 'pairing required' (1008) despite sharing the gateway's network namespace and auth token. The existing shouldSkipBackendSelfPairing only matched gateway-client/backend mode. CLI connections use cli/cli mode and were excluded. Additionally, isLocalDirectRequest produces false negatives in Docker (host networking, network_mode sharing) even when remoteAddress is 127.0.0.1, so CLI connections with valid shared auth skip the locality check entirely — the token is the trust anchor. Closes #55067 Related: #12210, #23471, #30740
This commit is contained in:
committed by
Peter Steinberger
parent
2b538464e1
commit
224fceee1a
@@ -1,10 +1,14 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { AuthRateLimiter } from "../../auth-rate-limit.js";
|
||||
import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../protocol/client-info.js";
|
||||
import type { ConnectParams } from "../../protocol/schema/types.js";
|
||||
import {
|
||||
BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP,
|
||||
resolveHandshakeBrowserSecurityContext,
|
||||
resolveUnauthorizedHandshakeContext,
|
||||
shouldAllowSilentLocalPairing,
|
||||
shouldSkipBackendSelfPairing,
|
||||
shouldTreatCliContainerHostAsLocal,
|
||||
} from "./handshake-auth-helpers.js";
|
||||
|
||||
function createRateLimiter(): AuthRateLimiter {
|
||||
@@ -103,7 +107,6 @@ describe("handshake auth helpers", () => {
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects silent role-upgrade for remote clients", () => {
|
||||
expect(
|
||||
shouldAllowSilentLocalPairing({
|
||||
@@ -115,4 +118,167 @@ describe("handshake auth helpers", () => {
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("treats CLI loopback/private-host connects as local only with shared auth", () => {
|
||||
const connectParams = {
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_IDS.CLI,
|
||||
mode: GATEWAY_CLIENT_MODES.CLI,
|
||||
},
|
||||
} as ConnectParams;
|
||||
expect(
|
||||
shouldTreatCliContainerHostAsLocal({
|
||||
connectParams,
|
||||
requestHost: "172.17.0.2:18789",
|
||||
remoteAddress: "127.0.0.1",
|
||||
hasProxyHeaders: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldTreatCliContainerHostAsLocal({
|
||||
connectParams,
|
||||
requestHost: "172.17.0.2:18789",
|
||||
remoteAddress: "127.0.0.1",
|
||||
hasProxyHeaders: true,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldTreatCliContainerHostAsLocal({
|
||||
connectParams,
|
||||
requestHost: "gateway.example",
|
||||
remoteAddress: "127.0.0.1",
|
||||
hasProxyHeaders: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldTreatCliContainerHostAsLocal({
|
||||
connectParams,
|
||||
requestHost: "172.17.0.2:18789",
|
||||
remoteAddress: "127.0.0.1",
|
||||
hasProxyHeaders: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "device-token",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not treat non-CLI clients as Docker-local CLI bypass candidates", () => {
|
||||
const connectParams = {
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT,
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
},
|
||||
} as ConnectParams;
|
||||
expect(
|
||||
shouldTreatCliContainerHostAsLocal({
|
||||
connectParams,
|
||||
requestHost: "172.17.0.2:18789",
|
||||
remoteAddress: "127.0.0.1",
|
||||
hasProxyHeaders: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("skips backend self-pairing only for local backend clients", () => {
|
||||
const connectParams = {
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT,
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
},
|
||||
} as ConnectParams;
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: true,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "password",
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: true,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: false,
|
||||
authMethod: "device-token",
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: false,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: false,
|
||||
authMethod: "device-token",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not skip backend self-pairing for CLI clients", () => {
|
||||
const connectParams = {
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_IDS.CLI,
|
||||
mode: GATEWAY_CLIENT_MODES.CLI,
|
||||
},
|
||||
} as ConnectParams;
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: true,
|
||||
hasBrowserOriginHeader: false,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects pairing bypass when browser origin header is present", () => {
|
||||
const connectParams = {
|
||||
client: {
|
||||
id: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT,
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
},
|
||||
} as ConnectParams;
|
||||
expect(
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient: true,
|
||||
hasBrowserOriginHeader: true,
|
||||
sharedAuthOk: true,
|
||||
authMethod: "token",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { AuthRateLimiter } from "../../auth-rate-limit.js";
|
||||
import type { GatewayAuthResult } from "../../auth.js";
|
||||
import { buildDeviceAuthPayload, buildDeviceAuthPayloadV3 } from "../../device-auth.js";
|
||||
import { isLoopbackAddress } from "../../net.js";
|
||||
import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../../protocol/client-info.js";
|
||||
import type { ConnectParams } from "../../protocol/index.js";
|
||||
import type { AuthProvidedKind } from "./auth-messages.js";
|
||||
|
||||
@@ -61,6 +62,44 @@ export function shouldAllowSilentLocalPairing(params: {
|
||||
);
|
||||
}
|
||||
|
||||
export function shouldSkipBackendSelfPairing(params: {
|
||||
connectParams: ConnectParams;
|
||||
isLocalClient: boolean;
|
||||
hasBrowserOriginHeader: boolean;
|
||||
sharedAuthOk: boolean;
|
||||
authMethod: GatewayAuthResult["method"];
|
||||
}): boolean {
|
||||
const isLocalTrustedClient =
|
||||
(params.connectParams.client.id === GATEWAY_CLIENT_IDS.GATEWAY_CLIENT &&
|
||||
params.connectParams.client.mode === GATEWAY_CLIENT_MODES.BACKEND) ||
|
||||
(params.connectParams.client.id === GATEWAY_CLIENT_IDS.CLI &&
|
||||
params.connectParams.client.mode === GATEWAY_CLIENT_MODES.CLI);
|
||||
if (!isLocalTrustedClient) {
|
||||
return false;
|
||||
}
|
||||
const usesSharedSecretAuth = params.authMethod === "token" || params.authMethod === "password";
|
||||
const usesDeviceTokenAuth = params.authMethod === "device-token";
|
||||
// `authMethod === "device-token"` only reaches this helper after the caller
|
||||
// has already accepted auth (`authOk === true`), so a separate
|
||||
// `deviceTokenAuthOk` flag would be redundant here.
|
||||
//
|
||||
// For any trusted client identity (backend or CLI) with a valid shared
|
||||
// secret (token/password) and no browser Origin header, skip pairing
|
||||
// regardless of isLocalClient. The shared secret is the trust anchor;
|
||||
// isLocalDirectRequest() produces false negatives in Docker containers
|
||||
// that share the gateway's network namespace via network_mode, even when
|
||||
// remoteAddress is 127.0.0.1.
|
||||
if (params.sharedAuthOk && usesSharedSecretAuth && !params.hasBrowserOriginHeader) {
|
||||
return true;
|
||||
}
|
||||
// For device-token auth, retain the locality check as defense-in-depth.
|
||||
return (
|
||||
params.isLocalClient &&
|
||||
!params.hasBrowserOriginHeader &&
|
||||
usesDeviceTokenAuth
|
||||
);
|
||||
}
|
||||
|
||||
function resolveSignatureToken(connectParams: ConnectParams): string | null {
|
||||
return (
|
||||
connectParams.auth?.token ??
|
||||
|
||||
@@ -109,6 +109,7 @@ import {
|
||||
resolveHandshakeBrowserSecurityContext,
|
||||
resolveUnauthorizedHandshakeContext,
|
||||
shouldAllowSilentLocalPairing,
|
||||
shouldSkipBackendSelfPairing,
|
||||
} from "./handshake-auth-helpers.js";
|
||||
import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized-flood-guard.js";
|
||||
|
||||
@@ -247,6 +248,7 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
const hasUntrustedProxyHeaders = hasProxyHeaders && !remoteIsTrustedProxy;
|
||||
const hostIsLocalish = isLocalishHost(requestHost);
|
||||
const isLocalClient = isLocalDirectRequest(upgradeReq, trustedProxies, allowRealIpFallback);
|
||||
|
||||
const reportedClientIp =
|
||||
isLocalClient || hasUntrustedProxyHeaders
|
||||
? undefined
|
||||
@@ -723,12 +725,20 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
authOk,
|
||||
authMethod,
|
||||
});
|
||||
const skipPairing = shouldSkipControlUiPairing(
|
||||
controlUiAuthPolicy,
|
||||
role,
|
||||
trustedProxyAuthOk,
|
||||
resolvedAuth.mode,
|
||||
);
|
||||
const skipPairing =
|
||||
shouldSkipBackendSelfPairing({
|
||||
connectParams,
|
||||
isLocalClient,
|
||||
hasBrowserOriginHeader,
|
||||
sharedAuthOk,
|
||||
authMethod,
|
||||
}) ||
|
||||
shouldSkipControlUiPairing(
|
||||
controlUiAuthPolicy,
|
||||
role,
|
||||
trustedProxyAuthOk,
|
||||
resolvedAuth.mode,
|
||||
);
|
||||
if (device && devicePublicKey && !skipPairing) {
|
||||
const formatAuditList = (items: string[] | undefined): string => {
|
||||
if (!items || items.length === 0) {
|
||||
|
||||
Reference in New Issue
Block a user