From 7612e7e2e20b23fc0372aa4a410dd18e84076be7 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:15:23 +1000 Subject: [PATCH] fix(zalouser): handle unusable QR login images --- extensions/zalouser/src/channel.test.ts | 5 +- extensions/zalouser/src/qr-temp-file.test.ts | 5 +- extensions/zalouser/src/qr-temp-file.ts | 12 +- extensions/zalouser/src/setup-surface.test.ts | 78 ++++++++++++ extensions/zalouser/src/setup-surface.ts | 120 +++++++++--------- .../zalouser/src/zalo-js.credentials.test.ts | 93 +++++++++++++- extensions/zalouser/src/zalo-js.ts | 12 +- 7 files changed, 256 insertions(+), 69 deletions(-) diff --git a/extensions/zalouser/src/channel.test.ts b/extensions/zalouser/src/channel.test.ts index 940cb918f519..262982f7871f 100644 --- a/extensions/zalouser/src/channel.test.ts +++ b/extensions/zalouser/src/channel.test.ts @@ -27,6 +27,9 @@ import { waitForZaloQrLoginMock, } from "./zalo-js.test-mocks.js"; +const PNG_1X1 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII="; + vi.mock("./qr-temp-file.js", () => ({ writeQrDataUrlToTempFile: vi.fn(async () => null), })); @@ -533,7 +536,7 @@ describe("zalouser account resolution", () => { startZaloQrLoginMock.mockResolvedValue({ message: "qr ready", - qrDataUrl: "data:image/png;base64,abc", + qrDataUrl: `data:image/png;base64,${PNG_1X1}`, } as never); waitForZaloQrLoginMock.mockResolvedValue({ connected: true, diff --git a/extensions/zalouser/src/qr-temp-file.test.ts b/extensions/zalouser/src/qr-temp-file.test.ts index ecd3090f494c..06b63877ea6c 100644 --- a/extensions/zalouser/src/qr-temp-file.test.ts +++ b/extensions/zalouser/src/qr-temp-file.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { writeQrDataUrlToTempFile } from "./qr-temp-file.js"; describe("writeQrDataUrlToTempFile", () => { + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); const profile = `test/profile-${process.pid}`; const expectedPath = path.join( resolvePreferredOpenClawTmpDir(), @@ -16,8 +17,8 @@ describe("writeQrDataUrlToTempFile", () => { }); it("overwrites the stable per-profile path and enforces private mode", async () => { - const firstData = Buffer.from("first-qr-image"); - const secondData = Buffer.from("second-qr-image"); + const firstData = Buffer.concat([pngHeader, Buffer.from("first-qr-image")]); + const secondData = Buffer.concat([pngHeader, Buffer.from("second-qr-image")]); const first = await writeQrDataUrlToTempFile( `data:image/png;base64,${firstData.toString("base64")}`, profile, diff --git a/extensions/zalouser/src/qr-temp-file.ts b/extensions/zalouser/src/qr-temp-file.ts index 841e0e3a899b..2ab537a5b0b2 100644 --- a/extensions/zalouser/src/qr-temp-file.ts +++ b/extensions/zalouser/src/qr-temp-file.ts @@ -1,25 +1,27 @@ // Zalouser plugin module implements qr temp file behavior. import fsp from "node:fs/promises"; import path from "node:path"; +import { sanitizeInlineImageDataUrl } from "openclaw/plugin-sdk/inline-image-data-url-runtime"; import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +const PNG_DATA_URL_PREFIX = "data:image/png;base64,"; + export async function writeQrDataUrlToTempFile( qrDataUrl: string, profile: string, ): Promise { - const trimmed = qrDataUrl.trim(); - const match = trimmed.match(/^data:image\/png;base64,(.+)$/i); - const base64 = (match?.[1] ?? "").trim(); - if (!base64) { + const normalized = sanitizeInlineImageDataUrl(qrDataUrl.trim()); + if (!normalized?.startsWith(PNG_DATA_URL_PREFIX)) { return null; } + const png = Buffer.from(normalized.slice(PNG_DATA_URL_PREFIX.length), "base64"); const safeProfile = profile.replace(/[^a-zA-Z0-9_-]+/g, "-") || "default"; // The stable private-root name lets QR refreshes overwrite instead of accumulating temp files. const filePath = path.join( resolvePreferredOpenClawTmpDir(), `openclaw-zalouser-qr-${safeProfile}.png`, ); - await fsp.writeFile(filePath, Buffer.from(base64, "base64"), { mode: 0o600 }); + await fsp.writeFile(filePath, png, { mode: 0o600 }); await fsp.chmod(filePath, 0o600); return filePath; } diff --git a/extensions/zalouser/src/setup-surface.test.ts b/extensions/zalouser/src/setup-surface.test.ts index aa5ad31cd6ab..7222d486af26 100644 --- a/extensions/zalouser/src/setup-surface.test.ts +++ b/extensions/zalouser/src/setup-surface.test.ts @@ -16,6 +16,7 @@ import { resolveZaloAllowFromEntriesMock, resolveZaloGroupsByEntriesMock, startZaloQrLoginMock, + waitForZaloQrLoginMock, } from "./zalo-js.test-mocks.js"; const zalouserConfigure = createPluginSetupWizardConfigure(zalouserSetupPlugin); @@ -204,6 +205,83 @@ describe("zalouser setup wizard", () => { ); }); + it.each([ + { name: "first login", authenticated: false }, + { name: "forced re-login", authenticated: true }, + ])("recovers when $name cannot present its QR image", async ({ authenticated }) => { + checkZaloAuthenticatedMock.mockResolvedValueOnce(authenticated); + startZaloQrLoginMock.mockResolvedValueOnce({ + message: "qr pending", + qrDataUrl: "data:text/plain;base64,bm90LWEtcG5n", + }); + waitForZaloQrLoginMock.mockClear(); + const note = vi.fn(async (_message: string, _title?: string) => {}); + const confirmations: string[] = []; + const prompter = createTestWizardPrompter({ + note, + confirm: vi.fn(async ({ message }: { message: string }) => { + confirmations.push(message); + return message !== "Zalo Personal already logged in. Keep session?"; + }), + }); + + await runSetup({ prompter }); + + expect(note).toHaveBeenCalledWith( + expect.stringContaining("Could not write QR image file"), + "QR Login", + ); + expect(note).not.toHaveBeenCalledWith( + expect.stringContaining("Scan + approve on phone, then continue."), + "QR Login", + ); + expect(confirmations).not.toContain("Did you scan and approve the QR on your phone?"); + expect(waitForZaloQrLoginMock).not.toHaveBeenCalled(); + }); + + it.each([ + { name: "first login", authenticated: false, expectedLogouts: 0 }, + { name: "forced re-login", authenticated: true, expectedLogouts: 1 }, + ])("reports $name QR startup failures", async ({ authenticated, expectedLogouts }) => { + checkZaloAuthenticatedMock.mockResolvedValueOnce(authenticated); + logoutZaloProfileMock.mockClear(); + startZaloQrLoginMock.mockClear(); + startZaloQrLoginMock.mockResolvedValueOnce({ + message: "Failed to start QR login: invalid QR image", + }); + waitForZaloQrLoginMock.mockClear(); + const note = vi.fn(async (_message: string, _title?: string) => {}); + const prompter = createTestWizardPrompter({ + note, + confirm: vi.fn(async ({ message }: { message: string }) => { + if (message === "Login via QR code now?") { + return true; + } + if (message === "Zalo Personal already logged in. Keep session?") { + return false; + } + return false; + }), + }); + + await runSetup({ prompter }); + + expect(note).toHaveBeenCalledWith( + "Failed to start QR login: invalid QR image", + "Login pending", + ); + expect(logoutZaloProfileMock).toHaveBeenCalledTimes(expectedLogouts); + expect(prompter.confirm).not.toHaveBeenCalledWith( + expect.objectContaining({ message: "Did you scan and approve the QR on your phone?" }), + ); + if (expectedLogouts > 0) { + expect(logoutZaloProfileMock.mock.invocationCallOrder[0]).toBeLessThan( + startZaloQrLoginMock.mock.invocationCallOrder[0]!, + ); + } + expect(waitForZaloQrLoginMock).not.toHaveBeenCalled(); + }); + it("prompts DM policy before group access in quickstart", async () => { const seen: string[] = []; const prompter = createQuickstartPrompter({ seen, dmPolicy: "pairing" }); diff --git a/extensions/zalouser/src/setup-surface.ts b/extensions/zalouser/src/setup-surface.ts index 8b5971d3bbce..970b811e16cf 100644 --- a/extensions/zalouser/src/setup-surface.ts +++ b/extensions/zalouser/src/setup-surface.ts @@ -272,6 +272,62 @@ async function promptZalouserQuickstartDmPolicy(params: { return setZalouserDmPolicy(cfg, accountId, policy); } +async function runZalouserQrLogin(params: { + profile: string; + prompter: Parameters>[0]["prompter"]; + beforePersistentEffect?: () => Promise; + replaceSession?: boolean; +}): Promise { + if (params.replaceSession) { + await params.beforePersistentEffect?.(); + await logoutZaloProfile(params.profile); + } + await params.beforePersistentEffect?.(); + const start = await startZaloQrLogin({ + profile: params.profile, + timeoutMs: 35_000, + ...(params.beforePersistentEffect + ? { beforeCredentialPersistence: params.beforePersistentEffect } + : {}), + }); + if (!start.qrDataUrl) { + await params.prompter.note(start.message, t("wizard.zalouser.loginPendingTitle")); + return; + } + + const qrPath = await writeQrDataUrlToTempFile(start.qrDataUrl, params.profile); + await params.prompter.note( + [ + start.message, + qrPath + ? t("wizard.zalouser.qrImageSaved", { path: qrPath }) + : t("wizard.zalouser.qrImageWriteFailed"), + ...(qrPath ? [t("wizard.zalouser.scanApproveContinue")] : []), + ].join("\n"), + t("wizard.zalouser.qrLoginTitle"), + ); + if (!qrPath) { + return; + } + + const scanned = await params.prompter.confirm({ + message: t("wizard.zalouser.qrScannedPrompt"), + initialValue: true, + }); + if (!scanned) { + return; + } + + const waited = await waitForZaloQrLogin({ + profile: params.profile, + timeoutMs: 120_000, + }); + await params.prompter.note( + waited.message, + waited.connected ? t("common.done") : t("wizard.zalouser.loginPendingTitle"), + ); +} + export { zalouserSetupAdapter } from "./setup-core.js"; export const zalouserSetupWizard: ChannelSetupWizard = { @@ -321,43 +377,13 @@ export const zalouserSetupWizard: ChannelSetupWizard = { }); if (wantsLogin) { - await options?.beforePersistentEffect?.(); - const start = await startZaloQrLogin({ + await runZalouserQrLogin({ profile: account.profile, - timeoutMs: 35_000, + prompter, ...(options?.beforePersistentEffect - ? { beforeCredentialPersistence: options.beforePersistentEffect } + ? { beforePersistentEffect: options.beforePersistentEffect } : {}), }); - if (start.qrDataUrl) { - const qrPath = await writeQrDataUrlToTempFile(start.qrDataUrl, account.profile); - await prompter.note( - [ - start.message, - qrPath - ? t("wizard.zalouser.qrImageSaved", { path: qrPath }) - : t("wizard.zalouser.qrImageWriteFailed"), - t("wizard.zalouser.scanApproveContinue"), - ].join("\n"), - t("wizard.zalouser.qrLoginTitle"), - ); - const scanned = await prompter.confirm({ - message: t("wizard.zalouser.qrScannedPrompt"), - initialValue: true, - }); - if (scanned) { - const waited = await waitForZaloQrLogin({ - profile: account.profile, - timeoutMs: 120_000, - }); - await prompter.note( - waited.message, - waited.connected ? t("common.done") : t("wizard.zalouser.loginPendingTitle"), - ); - } - } else { - await prompter.note(start.message, t("wizard.zalouser.loginPendingTitle")); - } } } else { const keepSession = await prompter.confirm({ @@ -365,34 +391,14 @@ export const zalouserSetupWizard: ChannelSetupWizard = { initialValue: true, }); if (!keepSession) { - await options?.beforePersistentEffect?.(); - await logoutZaloProfile(account.profile); - await options?.beforePersistentEffect?.(); - const start = await startZaloQrLogin({ + await runZalouserQrLogin({ profile: account.profile, - force: true, - timeoutMs: 35_000, + prompter, + replaceSession: true, ...(options?.beforePersistentEffect - ? { beforeCredentialPersistence: options.beforePersistentEffect } + ? { beforePersistentEffect: options.beforePersistentEffect } : {}), }); - if (start.qrDataUrl) { - const qrPath = await writeQrDataUrlToTempFile(start.qrDataUrl, account.profile); - await prompter.note( - [ - start.message, - qrPath ? t("wizard.zalouser.qrImageSaved", { path: qrPath }) : undefined, - ] - .filter(Boolean) - .join("\n"), - t("wizard.zalouser.qrLoginTitle"), - ); - const waited = await waitForZaloQrLogin({ profile: account.profile, timeoutMs: 120_000 }); - await prompter.note( - waited.message, - waited.connected ? t("common.done") : t("wizard.zalouser.loginPendingTitle"), - ); - } } } diff --git a/extensions/zalouser/src/zalo-js.credentials.test.ts b/extensions/zalouser/src/zalo-js.credentials.test.ts index 0734adcd170f..adcaee74266d 100644 --- a/extensions/zalouser/src/zalo-js.credentials.test.ts +++ b/extensions/zalouser/src/zalo-js.credentials.test.ts @@ -16,6 +16,9 @@ import { LoginQRCallbackEventType } from "./zca-constants.js"; const createZaloMock = vi.hoisted(() => vi.fn()); const ISO_TIMESTAMP_RE = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/u; +const PNG_1X1 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII="; +const GIF_1X1 = "R0lGODlhAQABAIAAAAAAAP///ywAAAAAAQABAAACAUwAOw=="; vi.mock("./zca-client.js", () => ({ createZalo: createZaloMock, @@ -149,7 +152,7 @@ describe("zalouser credential persistence", () => { type: LoginQRCallbackEventType.QRCodeGenerated, data: { code: "qr-code", - image: "data:image/png;base64,abc123", + image: `data:image/png;base64,${PNG_1X1}`, }, actions: { saveToFile: vi.fn(async () => undefined), @@ -188,6 +191,92 @@ describe("zalouser credential persistence", () => { } }); + it("rejects a non-PNG QR image and allows an immediate valid retry", async () => { + const profile = "qr-invalid-image-retry"; + const firstAbort = vi.fn(); + let rejectFirstLogin: (error: Error) => void = () => undefined; + const firstLogin = new Promise((_resolve, reject) => { + rejectFirstLogin = reject; + }); + let resolveSecondLogin: (api: API) => void = () => undefined; + const secondLogin = new Promise((resolve) => { + resolveSecondLogin = resolve; + }); + let secondCallback: ((event: LoginQRCallbackEvent) => unknown) | undefined; + const api = createMockApi({ + imei: "retry-imei", + userAgent: "retry-user-agent", + language: "vi", + cookies: [{ key: "zpsid", value: "retry", domain: "chat.zalo.me" }], + }); + + createZaloMock + .mockResolvedValueOnce({ + loginQR: async (_options: unknown, callback?: (event: LoginQRCallbackEvent) => unknown) => { + callback?.({ + type: LoginQRCallbackEventType.QRCodeGenerated, + data: { + code: "invalid-qr", + image: `data:image/gif;base64,${GIF_1X1}`, + }, + actions: { + saveToFile: vi.fn(async () => undefined), + retry: vi.fn(), + abort: () => { + firstAbort(); + rejectFirstLogin(new Error("aborted invalid QR login")); + }, + }, + }); + return await firstLogin; + }, + }) + .mockResolvedValueOnce({ + loginQR: async (_options: unknown, callback?: (event: LoginQRCallbackEvent) => unknown) => { + secondCallback = callback; + callback?.({ + type: LoginQRCallbackEventType.QRCodeGenerated, + data: { + code: "valid-qr", + image: PNG_1X1, + }, + actions: { + saveToFile: vi.fn(async () => undefined), + retry: vi.fn(), + abort: vi.fn(), + }, + }); + return await secondLogin; + }, + }); + + const rejected = await startZaloQrLogin({ profile, timeoutMs: 1000 }); + + expect(rejected.qrDataUrl).toBeUndefined(); + expect(rejected.message).toContain("invalid or non-PNG QR image"); + expect(firstAbort).toHaveBeenCalledTimes(1); + + const started = await startZaloQrLogin({ profile, timeoutMs: 1000 }); + expect(started.qrDataUrl).toBe(`data:image/png;base64,${PNG_1X1}`); + + secondCallback?.({ + type: LoginQRCallbackEventType.GotLoginInfo, + data: { + cookie: [{ key: "zpsid", value: "retry", domain: "chat.zalo.me" }], + imei: "retry-imei", + userAgent: "retry-user-agent", + }, + actions: null, + }); + resolveSecondLogin(api); + + await expect(waitForZaloQrLogin({ profile, timeoutMs: 1000 })).resolves.toEqual({ + connected: true, + message: "Login successful.", + }); + expect(createZaloMock).toHaveBeenCalledTimes(2); + }); + it("revalidates setup ownership immediately before QR credentials are written", async () => { const stateDir = await mkdtemp(path.join(os.tmpdir(), "openclaw-zalouser-credentials-")); const profile = "qr-stale-owner"; @@ -207,7 +296,7 @@ describe("zalouser credential persistence", () => { type: LoginQRCallbackEventType.QRCodeGenerated, data: { code: "qr-code", - image: "data:image/png;base64,abc123", + image: `data:image/png;base64,${PNG_1X1}`, }, actions: { saveToFile: vi.fn(async () => undefined), diff --git a/extensions/zalouser/src/zalo-js.ts b/extensions/zalouser/src/zalo-js.ts index 24c72cd9ef07..46c46fd13c6c 100644 --- a/extensions/zalouser/src/zalo-js.ts +++ b/extensions/zalouser/src/zalo-js.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; // Zalouser plugin module implements zalo js behavior. import { expectDefined } from "openclaw/plugin-sdk/expect-runtime"; +import { sanitizeInlineImageDataUrl } from "openclaw/plugin-sdk/inline-image-data-url-runtime"; import { extensionForMime } from "openclaw/plugin-sdk/media-mime"; import { asDateTimestampMs, @@ -1527,10 +1528,17 @@ export async function startZaloQrLogin(params: { switch (event.type) { case LoginQRCallbackEventType.QRCodeGenerated: { - const image = event.data.image.replace(/^data:image\/png;base64,/, ""); - current.qrDataUrl = image.startsWith("data:image") + const image = event.data.image.trim(); + const candidate = image.toLowerCase().startsWith("data:") ? image : `data:image/png;base64,${image}`; + const normalized = sanitizeInlineImageDataUrl(candidate); + if (!normalized?.startsWith("data:image/png;base64,")) { + delete current.qrDataUrl; + current.error = "Zalo returned an invalid or non-PNG QR image."; + break; + } + current.qrDataUrl = normalized; break; } case LoginQRCallbackEventType.QRCodeExpired: {