From b618b9b428a6bad4ca1ffdd18e08489ee52099b1 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 22 Apr 2026 23:34:50 -0400 Subject: [PATCH] fix: handle Matrix verification edge cases --- .../src/matrix/actions/verification.test.ts | 106 ++++++++++++++++++ .../matrix/src/matrix/actions/verification.ts | 82 +++++++++----- .../runners/contract/scenario-runtime-cli.ts | 4 +- 3 files changed, 162 insertions(+), 30 deletions(-) diff --git a/extensions/matrix/src/matrix/actions/verification.test.ts b/extensions/matrix/src/matrix/actions/verification.test.ts index e94a8e56fbd..c3096b1b180 100644 --- a/extensions/matrix/src/matrix/actions/verification.test.ts +++ b/extensions/matrix/src/matrix/actions/verification.test.ts @@ -420,6 +420,45 @@ describe("matrix verification actions", () => { expect(crypto.startVerification).not.toHaveBeenCalled(); }); + it("finalizes completed non-SAS self-verification without waiting for SAS", async () => { + const completed = { + completed: true, + hasSas: false, + id: "verification-1", + phaseName: "done", + transactionId: "tx-self", + }; + const crypto = { + confirmVerificationSas: vi.fn(), + listVerifications: vi.fn(async () => []), + requestVerification: vi.fn(async () => completed), + startVerification: vi.fn(), + }; + const confirmSas = vi.fn(async () => true); + const bootstrapOwnDeviceVerification = vi.fn(async () => ({ + success: true, + verification: mockVerifiedOwnerStatus(), + })); + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ + bootstrapOwnDeviceVerification, + crypto, + getOwnDeviceVerificationStatus: vi.fn(async () => mockVerifiedOwnerStatus()), + }); + }); + + await expect(runMatrixSelfVerification({ confirmSas, timeoutMs: 500 })).resolves.toMatchObject({ + completed: true, + deviceOwnerVerified: true, + id: "verification-1", + }); + + expect(crypto.listVerifications).not.toHaveBeenCalled(); + expect(crypto.startVerification).not.toHaveBeenCalled(); + expect(crypto.confirmVerificationSas).not.toHaveBeenCalled(); + expect(confirmSas).not.toHaveBeenCalled(); + }); + it("allows completed self-verification when only backup health remains degraded", async () => { const requested = { completed: false, @@ -544,4 +583,71 @@ describe("matrix verification actions", () => { reason: "OpenClaw self-verification did not complete", }); }); + + it("fails immediately when the self-verification request is cancelled while waiting", async () => { + const requested = { + completed: false, + hasSas: false, + id: "verification-1", + phaseName: "requested", + transactionId: "tx-self", + }; + const cancelled = { + ...requested, + error: "Remote cancelled", + pending: false, + phaseName: "cancelled", + }; + const crypto = { + cancelVerification: vi.fn(async () => cancelled), + listVerifications: vi.fn(async () => [cancelled]), + requestVerification: vi.fn(async () => requested), + }; + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ crypto }); + }); + + await expect( + runMatrixSelfVerification({ confirmSas: vi.fn(async () => true), timeoutMs: 500 }), + ).rejects.toThrow("Matrix self-verification was cancelled: Remote cancelled"); + + expect(crypto.listVerifications).toHaveBeenCalledTimes(1); + expect(crypto.cancelVerification).toHaveBeenCalledWith("verification-1", { + code: "m.user", + reason: "OpenClaw self-verification did not complete", + }); + }); + + it("cancels the request when SAS mismatch submission fails", async () => { + const sas = { + completed: false, + hasSas: true, + id: "verification-1", + phaseName: "started", + sas: { + decimal: [1, 2, 3], + }, + transactionId: "tx-self", + }; + const crypto = { + cancelVerification: vi.fn(async () => sas), + listVerifications: vi.fn(async () => [sas]), + mismatchVerificationSas: vi.fn(async () => { + throw new Error("failed to send SAS mismatch"); + }), + requestVerification: vi.fn(async () => sas), + }; + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ crypto }); + }); + + await expect( + runMatrixSelfVerification({ confirmSas: vi.fn(async () => false), timeoutMs: 500 }), + ).rejects.toThrow("failed to send SAS mismatch"); + + expect(crypto.cancelVerification).toHaveBeenCalledWith("verification-1", { + code: "m.user", + reason: "OpenClaw self-verification did not complete", + }); + }); }); diff --git a/extensions/matrix/src/matrix/actions/verification.ts b/extensions/matrix/src/matrix/actions/verification.ts index 575b84d63bb..3c8b1143e12 100644 --- a/extensions/matrix/src/matrix/actions/verification.ts +++ b/extensions/matrix/src/matrix/actions/verification.ts @@ -65,6 +65,10 @@ function shouldStartMatrixSasVerification(summary: MatrixVerificationSummary): b return !summary.hasSas && summary.phaseName !== "started" && !summary.completed; } +function isMatrixVerificationCancelled(summary: MatrixVerificationSummary): boolean { + return summary.phaseName === "cancelled"; +} + async function waitForMatrixVerificationSummary(params: { crypto: MatrixCryptoActionFacade; label: string; @@ -82,6 +86,13 @@ async function waitForMatrixVerificationSummary(params: { if (params.predicate(found)) { return found; } + if (isMatrixVerificationCancelled(found)) { + throw new Error( + `Matrix self-verification was cancelled${ + found.error ? `: ${found.error}` : ` while waiting to ${params.label}` + }`, + ); + } } await sleep(Math.min(250, Math.max(25, params.timeoutMs - (Date.now() - startedAt)))); } @@ -138,6 +149,33 @@ async function cancelMatrixSelfVerificationOnFailure(params: { .catch(() => undefined); } +async function completeMatrixSelfVerification(params: { + client: MatrixActionClient; + completed: MatrixVerificationSummary; + timeoutMs: number; +}): Promise { + const bootstrap = await params.client.bootstrapOwnDeviceVerification({ + allowAutomaticCrossSigningReset: false, + verifyOwnIdentity: true, + }); + if (!bootstrap.verification.verified) { + throw new Error( + `Matrix self-verification completed, but full Matrix identity trust is still incomplete: ${ + bootstrap.error ?? formatMatrixOwnerVerificationDiagnostics(bootstrap.verification) + }`, + ); + } + const ownerVerification = await waitForMatrixOwnerVerificationStatus({ + client: params.client, + timeoutMs: params.timeoutMs, + }); + return { + ...params.completed, + deviceOwnerVerified: ownerVerification.verified, + ownerVerification, + }; +} + export async function listMatrixVerifications(opts: MatrixActionClientOpts = {}) { return await withStartedActionClient(opts, async (client) => { const crypto = requireCrypto(client, opts); @@ -187,18 +225,22 @@ export async function runMatrixSelfVerification( requested = await crypto.requestVerification({ ownUser: true }); await params.onRequested?.(requested); - let ready = requested; - if (!ready.hasSas) { - ready = await waitForMatrixVerificationSummary({ - crypto, - label: "be accepted in another Matrix client", - request: requested, - timeoutMs, - predicate: isMatrixVerificationReadyForSas, - }); - } + const ready = isMatrixVerificationReadyForSas(requested) + ? requested + : await waitForMatrixVerificationSummary({ + crypto, + label: "be accepted in another Matrix client", + request: requested, + timeoutMs, + predicate: isMatrixVerificationReadyForSas, + }); await params.onReady?.(ready); + if (ready.completed) { + requestCompleted = true; + return await completeMatrixSelfVerification({ client, completed: ready, timeoutMs }); + } + const started = shouldStartMatrixSasVerification(ready) ? await crypto.startVerification(ready.id, "sas") : ready; @@ -219,8 +261,8 @@ export async function runMatrixSelfVerification( const matched = await params.confirmSas(sasSummary.sas, sasSummary); if (!matched) { - handledByMismatch = true; await crypto.mismatchVerificationSas(sasSummary.id); + handledByMismatch = true; throw new Error("Matrix SAS verification was not confirmed."); } @@ -235,23 +277,7 @@ export async function runMatrixSelfVerification( predicate: (summary) => summary.completed, }); requestCompleted = true; - const bootstrap = await client.bootstrapOwnDeviceVerification({ - allowAutomaticCrossSigningReset: false, - verifyOwnIdentity: true, - }); - if (!bootstrap.verification.verified) { - throw new Error( - `Matrix self-verification completed, but full Matrix identity trust is still incomplete: ${ - bootstrap.error ?? formatMatrixOwnerVerificationDiagnostics(bootstrap.verification) - }`, - ); - } - const ownerVerification = await waitForMatrixOwnerVerificationStatus({ client, timeoutMs }); - return { - ...completed, - deviceOwnerVerified: ownerVerification.verified, - ownerVerification, - }; + return await completeMatrixSelfVerification({ client, completed, timeoutMs }); } catch (error) { if (!requestCompleted && !handledByMismatch) { await cancelMatrixSelfVerificationOnFailure({ crypto, request: requested }); diff --git a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts index 0cdb422d153..f359737931f 100644 --- a/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts +++ b/extensions/qa-matrix/src/runners/contract/scenario-runtime-cli.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { spawn as startOpenClawCliProcess } from "node:child_process"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; @@ -69,7 +69,7 @@ export function startMatrixQaOpenClawCli(params: { } | undefined; - const child = spawn(process.execPath, [distEntryPath, ...params.args], { + const child = startOpenClawCliProcess(process.execPath, [distEntryPath, ...params.args], { cwd, env: params.env, stdio: ["pipe", "pipe", "pipe"],