mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-23 14:18:11 +00:00
fix(matrix-js): harden sas verification flow
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
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";
|
||||
@@ -293,6 +294,41 @@ describe("MatrixCryptoBootstrapper", () => {
|
||||
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();
|
||||
});
|
||||
|
||||
it("registers verification listeners only once across repeated bootstrap calls", async () => {
|
||||
const deps = createBootstrapperDeps();
|
||||
const crypto = createCryptoApi({
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
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";
|
||||
@@ -233,6 +234,12 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
|
||||
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(
|
||||
@@ -241,6 +248,13 @@ export class MatrixCryptoBootstrapper<TRawEvent extends MatrixRawEvent> {
|
||||
);
|
||||
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(
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { VerificationPhase } from "matrix-js-sdk/lib/crypto-api/verification.js";
|
||||
import {
|
||||
VerificationPhase,
|
||||
VerificationRequestEvent,
|
||||
} from "matrix-js-sdk/lib/crypto-api/verification.js";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
MatrixVerificationManager,
|
||||
@@ -154,6 +157,8 @@ describe("MatrixVerificationManager", () => {
|
||||
|
||||
const started = await manager.startVerification(tracked.id, "sas");
|
||||
expect(started.hasSas).toBe(true);
|
||||
expect(started.sas?.decimal).toEqual([111, 222, 333]);
|
||||
expect(started.sas?.emoji?.length).toBe(3);
|
||||
|
||||
const sas = manager.getVerificationSas(tracked.id);
|
||||
expect(sas.decimal).toEqual([111, 222, 333]);
|
||||
@@ -166,6 +171,91 @@ describe("MatrixVerificationManager", () => {
|
||||
expect(mismatch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("auto-starts an incoming verifier exposed via request change events", async () => {
|
||||
const verify = vi.fn(async () => {});
|
||||
const verifier = new MockVerifier(
|
||||
{
|
||||
sas: {
|
||||
decimal: [6158, 1986, 3513],
|
||||
emoji: [
|
||||
["gift", "Gift"],
|
||||
["globe", "Globe"],
|
||||
["horse", "Horse"],
|
||||
],
|
||||
},
|
||||
confirm: vi.fn(async () => {}),
|
||||
mismatch: vi.fn(),
|
||||
cancel: vi.fn(),
|
||||
},
|
||||
null,
|
||||
verify,
|
||||
);
|
||||
const request = new MockVerificationRequest({
|
||||
transactionId: "txn-incoming-change",
|
||||
verifier: undefined,
|
||||
});
|
||||
const manager = new MatrixVerificationManager();
|
||||
const tracked = manager.trackVerificationRequest(request);
|
||||
|
||||
request.verifier = verifier;
|
||||
request.emit(VerificationRequestEvent.Change);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(verify).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
const summary = manager.listVerifications().find((item) => item.id === tracked.id);
|
||||
expect(summary?.hasSas).toBe(true);
|
||||
expect(summary?.sas?.decimal).toEqual([6158, 1986, 3513]);
|
||||
expect(manager.getVerificationSas(tracked.id).decimal).toEqual([6158, 1986, 3513]);
|
||||
});
|
||||
|
||||
it("auto-starts inbound SAS when request becomes ready without a verifier", async () => {
|
||||
const verify = vi.fn(async () => {});
|
||||
const verifier = new MockVerifier(
|
||||
{
|
||||
sas: {
|
||||
decimal: [1234, 5678, 9012],
|
||||
emoji: [
|
||||
["gift", "Gift"],
|
||||
["rocket", "Rocket"],
|
||||
["butterfly", "Butterfly"],
|
||||
],
|
||||
},
|
||||
confirm: vi.fn(async () => {}),
|
||||
mismatch: vi.fn(),
|
||||
cancel: vi.fn(),
|
||||
},
|
||||
null,
|
||||
verify,
|
||||
);
|
||||
const request = new MockVerificationRequest({
|
||||
transactionId: "txn-auto-start-sas",
|
||||
initiatedByMe: false,
|
||||
verifier: undefined,
|
||||
});
|
||||
request.startVerification = vi.fn(async (_method: string) => {
|
||||
request.phase = VerificationPhase.Started;
|
||||
request.verifier = verifier;
|
||||
return verifier;
|
||||
});
|
||||
const manager = new MatrixVerificationManager();
|
||||
const tracked = manager.trackVerificationRequest(request);
|
||||
|
||||
request.phase = VerificationPhase.Ready;
|
||||
request.emit(VerificationRequestEvent.Change);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(request.startVerification).toHaveBeenCalledWith("m.sas.v1");
|
||||
});
|
||||
await vi.waitFor(() => {
|
||||
expect(verify).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
const summary = manager.listVerifications().find((item) => item.id === tracked.id);
|
||||
expect(summary?.hasSas).toBe(true);
|
||||
expect(summary?.sas?.decimal).toEqual([1234, 5678, 9012]);
|
||||
expect(manager.getVerificationSas(tracked.id).decimal).toEqual([1234, 5678, 9012]);
|
||||
});
|
||||
|
||||
it("prunes stale terminal sessions during list operations", () => {
|
||||
const now = new Date("2026-02-08T15:00:00.000Z").getTime();
|
||||
const nowSpy = vi.spyOn(Date, "now");
|
||||
|
||||
@@ -22,6 +22,10 @@ export type MatrixVerificationSummary = {
|
||||
chosenMethod?: string | null;
|
||||
canAccept: boolean;
|
||||
hasSas: boolean;
|
||||
sas?: {
|
||||
decimal?: [number, number, number];
|
||||
emoji?: Array<[string, string]>;
|
||||
};
|
||||
hasReciprocateQr: boolean;
|
||||
completed: boolean;
|
||||
error?: string;
|
||||
@@ -96,6 +100,7 @@ type MatrixVerificationSession = {
|
||||
activeVerifier?: MatrixVerifierLike;
|
||||
verifyPromise?: Promise<void>;
|
||||
verifyStarted: boolean;
|
||||
startRequested: boolean;
|
||||
sasCallbacks?: MatrixShowSasCallbacks;
|
||||
reciprocateQrCallbacks?: MatrixShowQrCodeCallbacks;
|
||||
};
|
||||
@@ -179,6 +184,10 @@ export class MatrixVerificationManager {
|
||||
const methods = Array.isArray(methodsRaw)
|
||||
? methodsRaw.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
const sasCallbacks = session.sasCallbacks ?? session.activeVerifier?.getShowSasCallbacks();
|
||||
if (sasCallbacks) {
|
||||
session.sasCallbacks = sasCallbacks;
|
||||
}
|
||||
const canAccept = phase < VerificationPhase.Ready && !accepting && !declining;
|
||||
return {
|
||||
id: session.id,
|
||||
@@ -194,7 +203,13 @@ export class MatrixVerificationManager {
|
||||
methods,
|
||||
chosenMethod: this.readRequestValue(request, () => request.chosenMethod ?? null, null),
|
||||
canAccept,
|
||||
hasSas: Boolean(session.sasCallbacks),
|
||||
hasSas: Boolean(sasCallbacks),
|
||||
sas: sasCallbacks
|
||||
? {
|
||||
decimal: sasCallbacks.sas.decimal,
|
||||
emoji: sasCallbacks.sas.emoji,
|
||||
}
|
||||
: undefined,
|
||||
hasReciprocateQr: Boolean(session.reciprocateQrCallbacks),
|
||||
completed: phase === VerificationPhase.Done,
|
||||
error: session.error,
|
||||
@@ -229,9 +244,56 @@ export class MatrixVerificationManager {
|
||||
if (verifier) {
|
||||
this.attachVerifierToVerificationSession(session, verifier);
|
||||
}
|
||||
this.maybeAutoStartInboundSas(session);
|
||||
});
|
||||
}
|
||||
|
||||
private maybeAutoStartInboundSas(session: MatrixVerificationSession): void {
|
||||
if (session.activeVerifier || session.verifyStarted || session.startRequested) {
|
||||
return;
|
||||
}
|
||||
if (this.readRequestValue(session.request, () => session.request.initiatedByMe, true)) {
|
||||
return;
|
||||
}
|
||||
const phase = this.readRequestValue(
|
||||
session.request,
|
||||
() => session.request.phase,
|
||||
VerificationPhase.Requested,
|
||||
);
|
||||
if (phase < VerificationPhase.Ready || phase >= VerificationPhase.Cancelled) {
|
||||
return;
|
||||
}
|
||||
const methodsRaw = this.readRequestValue<unknown>(
|
||||
session.request,
|
||||
() => session.request.methods,
|
||||
[],
|
||||
);
|
||||
const methods = Array.isArray(methodsRaw)
|
||||
? methodsRaw.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
const chosenMethod = this.readRequestValue(
|
||||
session.request,
|
||||
() => session.request.chosenMethod,
|
||||
null,
|
||||
);
|
||||
const supportsSas =
|
||||
methods.includes(VerificationMethod.Sas) || chosenMethod === VerificationMethod.Sas;
|
||||
if (!supportsSas) {
|
||||
return;
|
||||
}
|
||||
|
||||
session.startRequested = true;
|
||||
void session.request
|
||||
.startVerification(VerificationMethod.Sas)
|
||||
.then((verifier) => {
|
||||
this.attachVerifierToVerificationSession(session, verifier);
|
||||
this.touchVerificationSession(session);
|
||||
})
|
||||
.catch(() => {
|
||||
session.startRequested = false;
|
||||
});
|
||||
}
|
||||
|
||||
private attachVerifierToVerificationSession(
|
||||
session: MatrixVerificationSession,
|
||||
verifier: MatrixVerifierLike,
|
||||
@@ -250,6 +312,7 @@ export class MatrixVerificationManager {
|
||||
|
||||
const verifierObj = verifier as unknown as object;
|
||||
if (this.trackedVerificationVerifiers.has(verifierObj)) {
|
||||
this.ensureVerificationStarted(session);
|
||||
return;
|
||||
}
|
||||
this.trackedVerificationVerifiers.add(verifierObj);
|
||||
@@ -266,6 +329,7 @@ export class MatrixVerificationManager {
|
||||
session.error = err instanceof Error ? err.message : String(err);
|
||||
this.touchVerificationSession(session);
|
||||
});
|
||||
this.ensureVerificationStarted(session);
|
||||
}
|
||||
|
||||
private ensureVerificationStarted(session: MatrixVerificationSession): void {
|
||||
@@ -316,6 +380,7 @@ export class MatrixVerificationManager {
|
||||
createdAtMs: now,
|
||||
updatedAtMs: now,
|
||||
verifyStarted: false,
|
||||
startRequested: false,
|
||||
};
|
||||
this.verificationSessions.set(session.id, session);
|
||||
this.ensureVerificationRequestTracked(session);
|
||||
@@ -323,6 +388,7 @@ export class MatrixVerificationManager {
|
||||
if (verifier) {
|
||||
this.attachVerifierToVerificationSession(session, verifier);
|
||||
}
|
||||
this.maybeAutoStartInboundSas(session);
|
||||
return this.buildVerificationSummary(session);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user