mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 11:01:24 +00:00
Security: Prevent gateway credential exfiltration via URL override (#9179)
* Gateway: require explicit auth for url overrides * Gateway: scope credential blocking to non-local URLs only Address review feedback: the previous fix blocked credential fallback for ALL URL overrides, which was overly strict and could break workflows that use --url to switch between loopback/tailnet without passing credentials. Now credential fallback is only blocked for non-local URLs (public IPs, external hostnames). Local addresses (127.0.0.1, localhost, private IPs like 192.168.x.x, 10.x.x.x, tailnet 100.x.x.x) still get credential fallback as before. This maintains the security fix (preventing credential exfiltration to attacker-controlled URLs) while preserving backward compatibility for legitimate local URL overrides. * Security: require explicit credentials for gateway url overrides (#8113) (thanks @victormier) * Gateway: reuse explicit auth helper for url overrides (#8113) (thanks @victormier) * Tests: format gateway chat test (#8113) (thanks @victormier) * Tests: require explicit auth for gateway url overrides (#8113) (thanks @victormier) --------- Co-authored-by: Victor Mier <victormier@gmail.com>
This commit is contained in:
Gustavo Madeira Santana
committed by
GitHub
GitHub
parent
96abc1c864
commit
a13ff55bd9
@@ -113,9 +113,14 @@ describe("callGateway url resolution", () => {
|
||||
resolveGatewayPort.mockReturnValue(18789);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
|
||||
await callGateway({ method: "health", url: "wss://override.example/ws" });
|
||||
await callGateway({
|
||||
method: "health",
|
||||
url: "wss://override.example/ws",
|
||||
token: "explicit-token",
|
||||
});
|
||||
|
||||
expect(lastClientOptions?.url).toBe("wss://override.example/ws");
|
||||
expect(lastClientOptions?.token).toBe("explicit-token");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -257,6 +262,40 @@ describe("callGateway error details", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("callGateway url override auth requirements", () => {
|
||||
beforeEach(() => {
|
||||
loadConfig.mockReset();
|
||||
resolveGatewayPort.mockReset();
|
||||
pickPrimaryTailnetIPv4.mockReset();
|
||||
lastClientOptions = null;
|
||||
startMode = "hello";
|
||||
closeCode = 1006;
|
||||
closeReason = "";
|
||||
resolveGatewayPort.mockReturnValue(18789);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.OPENCLAW_GATEWAY_TOKEN;
|
||||
delete process.env.OPENCLAW_GATEWAY_PASSWORD;
|
||||
});
|
||||
|
||||
it("throws when url override is set without explicit credentials", async () => {
|
||||
process.env.OPENCLAW_GATEWAY_TOKEN = "env-token";
|
||||
process.env.OPENCLAW_GATEWAY_PASSWORD = "env-password";
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: {
|
||||
mode: "local",
|
||||
auth: { token: "local-token", password: "local-password" },
|
||||
},
|
||||
});
|
||||
|
||||
await expect(
|
||||
callGateway({ method: "health", url: "wss://override.example/ws" }),
|
||||
).rejects.toThrow("explicit credentials");
|
||||
});
|
||||
});
|
||||
|
||||
describe("callGateway password resolution", () => {
|
||||
const originalEnvPassword = process.env.OPENCLAW_GATEWAY_PASSWORD;
|
||||
|
||||
@@ -338,6 +377,24 @@ describe("callGateway password resolution", () => {
|
||||
|
||||
expect(lastClientOptions?.password).toBe("from-env");
|
||||
});
|
||||
|
||||
it("uses explicit password when url override is set", async () => {
|
||||
process.env.OPENCLAW_GATEWAY_PASSWORD = "from-env";
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: {
|
||||
mode: "local",
|
||||
auth: { password: "from-config" },
|
||||
},
|
||||
});
|
||||
|
||||
await callGateway({
|
||||
method: "health",
|
||||
url: "wss://override.example/ws",
|
||||
password: "explicit-password",
|
||||
});
|
||||
|
||||
expect(lastClientOptions?.password).toBe("explicit-password");
|
||||
});
|
||||
});
|
||||
|
||||
describe("callGateway token resolution", () => {
|
||||
@@ -364,18 +421,21 @@ describe("callGateway token resolution", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("uses remote token when remote mode uses url override", async () => {
|
||||
it("uses explicit token when url override is set", async () => {
|
||||
process.env.OPENCLAW_GATEWAY_TOKEN = "env-token";
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: { token: "remote-token" },
|
||||
mode: "local",
|
||||
auth: { token: "local-token" },
|
||||
},
|
||||
});
|
||||
|
||||
await callGateway({ method: "health", url: "wss://override.example/ws" });
|
||||
await callGateway({
|
||||
method: "health",
|
||||
url: "wss://override.example/ws",
|
||||
token: "explicit-token",
|
||||
});
|
||||
|
||||
expect(lastClientOptions?.token).toBe("remote-token");
|
||||
expect(lastClientOptions?.token).toBe("explicit-token");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,6 +51,43 @@ export type GatewayConnectionDetails = {
|
||||
message: string;
|
||||
};
|
||||
|
||||
export type ExplicitGatewayAuth = {
|
||||
token?: string;
|
||||
password?: string;
|
||||
};
|
||||
|
||||
export function resolveExplicitGatewayAuth(opts?: ExplicitGatewayAuth): ExplicitGatewayAuth {
|
||||
const token =
|
||||
typeof opts?.token === "string" && opts.token.trim().length > 0 ? opts.token.trim() : undefined;
|
||||
const password =
|
||||
typeof opts?.password === "string" && opts.password.trim().length > 0
|
||||
? opts.password.trim()
|
||||
: undefined;
|
||||
return { token, password };
|
||||
}
|
||||
|
||||
export function ensureExplicitGatewayAuth(params: {
|
||||
urlOverride?: string;
|
||||
auth: ExplicitGatewayAuth;
|
||||
errorHint: string;
|
||||
configPath?: string;
|
||||
}): void {
|
||||
if (!params.urlOverride) {
|
||||
return;
|
||||
}
|
||||
if (params.auth.token || params.auth.password) {
|
||||
return;
|
||||
}
|
||||
const message = [
|
||||
"gateway url override requires explicit credentials",
|
||||
params.errorHint,
|
||||
params.configPath ? `Config: ${params.configPath}` : undefined,
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join("\n");
|
||||
throw new Error(message);
|
||||
}
|
||||
|
||||
export function buildGatewayConnectionDetails(
|
||||
options: { config?: OpenClawConfig; url?: string; configPath?: string } = {},
|
||||
): GatewayConnectionDetails {
|
||||
@@ -118,6 +155,13 @@ export async function callGateway<T = Record<string, unknown>>(
|
||||
const remote = isRemoteMode ? config.gateway?.remote : undefined;
|
||||
const urlOverride =
|
||||
typeof opts.url === "string" && opts.url.trim().length > 0 ? opts.url.trim() : undefined;
|
||||
const explicitAuth = resolveExplicitGatewayAuth({ token: opts.token, password: opts.password });
|
||||
ensureExplicitGatewayAuth({
|
||||
urlOverride,
|
||||
auth: explicitAuth,
|
||||
errorHint: "Fix: pass --token or --password (or gatewayToken in tools).",
|
||||
configPath: opts.configPath ?? resolveConfigPath(process.env, resolveStateDir(process.env)),
|
||||
});
|
||||
const remoteUrl =
|
||||
typeof remote?.url === "string" && remote.url.trim().length > 0 ? remote.url.trim() : undefined;
|
||||
if (isRemoteMode && !urlOverride && !remoteUrl) {
|
||||
@@ -153,31 +197,31 @@ export async function callGateway<T = Record<string, unknown>>(
|
||||
remoteTlsFingerprint ||
|
||||
(tlsRuntime?.enabled ? tlsRuntime.fingerprintSha256 : undefined);
|
||||
const token =
|
||||
(typeof opts.token === "string" && opts.token.trim().length > 0
|
||||
? opts.token.trim()
|
||||
: undefined) ||
|
||||
(isRemoteMode
|
||||
? typeof remote?.token === "string" && remote.token.trim().length > 0
|
||||
? remote.token.trim()
|
||||
: undefined
|
||||
: process.env.OPENCLAW_GATEWAY_TOKEN?.trim() ||
|
||||
process.env.CLAWDBOT_GATEWAY_TOKEN?.trim() ||
|
||||
(typeof authToken === "string" && authToken.trim().length > 0
|
||||
? authToken.trim()
|
||||
: undefined));
|
||||
explicitAuth.token ||
|
||||
(!urlOverride
|
||||
? isRemoteMode
|
||||
? typeof remote?.token === "string" && remote.token.trim().length > 0
|
||||
? remote.token.trim()
|
||||
: undefined
|
||||
: process.env.OPENCLAW_GATEWAY_TOKEN?.trim() ||
|
||||
process.env.CLAWDBOT_GATEWAY_TOKEN?.trim() ||
|
||||
(typeof authToken === "string" && authToken.trim().length > 0
|
||||
? authToken.trim()
|
||||
: undefined)
|
||||
: undefined);
|
||||
const password =
|
||||
(typeof opts.password === "string" && opts.password.trim().length > 0
|
||||
? opts.password.trim()
|
||||
: undefined) ||
|
||||
process.env.OPENCLAW_GATEWAY_PASSWORD?.trim() ||
|
||||
process.env.CLAWDBOT_GATEWAY_PASSWORD?.trim() ||
|
||||
(isRemoteMode
|
||||
? typeof remote?.password === "string" && remote.password.trim().length > 0
|
||||
? remote.password.trim()
|
||||
: undefined
|
||||
: typeof authPassword === "string" && authPassword.trim().length > 0
|
||||
? authPassword.trim()
|
||||
: undefined);
|
||||
explicitAuth.password ||
|
||||
(!urlOverride
|
||||
? process.env.OPENCLAW_GATEWAY_PASSWORD?.trim() ||
|
||||
process.env.CLAWDBOT_GATEWAY_PASSWORD?.trim() ||
|
||||
(isRemoteMode
|
||||
? typeof remote?.password === "string" && remote.password.trim().length > 0
|
||||
? remote.password.trim()
|
||||
: undefined
|
||||
: typeof authPassword === "string" && authPassword.trim().length > 0
|
||||
? authPassword.trim()
|
||||
: undefined)
|
||||
: undefined);
|
||||
|
||||
const formatCloseError = (code: number, reason: string) => {
|
||||
const reasonText = reason?.trim() || "no close reason";
|
||||
|
||||
Reference in New Issue
Block a user