From efcc1d7f0108e7b480295a10111cebe1969cfbc9 Mon Sep 17 00:00:00 2001 From: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> Date: Wed, 12 Aug 2026 05:37:42 +1000 Subject: [PATCH] fix(zalouser): cancel failed QR login sessions --- extensions/zalouser/src/channel.adapters.ts | 18 +++++-- extensions/zalouser/src/channel.runtime.ts | 1 + extensions/zalouser/src/channel.test.ts | 3 ++ extensions/zalouser/src/setup-surface.test.ts | 42 ++++++++++++++- extensions/zalouser/src/setup-surface.ts | 36 +++++++++---- .../zalouser/src/zalo-js.credentials.test.ts | 52 +++++++++++++++++++ extensions/zalouser/src/zalo-js.test-mocks.ts | 4 ++ extensions/zalouser/src/zalo-js.ts | 5 ++ 8 files changed, 145 insertions(+), 16 deletions(-) diff --git a/extensions/zalouser/src/channel.adapters.ts b/extensions/zalouser/src/channel.adapters.ts index 7449d13617c8..372432b58e67 100644 --- a/extensions/zalouser/src/channel.adapters.ts +++ b/extensions/zalouser/src/channel.adapters.ts @@ -400,7 +400,8 @@ export const zalouserAuthAdapter = { accountId?: string | null; runtime: RuntimeEnv; }) => { - const { startZaloQrLogin, waitForZaloQrLogin } = await loadZalouserChannelRuntime(); + const { cancelZaloQrLogin, startZaloQrLogin, waitForZaloQrLogin } = + await loadZalouserChannelRuntime(); const account = resolveZalouserAccountSync({ cfg, accountId: accountId ?? resolveDefaultZalouserAccountId(cfg), @@ -415,19 +416,28 @@ export const zalouserAuthAdapter = { timeoutMs: 35_000, }); if (!started.qrDataUrl) { + cancelZaloQrLogin(account.profile); throw new Error(started.message || "Failed to start QR login"); } - const qrPath = await writeQrDataUrlToTempFile(started.qrDataUrl, account.profile); + let qrPath: string | null = null; + try { + qrPath = await writeQrDataUrlToTempFile(started.qrDataUrl, account.profile); + } finally { + if (!qrPath) { + // The QR vendor login can persist credentials asynchronously. Cancel at + // its lifecycle owner whenever presentation fails or throws. + cancelZaloQrLogin(account.profile); + } + } if (!qrPath) { - // The direct CLI path has no prompt-level recovery. Stop before polling so - // an unusable vendor image cannot leave the operator waiting with nothing to scan. throw new Error("Zalo QR login returned an unusable image. Start login again."); } runtime.log(`Scan QR image: ${qrPath}`); const waited = await waitForZaloQrLogin({ profile: account.profile, timeoutMs: 180_000 }); if (!waited.connected) { + cancelZaloQrLogin(account.profile); throw new Error(waited.message || "Zalouser login failed"); } diff --git a/extensions/zalouser/src/channel.runtime.ts b/extensions/zalouser/src/channel.runtime.ts index a4d27eebcc97..997555058c06 100644 --- a/extensions/zalouser/src/channel.runtime.ts +++ b/extensions/zalouser/src/channel.runtime.ts @@ -3,6 +3,7 @@ export { probeZalouser } from "./probe.js"; export { collectZalouserSecurityAuditFindings } from "./security-audit.js"; export { sendMessageZalouser, sendReactionZalouser } from "./send.js"; export { + cancelZaloQrLogin, listZaloFriendsMatching, listZaloGroupMembers, listZaloGroupsMatching, diff --git a/extensions/zalouser/src/channel.test.ts b/extensions/zalouser/src/channel.test.ts index ef93d2806634..3dd5a471174d 100644 --- a/extensions/zalouser/src/channel.test.ts +++ b/extensions/zalouser/src/channel.test.ts @@ -22,6 +22,7 @@ describe("zalouser target classification", () => { import { setZalouserRuntime } from "./runtime.js"; import { sendMessageZalouser, sendReactionZalouser } from "./send.js"; import { + cancelZaloQrLoginMock, listZaloFriendsMatchingMock, startZaloQrLoginMock, waitForZaloQrLoginMock, @@ -485,6 +486,7 @@ describe("zalouser channel policies", () => { describe("zalouser account resolution", () => { beforeEach(() => { + cancelZaloQrLoginMock.mockReset(); listZaloFriendsMatchingMock.mockReset(); startZaloQrLoginMock.mockReset(); waitForZaloQrLoginMock.mockReset(); @@ -606,5 +608,6 @@ describe("zalouser account resolution", () => { }), ).rejects.toThrow("Zalo QR login returned an unusable image. Start login again."); expect(waitForZaloQrLoginMock).not.toHaveBeenCalled(); + expect(cancelZaloQrLoginMock).toHaveBeenCalledWith("work-profile"); }); }); diff --git a/extensions/zalouser/src/setup-surface.test.ts b/extensions/zalouser/src/setup-surface.test.ts index 7222d486af26..483f633ce47e 100644 --- a/extensions/zalouser/src/setup-surface.test.ts +++ b/extensions/zalouser/src/setup-surface.test.ts @@ -5,12 +5,13 @@ import { createTestWizardPrompter, runSetupWizardConfigure, } from "openclaw/plugin-sdk/plugin-test-runtime"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../runtime-api.js"; import "./zalo-js.test-mocks.js"; import { zalouserSetupWizard } from "./setup-surface.js"; import { zalouserSetupPlugin } from "./setup-test-helpers.js"; import { + cancelZaloQrLoginMock, checkZaloAuthenticatedMock, logoutZaloProfileMock, resolveZaloAllowFromEntriesMock, @@ -19,6 +20,14 @@ import { waitForZaloQrLoginMock, } from "./zalo-js.test-mocks.js"; +const PNG_1X1 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII="; +const writeQrDataUrlToTempFileMock = vi.hoisted(() => vi.fn()); + +vi.mock("./qr-temp-file.js", () => ({ + writeQrDataUrlToTempFile: writeQrDataUrlToTempFileMock, +})); + const zalouserConfigure = createPluginSetupWizardConfigure(zalouserSetupPlugin); async function runSetup(params: { @@ -37,6 +46,11 @@ async function runSetup(params: { } describe("zalouser setup wizard", () => { + beforeEach(() => { + writeQrDataUrlToTempFileMock.mockReset(); + writeQrDataUrlToTempFileMock.mockResolvedValue("/tmp/zalouser-qr.png"); + }); + function expectEnabledDefaultSetup( result: Awaited>, dmPolicy?: "pairing" | "allowlist", @@ -209,6 +223,8 @@ describe("zalouser setup wizard", () => { { name: "first login", authenticated: false }, { name: "forced re-login", authenticated: true }, ])("recovers when $name cannot present its QR image", async ({ authenticated }) => { + cancelZaloQrLoginMock.mockClear(); + writeQrDataUrlToTempFileMock.mockResolvedValueOnce(null); checkZaloAuthenticatedMock.mockResolvedValueOnce(authenticated); startZaloQrLoginMock.mockResolvedValueOnce({ message: "qr pending", @@ -237,6 +253,30 @@ describe("zalouser setup wizard", () => { ); expect(confirmations).not.toContain("Did you scan and approve the QR on your phone?"); expect(waitForZaloQrLoginMock).not.toHaveBeenCalled(); + expect(cancelZaloQrLoginMock).toHaveBeenCalledWith("default"); + }); + + it("cancels the active QR login when scan confirmation is declined", async () => { + checkZaloAuthenticatedMock.mockResolvedValueOnce(false); + cancelZaloQrLoginMock.mockClear(); + startZaloQrLoginMock.mockResolvedValueOnce({ + message: "qr ready", + qrDataUrl: `data:image/png;base64,${PNG_1X1}`, + }); + waitForZaloQrLoginMock.mockClear(); + const prompter = createTestWizardPrompter({ + confirm: vi.fn(async ({ message }: { message: string }) => { + if (message === "Login via QR code now?") { + return true; + } + return false; + }), + }); + + await runSetup({ prompter }); + + expect(cancelZaloQrLoginMock).toHaveBeenCalledWith("default"); + expect(waitForZaloQrLoginMock).not.toHaveBeenCalled(); }); it.each([ diff --git a/extensions/zalouser/src/setup-surface.ts b/extensions/zalouser/src/setup-surface.ts index 970b811e16cf..cc00e9e0b15e 100644 --- a/extensions/zalouser/src/setup-surface.ts +++ b/extensions/zalouser/src/setup-surface.ts @@ -24,6 +24,7 @@ import { } from "./accounts.js"; import { writeQrDataUrlToTempFile } from "./qr-temp-file.js"; import { + cancelZaloQrLogin, logoutZaloProfile, resolveZaloAllowFromEntries, resolveZaloGroupsByEntries, @@ -291,21 +292,33 @@ async function runZalouserQrLogin(params: { : {}), }); if (!start.qrDataUrl) { + cancelZaloQrLogin(params.profile); 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"), - ); + let qrPath: string | null = null; + let presented = false; + try { + 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"), + ); + presented = qrPath !== null; + } finally { + if (!presented) { + // Failed presentation must retire the vendor login before setup returns; + // otherwise its worker can persist credentials after reporting failure. + cancelZaloQrLogin(params.profile); + } + } if (!qrPath) { return; } @@ -315,6 +328,7 @@ async function runZalouserQrLogin(params: { initialValue: true, }); if (!scanned) { + cancelZaloQrLogin(params.profile); return; } diff --git a/extensions/zalouser/src/zalo-js.credentials.test.ts b/extensions/zalouser/src/zalo-js.credentials.test.ts index adcaee74266d..fad906d8ca2b 100644 --- a/extensions/zalouser/src/zalo-js.credentials.test.ts +++ b/extensions/zalouser/src/zalo-js.credentials.test.ts @@ -35,6 +35,7 @@ import { type StoredZaloCredentials, } from "./session-state.js"; import { + cancelZaloQrLogin, checkZaloAuthenticated, listZaloFriends, sendZaloLink, @@ -277,6 +278,57 @@ describe("zalouser credential persistence", () => { expect(createZaloMock).toHaveBeenCalledTimes(2); }); + it("cancels the active vendor login before a late result can persist credentials", async () => { + const stateDir = await mkdtemp(path.join(os.tmpdir(), "openclaw-zalouser-credentials-")); + const profile = "qr-presentation-cancel"; + const abort = vi.fn(); + let resolveLogin: (api: API) => void = () => undefined; + const loginResult = new Promise((resolve) => { + resolveLogin = resolve; + }); + const api = createMockApi({ + imei: "cancelled-imei", + userAgent: "cancelled-user-agent", + cookies: [{ key: "zpsid", value: "cancelled", domain: "chat.zalo.me" }], + }); + + createZaloMock.mockResolvedValueOnce({ + loginQR: async (_options: unknown, callback?: (event: LoginQRCallbackEvent) => unknown) => { + callback?.({ + type: LoginQRCallbackEventType.QRCodeGenerated, + data: { + code: "cancelled-qr", + image: `data:image/png;base64,${PNG_1X1}`, + }, + actions: { + saveToFile: vi.fn(async () => undefined), + retry: vi.fn(), + abort, + }, + }); + return await loginResult; + }, + }); + + try { + await withEnvAsync({ OPENCLAW_STATE_DIR: stateDir }, async () => { + const started = await startZaloQrLogin({ profile, timeoutMs: 1000 }); + expect(started.qrDataUrl).toBe(`data:image/png;base64,${PNG_1X1}`); + + cancelZaloQrLogin(profile); + resolveLogin(api); + await new Promise((resolve) => { + setImmediate(resolve); + }); + + expect(abort).toHaveBeenCalledTimes(1); + expect(loadStoredZaloCredentials(profile)).toBeNull(); + }); + } finally { + await rm(stateDir, { recursive: true, force: true }); + } + }); + 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"; diff --git a/extensions/zalouser/src/zalo-js.test-mocks.ts b/extensions/zalouser/src/zalo-js.test-mocks.ts index ea776b2c063a..091e53741f93 100644 --- a/extensions/zalouser/src/zalo-js.test-mocks.ts +++ b/extensions/zalouser/src/zalo-js.test-mocks.ts @@ -3,6 +3,7 @@ import { vi, type Mock } from "vitest"; type ZaloJsModule = typeof import("./zalo-js.js"); type ZaloJsMocks = { + cancelZaloQrLoginMock: Mock; checkZaloAuthenticatedMock: Mock; getZaloUserInfoMock: Mock; listZaloFriendsMock: Mock; @@ -23,6 +24,7 @@ type ZaloJsMocks = { const zaloJsMocks = vi.hoisted( (): ZaloJsMocks => ({ + cancelZaloQrLoginMock: vi.fn(), checkZaloAuthenticatedMock: vi.fn(async () => false), getZaloUserInfoMock: vi.fn(async () => null), listZaloFriendsMock: vi.fn(async () => []), @@ -66,6 +68,7 @@ const zaloJsMocks = vi.hoisted( ); export const listZaloFriendsMock = zaloJsMocks.listZaloFriendsMock; +export const cancelZaloQrLoginMock = zaloJsMocks.cancelZaloQrLoginMock; export const listZaloFriendsMatchingMock = zaloJsMocks.listZaloFriendsMatchingMock; export const listZaloGroupMembersMock = zaloJsMocks.listZaloGroupMembersMock; export const listZaloGroupsMock = zaloJsMocks.listZaloGroupsMock; @@ -79,6 +82,7 @@ export const startZaloQrLoginMock = zaloJsMocks.startZaloQrLoginMock; export const waitForZaloQrLoginMock = zaloJsMocks.waitForZaloQrLoginMock; vi.mock("./zalo-js.js", () => ({ + cancelZaloQrLogin: cancelZaloQrLoginMock, checkZaloAuthenticated: zaloJsMocks.checkZaloAuthenticatedMock, getZaloUserInfo: zaloJsMocks.getZaloUserInfoMock, listZaloFriends: listZaloFriendsMock, diff --git a/extensions/zalouser/src/zalo-js.ts b/extensions/zalouser/src/zalo-js.ts index 46c46fd13c6c..2e72ff022ec1 100644 --- a/extensions/zalouser/src/zalo-js.ts +++ b/extensions/zalouser/src/zalo-js.ts @@ -757,6 +757,11 @@ function resetQrLogin(profileInput?: string | null): void { activeQrLogins.delete(profile); } +/** Cancel the profile's active vendor QR login before abandoning presentation. */ +export function cancelZaloQrLogin(profileInput?: string | null): void { + resetQrLogin(profileInput); +} + async function fetchGroupsByIds(api: API, ids: string[]): Promise> { const result = new Map(); for (let index = 0; index < ids.length; index += GROUP_INFO_CHUNK_SIZE) {