mirror of
https://fastgit.cc/github.com/openclaw/openclaw
synced 2026-05-01 06:36:23 +08:00
fix(matrix): close owner-side device verification loop on SAS confirm (#74542)
* fix(matrix): close owner-side device verification loop on SAS confirm
After SAS confirm via the `openclaw matrix verify confirm-sas` CLI, the
operator's Element X stayed in "Verifying…" because three things on the
bot side did not happen before the verb returned:
1. confirmVerificationSas didn't await the rust-crypto verifier promise.
`Verifier.verify()` resolves only after both sides exchange MACs and
the protocol fully settles, including cross-signing-key uploads
triggered by `crossSignDevice`. Returning early meant Element X's
next /keys/query saw an inconsistent state and the prompt persisted.
2. The 30s auto-confirm path (used when the operator initiates from
their phone) explicitly passed `{ trustOwnDevice: false }`, so the
bot never cross-signed its own device on this path. The check inside
trustOwnDeviceAfterConfirmedSas already gates on isSelfVerification,
so flipping the flag is safe — non-self requests remain a no-op.
3. The standalone `confirmMatrixVerificationSas` action did not call
`trustOwnIdentityAfterSelfVerification` (only the higher-level
`runMatrixSelfVerification` path did). Without that call, the bot
had not signed the operator's master key, so Element X had no path
to clear the prompt without a passive sync tick.
Three additive edits:
- verification-manager.ts (confirmVerificationSas): await
session.verifyPromise after confirmSasForSession returns.
verifyPromise is the .then().catch() chain set by
ensureVerificationStarted, which already routes rejections into
session.error, so awaiting it cannot double-throw.
- verification-manager.ts (maybeAutoConfirmSas): pass
{ trustOwnDevice: true } so the auto-confirm path also cross-signs
the bot device for self-verifications.
- actions/verification.ts (confirmMatrixVerificationSas): mirror the
trustOwnIdentityAfterSelfVerification call from
completeMatrixSelfVerification when the returned summary indicates
isSelfVerification.
Tests:
- verification-manager.test.ts: flipped the existing "auto-confirmed
self-verification" assertion (now expects trustOwnDeviceAfterSas to
be called); added two new tests for verifyPromise await and
rejection-on-summary.error.
- actions/verification.test.ts: two new tests asserting
confirmMatrixVerificationSas calls trustOwnIdentityAfterSelfVerification
on self-verifications and not on remote verifications.
Verified end-to-end against matrix.thepolycule.ca (Synapse 1.145.0+ess.1,
MAS-fronted): after `verify confirm-sas`, Element X's device-list view
shows the bot device with a green shield and no pending Verify prompt.
* fix(matrix): guard owner trust after failed SAS verification
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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<void>((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 () => {});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user