Matrix: dedupe SAS verification handling

This commit is contained in:
Gustavo Madeira Santana 2026-03-13 14:00:19 +00:00
parent 57e11f1d75
commit 80be1bb356
No known key found for this signature in database
6 changed files with 150 additions and 86 deletions

View File

@ -291,6 +291,61 @@ describe("registerMatrixMonitorEvents verification routing", () => {
expect(body).toContain("SAS decimal: 6158 1986 3513");
});
it("posts SAS notices from summary updates using the room mapped by earlier flow events", async () => {
const { sendMessage, roomEventListener, verificationSummaryListener } = createHarness({
joinedMembersByRoom: {
"!dm:example.org": ["@alice:example.org", "@bot:example.org"],
},
});
if (!verificationSummaryListener) {
throw new Error("verification.summary listener was not registered");
}
roomEventListener("!dm:example.org", {
event_id: "$start-mapped",
sender: "@alice:example.org",
type: "m.key.verification.start",
origin_server_ts: Date.now(),
content: {
transaction_id: "txn-mapped-room",
"m.relates_to": { event_id: "$req-mapped" },
},
});
verificationSummaryListener({
id: "verification-mapped",
transactionId: "txn-mapped-room",
otherUserId: "@alice:example.org",
isSelfVerification: false,
initiatedByMe: false,
phase: 3,
phaseName: "started",
pending: true,
methods: ["m.sas.v1"],
canAccept: false,
hasSas: true,
sas: {
decimal: [1111, 2222, 3333],
emoji: [
["🚀", "Rocket"],
["🦋", "Butterfly"],
["📕", "Book"],
],
},
hasReciprocateQr: false,
completed: false,
createdAt: new Date("2026-02-25T21:42:54.000Z").toISOString(),
updatedAt: new Date("2026-02-25T21:42:55.000Z").toISOString(),
});
await vi.waitFor(() => {
const bodies = (sendMessage.mock.calls as unknown[][]).map((call) =>
String((call[1] as { body?: string } | undefined)?.body ?? ""),
);
expect(bodies.some((body) => body.includes("SAS decimal: 1111 2222 3333"))).toBe(true);
});
});
it("retries SAS notice lookup when start arrives before SAS payload is available", async () => {
vi.useFakeTimers();
const verifications: Array<{

View File

@ -307,9 +307,33 @@ export function createMatrixVerificationEventRouter(params: {
}) {
const routedVerificationEvents = new Set<string>();
const routedVerificationSasFingerprints = new Set<string>();
const routedVerificationStageNotices = new Set<string>();
const verificationFlowRooms = new Map<string, string>();
function rememberVerificationRoom(roomId: string, event: MatrixRawEvent, flowId: string | null) {
for (const candidate of resolveVerificationFlowCandidates({ event, flowId })) {
verificationFlowRooms.set(candidate, roomId);
if (verificationFlowRooms.size > MAX_TRACKED_VERIFICATION_EVENTS) {
const oldest = verificationFlowRooms.keys().next().value;
if (typeof oldest === "string") {
verificationFlowRooms.delete(oldest);
}
}
}
}
function resolveSummaryRoomId(summary: MatrixVerificationSummaryLike): string | null {
return (
trimMaybeString(summary.roomId) ??
trimMaybeString(
summary.transactionId ? verificationFlowRooms.get(summary.transactionId) : null,
) ??
trimMaybeString(verificationFlowRooms.get(summary.id))
);
}
async function routeVerificationSummary(summary: MatrixVerificationSummaryLike): Promise<void> {
const roomId = trimMaybeString(summary.roomId);
const roomId = resolveSummaryRoomId(summary);
if (!roomId || !isActiveVerificationSummary(summary)) {
return;
}
@ -350,6 +374,7 @@ export function createMatrixVerificationEventRouter(params: {
if (!signal) {
return false;
}
rememberVerificationRoom(roomId, event, signal.flowId);
void (async () => {
const flowId = signal.flowId;
@ -381,7 +406,10 @@ export function createMatrixVerificationEventRouter(params: {
const notices: string[] = [];
if (stageNotice) {
notices.push(stageNotice);
const stageKey = `${roomId}:${senderId}:${flowId ?? sourceFingerprint}:${signal.stage}`;
if (trackBounded(routedVerificationStageNotices, stageKey)) {
notices.push(stageNotice);
}
}
if (summary && sasNotice) {
const sasFingerprint = `${summary.id}:${JSON.stringify(summary.sas)}`;

View File

@ -1,4 +1,3 @@
import { VerificationPhase } from "matrix-js-sdk/lib/crypto-api/verification.js";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { MatrixCryptoBootstrapper, type MatrixCryptoBootstrapperDeps } from "./crypto-bootstrap.js";
import type { MatrixCryptoBootstrapApi, MatrixRawEvent } from "./types.js";
@ -418,7 +417,7 @@ describe("MatrixCryptoBootstrapper", () => {
expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(2);
});
it("auto-accepts incoming verification requests from other users", async () => {
it("tracks incoming verification requests from other users", async () => {
const deps = createBootstrapperDeps();
const listeners = new Map<string, (...args: unknown[]) => void>();
const crypto = createCryptoApi({
@ -450,10 +449,10 @@ describe("MatrixCryptoBootstrapper", () => {
expect(deps.verificationManager.trackVerificationRequest).toHaveBeenCalledWith(
verificationRequest,
);
expect(verificationRequest.accept).toHaveBeenCalledTimes(1);
expect(verificationRequest.accept).not.toHaveBeenCalled();
});
it("still auto-accepts verification when tracking summary throws", async () => {
it("does not touch request state when tracking summary throws", async () => {
const deps = createBootstrapperDeps();
deps.verificationManager.trackVerificationRequest = vi.fn(() => {
throw new Error("summary failure");
@ -485,41 +484,6 @@ describe("MatrixCryptoBootstrapper", () => {
expect(listener).toBeTypeOf("function");
await listener?.(verificationRequest);
expect(verificationRequest.accept).toHaveBeenCalledTimes(1);
});
it("skips auto-accept for requests that are no longer requested", async () => {
const deps = createBootstrapperDeps();
const listeners = new Map<string, (...args: unknown[]) => 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<MatrixRawEvent>,
);
await bootstrapper.bootstrap(crypto);
const verificationRequest = {
otherUserId: "@alice:example.org",
isSelfVerification: false,
initiatedByMe: false,
phase: VerificationPhase.Cancelled,
accepting: false,
declining: 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).not.toHaveBeenCalled();
});

View File

@ -1,5 +1,4 @@
import { CryptoEvent } from "matrix-js-sdk/lib/crypto-api/CryptoEvent.js";
import { VerificationPhase } from "matrix-js-sdk/lib/crypto-api/verification.js";
import type { MatrixDecryptBridge } from "./decrypt-bridge.js";
import { LogService } from "./logger.js";
import type { MatrixRecoveryKeyStore } from "./recovery-key-store.js";
@ -273,7 +272,8 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
}
this.verificationHandlerRegistered = true;
// Auto-accept incoming verification requests from other users/devices.
// Track incoming requests; verification lifecycle decisions live in the
// verification manager so acceptance/start/dedupe share one code path.
crypto.on(CryptoEvent.VerificationRequestReceived, async (request) => {
const verificationRequest = request as MatrixVerificationRequestLike;
try {
@ -285,48 +285,6 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
err,
);
}
const otherUserId = verificationRequest.otherUserId;
const isSelfVerification = verificationRequest.isSelfVerification;
const initiatedByMe = verificationRequest.initiatedByMe;
const phase =
typeof verificationRequest.phase === "number"
? verificationRequest.phase
: VerificationPhase.Requested;
const accepting = verificationRequest.accepting === true;
const declining = verificationRequest.declining === true;
if (isSelfVerification || initiatedByMe) {
LogService.debug(
"MatrixClientLite",
`Ignoring ${isSelfVerification ? "self" : "initiated"} verification request from ${otherUserId}`,
);
return;
}
if (phase !== VerificationPhase.Requested || accepting || declining) {
LogService.debug(
"MatrixClientLite",
`Skipping auto-accept for ${otherUserId} in phase=${phase} accepting=${accepting} declining=${declining}`,
);
return;
}
try {
LogService.info(
"MatrixClientLite",
`Auto-accepting verification request from ${otherUserId}`,
);
await verificationRequest.accept();
LogService.info(
"MatrixClientLite",
`Verification request from ${otherUserId} accepted, waiting for SAS...`,
);
} catch (err) {
LogService.warn(
"MatrixClientLite",
`Failed to auto-accept verification from ${otherUserId}:`,
err,
);
}
});
this.deps.decryptBridge.bindCryptoRetrySignals(crypto);

View File

@ -92,6 +92,7 @@ describe("MatrixVerificationManager", () => {
const request = new MockVerificationRequest({
transactionId: "txn-rust-methods",
phase: VerificationPhase.Requested,
initiatedByMe: true,
});
Object.defineProperty(request, "methods", {
get() {
@ -103,7 +104,7 @@ describe("MatrixVerificationManager", () => {
expect(summary.id).toBeTruthy();
expect(summary.methods).toEqual([]);
expect(summary.phase).toBe(VerificationPhase.Requested);
expect(summary.phaseName).toBe("requested");
});
it("reuses the same tracked id for repeated transaction IDs", () => {
@ -302,6 +303,26 @@ describe("MatrixVerificationManager", () => {
expect(manager.getVerificationSas(tracked.id).decimal).toEqual([1234, 5678, 9012]);
});
it("auto-accepts incoming verification requests only once per transaction", async () => {
const request = new MockVerificationRequest({
transactionId: "txn-auto-accept-once",
initiatedByMe: false,
isSelfVerification: false,
phase: VerificationPhase.Requested,
accepting: false,
declining: false,
});
const manager = new MatrixVerificationManager();
manager.trackVerificationRequest(request);
request.emit(VerificationRequestEvent.Change);
manager.trackVerificationRequest(request);
await vi.waitFor(() => {
expect(request.accept).toHaveBeenCalledTimes(1);
});
});
it("auto-confirms inbound SAS after a human-safe delay", async () => {
vi.useFakeTimers();
const confirm = vi.fn(async () => {});

View File

@ -103,6 +103,7 @@ type MatrixVerificationSession = {
verifyPromise?: Promise<void>;
verifyStarted: boolean;
startRequested: boolean;
acceptRequested: boolean;
sasAutoConfirmStarted: boolean;
sasAutoConfirmTimer?: ReturnType<typeof setTimeout>;
sasCallbacks?: MatrixShowSasCallbacks;
@ -262,6 +263,7 @@ export class MatrixVerificationManager {
this.trackedVerificationRequests.add(requestObj);
session.request.on(VerificationRequestEvent.Change, () => {
this.touchVerificationSession(session);
this.maybeAutoAcceptInboundRequest(session);
const verifier = this.readRequestValue(session.request, () => session.request.verifier, null);
if (verifier) {
this.attachVerifierToVerificationSession(session, verifier);
@ -270,6 +272,40 @@ export class MatrixVerificationManager {
});
}
private maybeAutoAcceptInboundRequest(session: MatrixVerificationSession): void {
if (session.acceptRequested) {
return;
}
const request = session.request;
const isSelfVerification = this.readRequestValue(
request,
() => request.isSelfVerification,
false,
);
const initiatedByMe = this.readRequestValue(request, () => request.initiatedByMe, false);
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);
if (isSelfVerification || initiatedByMe) {
return;
}
if (phase !== VerificationPhase.Requested || accepting || declining) {
return;
}
session.acceptRequested = true;
void request
.accept()
.then(() => {
this.touchVerificationSession(session);
})
.catch((err) => {
session.acceptRequested = false;
session.error = err instanceof Error ? err.message : String(err);
this.touchVerificationSession(session);
});
}
private maybeAutoStartInboundSas(session: MatrixVerificationSession): void {
if (session.activeVerifier || session.verifyStarted || session.startRequested) {
return;
@ -450,10 +486,12 @@ export class MatrixVerificationManager {
updatedAtMs: now,
verifyStarted: false,
startRequested: false,
acceptRequested: false,
sasAutoConfirmStarted: false,
};
this.verificationSessions.set(session.id, session);
this.ensureVerificationRequestTracked(session);
this.maybeAutoAcceptInboundRequest(session);
const verifier = this.readRequestValue(request, () => request.verifier, null);
if (verifier) {
this.attachVerifierToVerificationSession(session, verifier);