diff --git a/extensions/matrix/CHANGELOG.md b/extensions/matrix/CHANGELOG.md index 9276d92c199..7793ebf1044 100644 --- a/extensions/matrix/CHANGELOG.md +++ b/extensions/matrix/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- Matrix/E2EE: close the owner-side device verification loop when SAS lands via the CLI. `verify confirm-sas` now (1) awaits the rust-crypto verifier promise so the done-exchange and any cross-signing uploads triggered by `crossSignDevice` settle before the verb returns, (2) cross-signs the bot device on the auto-confirmed inbound SAS path (previously skipped), and (3) calls `trustOwnIdentityAfterSelfVerification` from the standalone `confirmMatrixVerificationSas` action so the operator's Element X clears the "Verify" prompt without waiting for a passive sync tick [AI-assisted]. Thanks @nklock. - Matrix/E2EE: stabilize recovery and broken-device QA flows while avoiding device-cleanup sync races that could leave shutdown-time crypto work running. Thanks @gumadeiras. ## 2026.4.25 diff --git a/extensions/matrix/src/matrix/actions/verification.test.ts b/extensions/matrix/src/matrix/actions/verification.test.ts index d7b33a9de98..fbf5f6a2291 100644 --- a/extensions/matrix/src/matrix/actions/verification.test.ts +++ b/extensions/matrix/src/matrix/actions/verification.test.ts @@ -38,10 +38,12 @@ let getMatrixVerificationStatus: typeof import("./verification.js").getMatrixVer let restoreMatrixRoomKeyBackup: typeof import("./verification.js").restoreMatrixRoomKeyBackup; let runMatrixSelfVerification: typeof import("./verification.js").runMatrixSelfVerification; let startMatrixVerification: typeof import("./verification.js").startMatrixVerification; +let confirmMatrixVerificationSas: typeof import("./verification.js").confirmMatrixVerificationSas; describe("matrix verification actions", () => { beforeAll(async () => { ({ + confirmMatrixVerificationSas, getMatrixEncryptionStatus, getMatrixRoomKeyBackupStatus, getMatrixVerificationStatus, @@ -1001,4 +1003,74 @@ describe("matrix verification actions", () => { reason: "OpenClaw self-verification did not complete", }); }); + + it("confirmMatrixVerificationSas calls trustOwnIdentityAfterSelfVerification on a self-verification", async () => { + const crypto = { + confirmVerificationSas: vi.fn(async () => ({ + completed: true, + hasSas: true, + id: "verification-self", + isSelfVerification: true, + phaseName: "done", + transactionId: "tx-self", + })), + }; + const trustOwnIdentityAfterSelfVerification = vi.fn(async () => {}); + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ crypto, trustOwnIdentityAfterSelfVerification }); + }); + + const summary = await confirmMatrixVerificationSas("verification-self"); + + expect(crypto.confirmVerificationSas).toHaveBeenCalledWith("verification-self"); + expect(trustOwnIdentityAfterSelfVerification).toHaveBeenCalledTimes(1); + expect(summary.isSelfVerification).toBe(true); + }); + + it("confirmMatrixVerificationSas does not call trustOwnIdentityAfterSelfVerification on a non-self verification", async () => { + const crypto = { + confirmVerificationSas: vi.fn(async () => ({ + completed: true, + hasSas: true, + id: "verification-remote", + isSelfVerification: false, + phaseName: "done", + transactionId: "tx-remote", + })), + }; + const trustOwnIdentityAfterSelfVerification = vi.fn(async () => {}); + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ crypto, trustOwnIdentityAfterSelfVerification }); + }); + + const summary = await confirmMatrixVerificationSas("verification-remote"); + + expect(crypto.confirmVerificationSas).toHaveBeenCalledWith("verification-remote"); + expect(trustOwnIdentityAfterSelfVerification).not.toHaveBeenCalled(); + expect(summary.isSelfVerification).toBe(false); + }); + + it("confirmMatrixVerificationSas does not trust own identity when self-verification failed", async () => { + const crypto = { + confirmVerificationSas: vi.fn(async () => ({ + completed: false, + error: "verifier rejected mid-protocol", + hasSas: true, + id: "verification-self", + isSelfVerification: true, + phaseName: "started", + transactionId: "tx-self", + })), + }; + const trustOwnIdentityAfterSelfVerification = vi.fn(async () => {}); + withStartedActionClientMock.mockImplementation(async (_opts, run) => { + return await run({ crypto, trustOwnIdentityAfterSelfVerification }); + }); + + const summary = await confirmMatrixVerificationSas("verification-self"); + + expect(crypto.confirmVerificationSas).toHaveBeenCalledWith("verification-self"); + expect(trustOwnIdentityAfterSelfVerification).not.toHaveBeenCalled(); + expect(summary.error).toMatch(/verifier rejected mid-protocol/); + }); }); diff --git a/extensions/matrix/src/matrix/actions/verification.ts b/extensions/matrix/src/matrix/actions/verification.ts index c23425e2256..72bbe87050a 100644 --- a/extensions/matrix/src/matrix/actions/verification.ts +++ b/extensions/matrix/src/matrix/actions/verification.ts @@ -437,7 +437,16 @@ export async function confirmMatrixVerificationSas( return await withStartedActionClient(opts, async (client) => { const crypto = requireCrypto(client, opts); await ensureMatrixVerificationDmTracked(crypto, opts); - return await crypto.confirmVerificationSas(resolveVerificationId(requestId)); + const summary = await crypto.confirmVerificationSas(resolveVerificationId(requestId)); + // For self-verifications, mirror the trust-own-identity step that the + // higher-level runMatrixSelfVerification path already performs at + // completeMatrixSelfVerification: cross-sign the operator's master key + // from the bot side so Element X clears the "Verify" prompt without + // waiting for a passive sync tick. Non-self verifications are a no-op. + if (summary.isSelfVerification && summary.completed && !summary.error) { + await client.trustOwnIdentityAfterSelfVerification?.(); + } + return summary; }); } diff --git a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts index 65b56777930..4c8dc321ecf 100644 --- a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts +++ b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts @@ -520,7 +520,7 @@ describe("MatrixVerificationManager", () => { } }); - it("does not cross-sign the other own device after auto-confirmed self-verification SAS", async () => { + it("cross-signs the other own device after auto-confirmed self-verification SAS", async () => { vi.useFakeTimers(); const { confirm, verifier } = createSasVerifierFixture({ decimal: [6158, 1986, 3513], @@ -541,12 +541,81 @@ describe("MatrixVerificationManager", () => { await vi.advanceTimersByTimeAsync(30_100); expect(confirm).toHaveBeenCalledTimes(1); - expect(trustOwnDeviceAfterSas).not.toHaveBeenCalled(); + expect(trustOwnDeviceAfterSas).toHaveBeenCalledWith("OTHERDEVICE"); } finally { vi.useRealTimers(); } }); + it("confirmVerificationSas awaits the verifier's verify promise before resolving", async () => { + let resolveVerify!: () => void; + const verifyPromise = new Promise((res) => { + resolveVerify = res; + }); + const verifyImpl = vi.fn(() => verifyPromise); + const { confirm, verifier } = createSasVerifierFixture({ + decimal: [111, 222, 333], + emoji: [["cat", "Cat"]], + verifyImpl, + }); + const trustOwnDeviceAfterSas = vi.fn(async () => {}); + const request = new MockVerificationRequest({ + isSelfVerification: true, + otherDeviceId: "OTHERDEVICE", + transactionId: "txn-await-verify", + initiatedByMe: true, + verifier, + }); + const manager = new MatrixVerificationManager({ trustOwnDeviceAfterSas }); + const tracked = manager.trackVerificationRequest(request); + + await manager.startVerification(tracked.id, "sas"); + expect(verifyImpl).toHaveBeenCalledTimes(1); + + let confirmResolved = false; + const confirmPromise = manager.confirmVerificationSas(tracked.id).then(() => { + confirmResolved = true; + }); + + // Yield once so confirmSasForSession + trustOwnDeviceAfterSas finish, but + // verifyPromise stays pending. confirmVerificationSas must still be + // blocked awaiting verifyPromise. + await Promise.resolve(); + await Promise.resolve(); + expect(confirm).toHaveBeenCalledTimes(1); + expect(trustOwnDeviceAfterSas).toHaveBeenCalledWith("OTHERDEVICE"); + expect(confirmResolved).toBe(false); + + resolveVerify(); + await confirmPromise; + expect(confirmResolved).toBe(true); + }); + + it("confirmVerificationSas surfaces a verifier-promise rejection on session.error", async () => { + const verifyImpl = vi.fn(async () => { + throw new Error("verifier rejected mid-protocol"); + }); + const { verifier } = createSasVerifierFixture({ + decimal: [111, 222, 333], + emoji: [["cat", "Cat"]], + verifyImpl, + }); + const request = new MockVerificationRequest({ + isSelfVerification: true, + otherDeviceId: "OTHERDEVICE", + transactionId: "txn-verify-rejects", + initiatedByMe: true, + verifier, + }); + const manager = new MatrixVerificationManager(); + const tracked = manager.trackVerificationRequest(request); + + await manager.startVerification(tracked.id, "sas"); + const summary = await manager.confirmVerificationSas(tracked.id); + + expect(summary.error).toMatch(/verifier rejected mid-protocol/); + }); + it("does not auto-confirm SAS for verifications initiated by this device", async () => { vi.useFakeTimers(); const confirm = vi.fn(async () => {}); diff --git a/extensions/matrix/src/matrix/sdk/verification-manager.ts b/extensions/matrix/src/matrix/sdk/verification-manager.ts index 08698a61cf8..cfdd9bc7f63 100644 --- a/extensions/matrix/src/matrix/sdk/verification-manager.ts +++ b/extensions/matrix/src/matrix/sdk/verification-manager.ts @@ -504,7 +504,11 @@ export class MatrixVerificationManager { return; } session.sasAutoConfirmStarted = true; - void this.confirmSasForSession(session, callbacks, { trustOwnDevice: false }) + // For self-verifications, trustOwnDeviceAfterConfirmedSas is gated on + // isSelfVerification, so non-self requests remain unaffected. Without + // this, the bot's own device never gets cross-signed when SAS lands + // via the auto-confirm timer (initiated remotely). + void this.confirmSasForSession(session, callbacks, { trustOwnDevice: true }) .then(() => { this.touchVerificationSession(session); }) @@ -732,6 +736,15 @@ export class MatrixVerificationManager { session.sasCallbacks = callbacks; session.sasAutoConfirmStarted = true; await this.confirmSasForSession(session, callbacks); + // Wait for the rust-crypto verifier to fully resolve (done-exchange + any + // pending cross-signing uploads triggered by trustOwnDeviceAfterSas) so + // the operator's client sees a settled state on the next /keys/query. + // verifyPromise is set inside ensureVerificationStarted and already + // funnels its own rejection into session.error, so awaiting it here + // cannot double-throw. + if (session.verifyPromise) { + await session.verifyPromise; + } this.touchVerificationSession(session); return this.buildVerificationSummary(session); }