mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(browser): scope control auth to active gateway mode (#65639)
Browser control now authorizes only the resolved active gateway credential and fails closed when password mode lacks a resolved password. Also removes the duplicate Slack test-helper middleware stub that kept current CI red after the base rebase. Fixes #65626. Co-authored-by: Coy Geek <65363919+coygeek@users.noreply.github.com>
This commit is contained in:
@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
- Slack/exec approvals: resolve native approval button clicks over the Gateway instead of delivering `/approve ...` as plain agent text, preserving retry buttons if Gateway resolution fails. Fixes #71023. (#71025) Thanks @marusan03.
|
||||
- Slack/files: return non-image `download-file` results as local file paths instead of image payloads, and include Slack file IDs in inbound file placeholders so agents can call `download-file`. Fixes #71212. Thanks @teamrazo.
|
||||
- Browser control: scope standalone loopback auth to the resolved active gateway credential and fail closed when password mode lacks a resolved password, so inactive tokens or passwords no longer authorize browser routes. Fixes #65626. (#65639) Thanks @coygeek.
|
||||
- Discord/replies: run `message_sending` plugin hooks for Discord reply delivery, including DM targets, so plugins can transform or cancel outbound Discord replies consistently with other channels. Fixes #59350. (#71094) Thanks @wei840222.
|
||||
- Control UI/commands: carry provider-owned thinking option ids/labels in session rows and defaults so fresh sessions show and accept dynamic modes such as `adaptive`, `xhigh`, and `max`. Fixes #71269. Thanks @Young-Khalil.
|
||||
- Image generation: make explicit `model=` overrides exact-only so failed `openai/gpt-image-2` requests no longer fall through to Gemini or other configured providers, and update `image_generate list` to mention OpenAI Codex OAuth as valid auth for `openai/gpt-image-2`. Fixes #71290 and #71231. Thanks @Young-Khalil and @steipete.
|
||||
|
||||
@@ -18,7 +18,9 @@ const mocks = vi.hoisted(() => ({
|
||||
? undefined
|
||||
: undefined;
|
||||
const password = typeof authConfig?.password === "string" ? authConfig.password : undefined;
|
||||
const mode = authConfig?.mode ?? (password ? "password" : token ? "token" : "token");
|
||||
return {
|
||||
mode,
|
||||
token,
|
||||
password,
|
||||
};
|
||||
@@ -96,6 +98,7 @@ async function expectUnresolvedBrowserSecretRefSkipsPersistence(cfg: OpenClawCon
|
||||
}
|
||||
|
||||
let ensureBrowserControlAuth: typeof import("./control-auth.js").ensureBrowserControlAuth;
|
||||
let resolveBrowserControlAuth: typeof import("./control-auth.js").resolveBrowserControlAuth;
|
||||
|
||||
describe("ensureBrowserControlAuth", () => {
|
||||
const expectExplicitModeSkipsAutoAuth = async (mode: "password") => {
|
||||
@@ -129,7 +132,7 @@ describe("ensureBrowserControlAuth", () => {
|
||||
};
|
||||
|
||||
beforeAll(async () => {
|
||||
({ ensureBrowserControlAuth } = await import("./control-auth.js"));
|
||||
({ ensureBrowserControlAuth, resolveBrowserControlAuth } = await import("./control-auth.js"));
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -157,6 +160,100 @@ describe("ensureBrowserControlAuth", () => {
|
||||
expect(mocks.ensureGatewayStartupAuth).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns only the active credential in password mode", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
mode: "password",
|
||||
token: "inactive-token",
|
||||
password: "active-password",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({
|
||||
password: "active-password",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns only the resolved active credential when mode is inferred", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
token: "inactive-token",
|
||||
password: "active-password",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({
|
||||
password: "active-password",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns only the browser token in none mode", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
mode: "none",
|
||||
token: "browser-token",
|
||||
password: "inactive-password",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({
|
||||
token: "browser-token",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns only the active token in token mode", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
mode: "token",
|
||||
token: "active-token",
|
||||
password: "inactive-password",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({
|
||||
token: "active-token",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns only the browser password in trusted-proxy mode", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
mode: "trusted-proxy",
|
||||
token: "inactive-token",
|
||||
password: "browser-password",
|
||||
trustedProxy: { userHeader: "x-forwarded-user" },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({
|
||||
password: "browser-password",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not accept an inactive token in trusted-proxy mode", () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
auth: {
|
||||
mode: "trusted-proxy",
|
||||
token: "inactive-token",
|
||||
trustedProxy: { userHeader: "x-forwarded-user" },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
expect(resolveBrowserControlAuth(cfg, {} as NodeJS.ProcessEnv)).toEqual({});
|
||||
});
|
||||
|
||||
it("auto-generates and persists a token when auth is missing", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
browser: {
|
||||
|
||||
@@ -24,10 +24,18 @@ export function resolveBrowserControlAuth(
|
||||
});
|
||||
const token = normalizeOptionalString(auth.token) ?? "";
|
||||
const password = normalizeOptionalString(auth.password) ?? "";
|
||||
return {
|
||||
token: token || undefined,
|
||||
password: password || undefined,
|
||||
};
|
||||
const mode = auth.mode;
|
||||
|
||||
switch (mode) {
|
||||
case "password":
|
||||
case "trusted-proxy":
|
||||
return { password: password || undefined };
|
||||
case "token":
|
||||
case "none":
|
||||
return { token: token || undefined };
|
||||
default:
|
||||
return {};
|
||||
}
|
||||
}
|
||||
|
||||
export function shouldAutoGenerateBrowserAuth(env: NodeJS.ProcessEnv): boolean {
|
||||
|
||||
@@ -13,6 +13,7 @@ type EnsureBrowserControlAuthResult = {
|
||||
const mocks = vi.hoisted(() => ({
|
||||
controlPort: 0,
|
||||
gatewayAuthMode: undefined as "password" | undefined,
|
||||
gatewayAuthToken: undefined as string | undefined,
|
||||
ensureBrowserControlAuth: vi.fn<() => Promise<EnsureBrowserControlAuthResult>>(async () => {
|
||||
throw new Error("read-only config");
|
||||
}),
|
||||
@@ -31,7 +32,9 @@ vi.mock("../config/config.js", async () => {
|
||||
loadConfig: () => {
|
||||
return {
|
||||
browser: browserConfig,
|
||||
...(mocks.gatewayAuthMode ? { gateway: { auth: { mode: mocks.gatewayAuthMode } } } : {}),
|
||||
...(mocks.gatewayAuthMode || mocks.gatewayAuthToken
|
||||
? { gateway: { auth: { mode: mocks.gatewayAuthMode, token: mocks.gatewayAuthToken } } }
|
||||
: {}),
|
||||
};
|
||||
},
|
||||
};
|
||||
@@ -75,6 +78,7 @@ describe("browser control auth bootstrap failures", () => {
|
||||
beforeEach(async () => {
|
||||
mocks.controlPort = await getFreePort();
|
||||
mocks.gatewayAuthMode = undefined;
|
||||
mocks.gatewayAuthToken = undefined;
|
||||
mocks.ensureBrowserControlAuth.mockClear();
|
||||
mocks.resolveBrowserControlAuth.mockClear();
|
||||
mocks.shouldAutoGenerateBrowserAuth.mockClear();
|
||||
@@ -107,7 +111,7 @@ describe("browser control auth bootstrap failures", () => {
|
||||
expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps legacy password-mode startup when password is not configured", async () => {
|
||||
it("fails closed when password mode has no resolved password", async () => {
|
||||
mocks.gatewayAuthMode = "password";
|
||||
mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} });
|
||||
mocks.resolveBrowserControlAuth.mockReturnValueOnce({});
|
||||
@@ -115,6 +119,20 @@ describe("browser control auth bootstrap failures", () => {
|
||||
|
||||
const started = await startBrowserControlServerFromConfig();
|
||||
|
||||
expect(started).not.toBeNull();
|
||||
expect(started).toBeNull();
|
||||
expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails closed when password mode drops an inactive token but has no password", async () => {
|
||||
mocks.gatewayAuthMode = "password";
|
||||
mocks.gatewayAuthToken = "inactive-token";
|
||||
mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: {} });
|
||||
mocks.resolveBrowserControlAuth.mockReturnValueOnce({});
|
||||
mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(true);
|
||||
|
||||
const started = await startBrowserControlServerFromConfig();
|
||||
|
||||
expect(started).toBeNull();
|
||||
expect(mocks.ensureExtensionRelayForProfiles).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -66,4 +66,44 @@ describe("browser control HTTP auth", () => {
|
||||
expect(ok.status).toBe(200);
|
||||
expect((await ok.json()) as { ok: boolean }).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it("rejects bearer auth when password mode is active", async () => {
|
||||
const base = `http://127.0.0.1:${port}`;
|
||||
|
||||
server?.removeAllListeners("request");
|
||||
server?.on("request", (req: IncomingMessage, res: ServerResponse) => {
|
||||
if (!isAuthorizedBrowserRequest(req, { password: "browser-password" })) {
|
||||
res.statusCode = 401;
|
||||
res.end("Unauthorized");
|
||||
return;
|
||||
}
|
||||
res.statusCode = 200;
|
||||
res.end("ok");
|
||||
});
|
||||
|
||||
const bearer = await realFetch(`${base}/`, {
|
||||
headers: {
|
||||
Authorization: "Bearer browser-control-secret",
|
||||
},
|
||||
});
|
||||
expect(bearer.status).toBe(401);
|
||||
|
||||
const password = await realFetch(`${base}/`, {
|
||||
headers: {
|
||||
"x-openclaw-password": "browser-password",
|
||||
},
|
||||
});
|
||||
expect(password.status).toBe(200);
|
||||
});
|
||||
|
||||
it("rejects password auth when token mode is active", async () => {
|
||||
const base = `http://127.0.0.1:${port}`;
|
||||
|
||||
const password = await realFetch(`${base}/`, {
|
||||
headers: {
|
||||
"x-openclaw-password": "browser-control-secret",
|
||||
},
|
||||
});
|
||||
expect(password.status).toBe(401);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -54,17 +54,7 @@ export async function startBrowserControlServerFromConfig(): Promise<BrowserServ
|
||||
|
||||
const browserAuthRequired =
|
||||
browserAuthBootstrapFailed || shouldAutoGenerateBrowserAuth(process.env);
|
||||
const allowLegacyPasswordModeWithoutSecret =
|
||||
!browserAuthBootstrapFailed &&
|
||||
cfg.gateway?.auth?.mode === "password" &&
|
||||
!browserAuth.token &&
|
||||
!browserAuth.password;
|
||||
if (
|
||||
browserAuthRequired &&
|
||||
!allowLegacyPasswordModeWithoutSecret &&
|
||||
!browserAuth.token &&
|
||||
!browserAuth.password
|
||||
) {
|
||||
if (browserAuthRequired && !browserAuth.token && !browserAuth.password) {
|
||||
if (browserAuthBootstrapFailed) {
|
||||
logServer.error(
|
||||
"browser control startup aborted: authentication bootstrap failed " +
|
||||
|
||||
@@ -287,9 +287,6 @@ vi.mock("@slack/bolt", () => {
|
||||
command() {
|
||||
/* no-op */
|
||||
}
|
||||
use() {
|
||||
/* no-op */
|
||||
}
|
||||
start = vi.fn().mockResolvedValue(undefined);
|
||||
stop = vi.fn().mockResolvedValue(undefined);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user