From c17fae4a11d10368c31334c2e421d6df1490dde0 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 23 Feb 2026 00:55:44 -0500 Subject: [PATCH] Matrix-js: harden rust verification request handling --- .../src/matrix/sdk/crypto-bootstrap.test.ts | 35 ++++++++++ .../src/matrix/sdk/crypto-bootstrap.ts | 10 ++- .../matrix/sdk/verification-manager.test.ts | 19 +++++ .../src/matrix/sdk/verification-manager.ts | 70 +++++++++++++------ 4 files changed, 112 insertions(+), 22 deletions(-) diff --git a/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.test.ts b/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.test.ts index c08a5f00e7e..bf0a540eb42 100644 --- a/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.test.ts +++ b/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.test.ts @@ -258,6 +258,41 @@ describe("MatrixCryptoBootstrapper", () => { expect(verificationRequest.accept).toHaveBeenCalledTimes(1); }); + it("still auto-accepts verification when tracking summary throws", async () => { + const deps = createBootstrapperDeps(); + deps.verificationManager.trackVerificationRequest = vi.fn(() => { + throw new Error("summary failure"); + }); + const listeners = new Map void>(); + const crypto = createCryptoApi({ + getDeviceVerificationStatus: vi.fn(async () => ({ + isVerified: () => true, + })), + on: vi.fn((eventName: string, listener: (...args: unknown[]) => void) => { + listeners.set(eventName, listener); + }), + }); + const bootstrapper = new MatrixCryptoBootstrapper( + deps as unknown as MatrixCryptoBootstrapperDeps, + ); + + await bootstrapper.bootstrap(crypto); + + const verificationRequest = { + otherUserId: "@alice:example.org", + isSelfVerification: false, + initiatedByMe: false, + accept: vi.fn(async () => {}), + }; + const listener = Array.from(listeners.entries()).find(([eventName]) => + eventName.toLowerCase().includes("verificationrequest"), + )?.[1]; + expect(listener).toBeTypeOf("function"); + await listener?.(verificationRequest); + + expect(verificationRequest.accept).toHaveBeenCalledTimes(1); + }); + it("registers verification listeners only once across repeated bootstrap calls", async () => { const deps = createBootstrapperDeps(); const crypto = createCryptoApi({ diff --git a/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.ts b/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.ts index 4a737dab7d4..af2a9138434 100644 --- a/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.ts +++ b/extensions/matrix-js/src/matrix/sdk/crypto-bootstrap.ts @@ -221,7 +221,15 @@ export class MatrixCryptoBootstrapper { // Auto-accept incoming verification requests from other users/devices. crypto.on(CryptoEvent.VerificationRequestReceived, async (request) => { const verificationRequest = request as MatrixVerificationRequestLike; - this.deps.verificationManager.trackVerificationRequest(verificationRequest); + try { + this.deps.verificationManager.trackVerificationRequest(verificationRequest); + } catch (err) { + LogService.warn( + "MatrixClientLite", + `Failed to track verification request from ${verificationRequest.otherUserId}:`, + err, + ); + } const otherUserId = verificationRequest.otherUserId; const isSelfVerification = verificationRequest.isSelfVerification; const initiatedByMe = verificationRequest.initiatedByMe; diff --git a/extensions/matrix-js/src/matrix/sdk/verification-manager.test.ts b/extensions/matrix-js/src/matrix/sdk/verification-manager.test.ts index 4d3e24a529f..804db28989f 100644 --- a/extensions/matrix-js/src/matrix/sdk/verification-manager.test.ts +++ b/extensions/matrix-js/src/matrix/sdk/verification-manager.test.ts @@ -84,6 +84,25 @@ class MockVerificationRequest extends EventEmitter implements MatrixVerification } describe("MatrixVerificationManager", () => { + it("handles rust verification requests whose methods getter throws", () => { + const manager = new MatrixVerificationManager(); + const request = new MockVerificationRequest({ + transactionId: "txn-rust-methods", + phase: VerificationPhase.Requested, + }); + Object.defineProperty(request, "methods", { + get() { + throw new Error("not implemented"); + }, + }); + + const summary = manager.trackVerificationRequest(request); + + expect(summary.id).toBeTruthy(); + expect(summary.methods).toEqual([]); + expect(summary.phase).toBe(VerificationPhase.Requested); + }); + it("reuses the same tracked id for repeated transaction IDs", () => { const manager = new MatrixVerificationManager(); const first = new MockVerificationRequest({ diff --git a/extensions/matrix-js/src/matrix/sdk/verification-manager.ts b/extensions/matrix-js/src/matrix/sdk/verification-manager.ts index a9a378aa0b1..87c8836461a 100644 --- a/extensions/matrix-js/src/matrix/sdk/verification-manager.ts +++ b/extensions/matrix-js/src/matrix/sdk/verification-manager.ts @@ -109,9 +109,21 @@ export class MatrixVerificationManager { private readonly trackedVerificationRequests = new WeakSet(); private readonly trackedVerificationVerifiers = new WeakSet(); + private readRequestValue( + request: MatrixVerificationRequestLike, + reader: () => T, + fallback: T, + ): T { + try { + return reader(); + } catch { + return fallback; + } + } + private pruneVerificationSessions(nowMs: number): void { for (const [id, session] of this.verificationSessions) { - const phase = session.request.phase; + const phase = this.readRequestValue(session.request, () => session.request.phase, -1); const isTerminal = phase === VerificationPhase.Done || phase === VerificationPhase.Cancelled; if (isTerminal && nowMs - session.updatedAtMs > TERMINAL_SESSION_RETENTION_MS) { this.verificationSessions.delete(id); @@ -159,21 +171,28 @@ export class MatrixVerificationManager { private buildVerificationSummary(session: MatrixVerificationSession): MatrixVerificationSummary { const request = session.request; - const phase = request.phase; - const canAccept = phase < VerificationPhase.Ready && !request.accepting && !request.declining; + const phase = this.readRequestValue(request, () => request.phase, VerificationPhase.Requested); + const accepting = this.readRequestValue(request, () => request.accepting, false); + const declining = this.readRequestValue(request, () => request.declining, false); + const pending = this.readRequestValue(request, () => request.pending, false); + const methodsRaw = this.readRequestValue(request, () => request.methods, []); + const methods = Array.isArray(methodsRaw) + ? methodsRaw.filter((entry): entry is string => typeof entry === "string") + : []; + const canAccept = phase < VerificationPhase.Ready && !accepting && !declining; return { id: session.id, - transactionId: request.transactionId, - roomId: request.roomId, - otherUserId: request.otherUserId, - otherDeviceId: request.otherDeviceId, - isSelfVerification: request.isSelfVerification, - initiatedByMe: request.initiatedByMe, + transactionId: this.readRequestValue(request, () => request.transactionId, undefined), + roomId: this.readRequestValue(request, () => request.roomId, undefined), + otherUserId: this.readRequestValue(request, () => request.otherUserId, "unknown"), + otherDeviceId: this.readRequestValue(request, () => request.otherDeviceId, undefined), + isSelfVerification: this.readRequestValue(request, () => request.isSelfVerification, false), + initiatedByMe: this.readRequestValue(request, () => request.initiatedByMe, false), phase, phaseName: this.getVerificationPhaseName(phase), - pending: request.pending, - methods: Array.isArray(request.methods) ? request.methods : [], - chosenMethod: request.chosenMethod ?? null, + pending, + methods, + chosenMethod: this.readRequestValue(request, () => request.chosenMethod ?? null, null), canAccept, hasSas: Boolean(session.sasCallbacks), hasReciprocateQr: Boolean(session.reciprocateQrCallbacks), @@ -190,7 +209,8 @@ export class MatrixVerificationManager { return direct; } for (const session of this.verificationSessions.values()) { - if (session.request.transactionId === id) { + const txId = this.readRequestValue(session.request, () => session.request.transactionId, ""); + if (txId === id) { return session; } } @@ -205,8 +225,9 @@ export class MatrixVerificationManager { this.trackedVerificationRequests.add(requestObj); session.request.on(VerificationRequestEvent.Change, () => { this.touchVerificationSession(session); - if (session.request.verifier) { - this.attachVerifierToVerificationSession(session, session.request.verifier); + const verifier = this.readRequestValue(session.request, () => session.request.verifier, null); + if (verifier) { + this.attachVerifierToVerificationSession(session, verifier); } }); } @@ -266,14 +287,20 @@ export class MatrixVerificationManager { trackVerificationRequest(request: MatrixVerificationRequestLike): MatrixVerificationSummary { this.pruneVerificationSessions(Date.now()); - const txId = request.transactionId?.trim(); + const txId = this.readRequestValue(request, () => request.transactionId?.trim(), ""); if (txId) { for (const existing of this.verificationSessions.values()) { - if (existing.request.transactionId === txId) { + const existingTxId = this.readRequestValue( + existing.request, + () => existing.request.transactionId, + "", + ); + if (existingTxId === txId) { existing.request = request; this.ensureVerificationRequestTracked(existing); - if (request.verifier) { - this.attachVerifierToVerificationSession(existing, request.verifier); + const verifier = this.readRequestValue(request, () => request.verifier, null); + if (verifier) { + this.attachVerifierToVerificationSession(existing, verifier); } this.touchVerificationSession(existing); return this.buildVerificationSummary(existing); @@ -292,8 +319,9 @@ export class MatrixVerificationManager { }; this.verificationSessions.set(session.id, session); this.ensureVerificationRequestTracked(session); - if (request.verifier) { - this.attachVerifierToVerificationSession(session, request.verifier); + const verifier = this.readRequestValue(request, () => request.verifier, null); + if (verifier) { + this.attachVerifierToVerificationSession(session, verifier); } return this.buildVerificationSummary(session); }