From ac276e0470f00412468f298b35a779c39e55e801 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Thu, 23 Apr 2026 09:05:28 -0400 Subject: [PATCH] fix: complete Matrix self-verification trust --- .../src/matrix/actions/verification.test.ts | 20 ++++-- .../matrix/src/matrix/actions/verification.ts | 17 +++-- extensions/matrix/src/matrix/sdk.test.ts | 23 ++++++ extensions/matrix/src/matrix/sdk.ts | 43 +++++++++++- .../matrix/sdk/verification-manager.test.ts | 70 +++++++++++++++++++ .../src/matrix/sdk/verification-manager.ts | 35 +++++++++- 6 files changed, 190 insertions(+), 18 deletions(-) diff --git a/extensions/matrix/src/matrix/actions/verification.test.ts b/extensions/matrix/src/matrix/actions/verification.test.ts index 63eeb7955c0..c7b96e2e6a1 100644 --- a/extensions/matrix/src/matrix/actions/verification.test.ts +++ b/extensions/matrix/src/matrix/actions/verification.test.ts @@ -370,8 +370,9 @@ describe("matrix verification actions", () => { expect(crypto.confirmVerificationSas).toHaveBeenCalledWith("verification-1"); expect(bootstrapOwnDeviceVerification).toHaveBeenCalledWith({ allowAutomaticCrossSigningReset: false, + strict: false, }); - expect(getOwnDeviceVerificationStatus).toHaveBeenCalled(); + expect(getOwnDeviceVerificationStatus).not.toHaveBeenCalled(); }); it("does not complete self-verification until the OpenClaw device has full Matrix identity trust", async () => { @@ -407,10 +408,16 @@ describe("matrix verification actions", () => { .mockResolvedValueOnce(mockVerifiedOwnerStatus()); const bootstrapOwnDeviceVerification = vi.fn(async () => ({ success: true, - verification: mockVerifiedOwnerStatus(), + verification: mockUnverifiedOwnerStatus(), })); + const trustOwnIdentityAfterSelfVerification = vi.fn(async () => {}); withStartedActionClientMock.mockImplementation(async (_opts, run) => { - return await run({ bootstrapOwnDeviceVerification, crypto, getOwnDeviceVerificationStatus }); + return await run({ + bootstrapOwnDeviceVerification, + crypto, + getOwnDeviceVerificationStatus, + trustOwnIdentityAfterSelfVerification, + }); }); await expect( @@ -424,6 +431,7 @@ describe("matrix verification actions", () => { }); expect(getOwnDeviceVerificationStatus).toHaveBeenCalledTimes(2); + expect(trustOwnIdentityAfterSelfVerification).toHaveBeenCalledTimes(1); }); it("waits for SAS data without restarting an already-started self-verification", async () => { @@ -650,10 +658,14 @@ describe("matrix verification actions", () => { await expect( runMatrixSelfVerification({ confirmSas: vi.fn(async () => true), timeoutMs: 30 }), ).rejects.toThrow( - "Matrix self-verification completed, but full Matrix identity trust is still incomplete", + "Timed out waiting for Matrix self-verification to establish full Matrix identity trust", ); expect(crypto.cancelVerification).not.toHaveBeenCalled(); + expect(bootstrapOwnDeviceVerification).toHaveBeenCalledWith({ + allowAutomaticCrossSigningReset: false, + strict: false, + }); }); it("cancels the pending self-verification request when acceptance times out", async () => { diff --git a/extensions/matrix/src/matrix/actions/verification.ts b/extensions/matrix/src/matrix/actions/verification.ts index 8c16875db35..4a060a64705 100644 --- a/extensions/matrix/src/matrix/actions/verification.ts +++ b/extensions/matrix/src/matrix/actions/verification.ts @@ -210,18 +210,17 @@ async function completeMatrixSelfVerification(params: { }): Promise { const bootstrap = await params.client.bootstrapOwnDeviceVerification({ allowAutomaticCrossSigningReset: false, + strict: false, }); if (!bootstrap.verification.verified) { - throw new Error( - `Matrix self-verification completed, but full Matrix identity trust is still incomplete: ${ - bootstrap.error ?? formatMatrixOwnerVerificationDiagnostics(bootstrap.verification) - }`, - ); + await params.client.trustOwnIdentityAfterSelfVerification?.(); } - const ownerVerification = await waitForMatrixOwnerVerificationStatus({ - client: params.client, - timeoutMs: params.timeoutMs, - }); + const ownerVerification = bootstrap.verification.verified + ? bootstrap.verification + : await waitForMatrixOwnerVerificationStatus({ + client: params.client, + timeoutMs: params.timeoutMs, + }); return { ...params.completed, deviceOwnerVerified: ownerVerification.verified, diff --git a/extensions/matrix/src/matrix/sdk.test.ts b/extensions/matrix/src/matrix/sdk.test.ts index 205f326194b..ae292c1a685 100644 --- a/extensions/matrix/src/matrix/sdk.test.ts +++ b/extensions/matrix/src/matrix/sdk.test.ts @@ -1222,6 +1222,29 @@ describe("MatrixClient crypto bootstrapping", () => { ); }); + it("trusts the own Matrix identity after completed self-verification", async () => { + const verifyOwnIdentity = vi.fn(async () => ({})); + const freeOwnIdentity = vi.fn(); + matrixJsClient.getCrypto = vi.fn(() => ({ + on: vi.fn(), + getOwnIdentity: vi.fn(async () => ({ + free: freeOwnIdentity, + isVerified: () => false, + verify: verifyOwnIdentity, + })), + requestOwnUserVerification: vi.fn(async () => null), + })); + + const client = new MatrixClient("https://matrix.example.org", "token", { + encryption: true, + }); + + await client.trustOwnIdentityAfterSelfVerification(); + + expect(verifyOwnIdentity).toHaveBeenCalledTimes(1); + expect(freeOwnIdentity).toHaveBeenCalledTimes(1); + }); + it("retries bootstrap with forced reset when initial publish/verification is incomplete", async () => { matrixJsClient.getCrypto = vi.fn(() => ({ on: vi.fn() })); const client = new MatrixClient("https://matrix.example.org", "token", { diff --git a/extensions/matrix/src/matrix/sdk.ts b/extensions/matrix/src/matrix/sdk.ts index db4769b5fcf..0a1338536d0 100644 --- a/extensions/matrix/src/matrix/sdk.ts +++ b/extensions/matrix/src/matrix/sdk.ts @@ -165,12 +165,13 @@ const MATRIX_AUTOMATIC_REPAIR_BOOTSTRAP_OPTIONS = { function createMatrixExplicitBootstrapOptions(params?: { allowAutomaticCrossSigningReset?: boolean; forceResetCrossSigning?: boolean; + strict?: boolean; }): MatrixCryptoBootstrapOptions { return { forceResetCrossSigning: params?.forceResetCrossSigning === true, allowAutomaticCrossSigningReset: params?.allowAutomaticCrossSigningReset !== false, allowSecretStorageRecreateWithoutRecoveryKey: true, - strict: true, + strict: params?.strict !== false, }; } @@ -367,7 +368,15 @@ export class MatrixClient { return; } - this.verificationManager ??= new runtime.MatrixVerificationManager(); + this.verificationManager ??= new runtime.MatrixVerificationManager({ + trustOwnDeviceAfterSas: async (deviceId: string) => { + const crypto = this.client.getCrypto() as MatrixCryptoBootstrapApi | undefined; + if (typeof crypto?.crossSignDevice !== "function") { + throw new Error("Matrix crypto backend does not support cross-signing devices"); + } + await crypto.crossSignDevice(deviceId); + }, + }); this.cryptoBootstrapper ??= new runtime.MatrixCryptoBootstrapper({ getUserId: () => this.getUserId(), getPassword: () => this.password, @@ -1127,6 +1136,35 @@ export class MatrixClient { }; } + async trustOwnIdentityAfterSelfVerification(): Promise { + if (!this.encryptionEnabled) { + return; + } + + await this.ensureStartedForCryptoControlPlane(); + await this.ensureCryptoSupportInitialized(); + const crypto = this.client.getCrypto() as MatrixCryptoBootstrapApi | undefined; + const ownIdentity = + crypto && typeof crypto.getOwnIdentity === "function" + ? await crypto.getOwnIdentity().catch(() => undefined) + : undefined; + if (!ownIdentity) { + return; + } + + try { + if (typeof ownIdentity.isVerified === "function" && ownIdentity.isVerified()) { + return; + } + if (typeof ownIdentity.verify !== "function") { + throw new Error("Matrix crypto backend does not support trusting own identity"); + } + await ownIdentity.verify(); + } finally { + ownIdentity.free?.(); + } + } + async verifyWithRecoveryKey( rawRecoveryKey: string, ): Promise { @@ -1486,6 +1524,7 @@ export class MatrixClient { allowAutomaticCrossSigningReset?: boolean; recoveryKey?: string; forceResetCrossSigning?: boolean; + strict?: boolean; }): Promise { const pendingVerifications = async (): Promise => this.crypto ? (await this.crypto.listVerifications()).length : 0; diff --git a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts index 072a1035cb0..597daf05d3d 100644 --- a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts +++ b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts @@ -259,6 +259,49 @@ describe("MatrixVerificationManager", () => { expect(mismatch).toHaveBeenCalledTimes(1); }); + it("cross-signs the other own device after confirmed self-verification SAS", async () => { + const { confirm, verifier } = createSasVerifierFixture({ + decimal: [111, 222, 333], + emoji: [["cat", "cat"]], + }); + const trustOwnDeviceAfterSas = vi.fn(async () => {}); + const request = new MockVerificationRequest({ + isSelfVerification: true, + otherDeviceId: "OTHERDEVICE", + transactionId: "txn-self-sas", + verifier, + }); + const manager = new MatrixVerificationManager({ trustOwnDeviceAfterSas }); + const tracked = manager.trackVerificationRequest(request); + + await manager.startVerification(tracked.id, "sas"); + await manager.confirmVerificationSas(tracked.id); + + expect(confirm).toHaveBeenCalledTimes(1); + expect(trustOwnDeviceAfterSas).toHaveBeenCalledWith("OTHERDEVICE"); + }); + + it("does not cross-sign non-self SAS verifications", async () => { + const { verifier } = createSasVerifierFixture({ + decimal: [111, 222, 333], + emoji: [["cat", "cat"]], + }); + const trustOwnDeviceAfterSas = vi.fn(async () => {}); + const request = new MockVerificationRequest({ + isSelfVerification: false, + otherDeviceId: "OTHERDEVICE", + transactionId: "txn-remote-sas", + verifier, + }); + const manager = new MatrixVerificationManager({ trustOwnDeviceAfterSas }); + const tracked = manager.trackVerificationRequest(request); + + await manager.startVerification(tracked.id, "sas"); + await manager.confirmVerificationSas(tracked.id); + + expect(trustOwnDeviceAfterSas).not.toHaveBeenCalled(); + }); + it("auto-starts an incoming verifier exposed via request change events", async () => { const { verifier, verify } = createSasVerifierFixture({ decimal: [6158, 1986, 3513], @@ -438,6 +481,33 @@ describe("MatrixVerificationManager", () => { } }); + it("cross-signs the other own device after auto-confirmed self-verification SAS", async () => { + vi.useFakeTimers(); + const { confirm, verifier } = createSasVerifierFixture({ + decimal: [6158, 1986, 3513], + emoji: [["gift", "Gift"]], + }); + const trustOwnDeviceAfterSas = vi.fn(async () => {}); + const request = new MockVerificationRequest({ + isSelfVerification: true, + otherDeviceId: "OTHERDEVICE", + transactionId: "txn-auto-confirm-self", + initiatedByMe: false, + verifier, + }); + try { + const manager = new MatrixVerificationManager({ trustOwnDeviceAfterSas }); + manager.trackVerificationRequest(request); + + await vi.advanceTimersByTimeAsync(30_100); + + expect(confirm).toHaveBeenCalledTimes(1); + expect(trustOwnDeviceAfterSas).toHaveBeenCalledWith("OTHERDEVICE"); + } finally { + vi.useRealTimers(); + } + }); + 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 29e78d3c6c8..00133387f0f 100644 --- a/extensions/matrix/src/matrix/sdk/verification-manager.ts +++ b/extensions/matrix/src/matrix/sdk/verification-manager.ts @@ -52,6 +52,7 @@ export type MatrixVerificationSummary = { }; type MatrixVerificationSummaryListener = (summary: MatrixVerificationSummary) => void; +type MatrixVerificationOwnerTrustCallback = (deviceId: string) => Promise; export type MatrixShowSasCallbacks = { sas: { @@ -153,6 +154,12 @@ export class MatrixVerificationManager { private readonly trackedVerificationVerifiers = new WeakSet(); private readonly summaryListeners = new Set(); + constructor( + private readonly opts: { + trustOwnDeviceAfterSas?: MatrixVerificationOwnerTrustCallback; + } = {}, + ) {} + private readRequestValue( request: MatrixVerificationRequestLike, reader: () => T, @@ -493,8 +500,7 @@ export class MatrixVerificationManager { return; } session.sasAutoConfirmStarted = true; - void callbacks - .confirm() + void this.confirmSasForSession(session, callbacks) .then(() => { this.touchVerificationSession(session); }) @@ -505,6 +511,14 @@ export class MatrixVerificationManager { }, SAS_AUTO_CONFIRM_DELAY_MS); } + private async confirmSasForSession( + session: MatrixVerificationSession, + callbacks: MatrixShowSasCallbacks, + ): Promise { + await callbacks.confirm(); + await this.trustOwnDeviceAfterConfirmedSas(session); + } + private ensureVerificationStarted(session: MatrixVerificationSession): void { if (!session.activeVerifier || session.verifyStarted) { return; @@ -522,6 +536,21 @@ export class MatrixVerificationManager { }); } + private async trustOwnDeviceAfterConfirmedSas(session: MatrixVerificationSession): Promise { + if (!this.readRequestValue(session.request, () => session.request.isSelfVerification, false)) { + return; + } + const deviceId = this.readRequestValue( + session.request, + () => session.request.otherDeviceId?.trim(), + "", + ); + if (!deviceId || !this.opts.trustOwnDeviceAfterSas) { + return; + } + await this.opts.trustOwnDeviceAfterSas(deviceId); + } + onSummaryChanged(listener: MatrixVerificationSummaryListener): () => void { this.summaryListeners.add(listener); return () => { @@ -695,7 +724,7 @@ export class MatrixVerificationManager { this.clearSasAutoConfirmTimer(session); session.sasCallbacks = callbacks; session.sasAutoConfirmStarted = true; - await callbacks.confirm(); + await this.confirmSasForSession(session, callbacks); this.touchVerificationSession(session); return this.buildVerificationSummary(session); }