fix(matrix): close owner-side device verification loop on SAS confirm by nklock · Pull Request #74542 · openclaw/openclaw

nklock · 2026-04-29T18:23:53Z

Close owner-side device verification loop on SAS confirm

What this PR does

After running openclaw matrix verify confirm-sas on a self-verification, the operator's Element X stayed in "Verifying…" because three things on the bot side did not happen before the verb returned:

confirmVerificationSas did not await the rust-crypto verifier promise. Verifier.verify() in matrix-js-sdk resolves only after both sides exchange MACs and the protocol fully settles, including the cross-signing-key uploads triggered by crossSignDevice. Returning early meant the operator's next /keys/query saw an inconsistent state and the prompt persisted.
The 30-second auto-confirm path explicitly skipped device cross-signing. When the operator initiated SAS from their phone (initiatedByMe === false), the bot's auto-confirm timer fired but called confirmSasForSession with { trustOwnDevice: false }. The bot's own device was never cross-signed on this path. The check inside trustOwnDeviceAfterConfirmedSas already gates on isSelfVerification, so flipping the flag is safe — non-self requests remain a no-op.
The standalone confirmMatrixVerificationSas action did not call trustOwnIdentityAfterSelfVerification. Only the higher-level runMatrixSelfVerification path called it (at completeMatrixSelfVerification). The standalone CLI verb skipped cross-signing the operator's master key from the bot side, so Element X had no path to clear the prompt without a passive sync tick.

After this change, verify confirm-sas on a self-verification settles all three before returning, and Element X's device-list view shows the bot device with a green shield with no remaining "Verify" prompt.

Changes

`extensions/matrix/src/matrix/sdk/verification-manager.ts`

confirmVerificationSas (line 725) — after confirmSasForSession, await session.verifyPromise if defined. verifyPromise is the .then().catch() chain set by ensureVerificationStarted, which already routes rejections into session.error, so awaiting it cannot double-throw.
maybeAutoConfirmSas (line 507) — pass { trustOwnDevice: true } to confirmSasForSession. The check inside trustOwnDeviceAfterConfirmedSas (gates on isSelfVerification) keeps non-self verifications unaffected.

`extensions/matrix/src/matrix/actions/verification.ts`

confirmMatrixVerificationSas (line 433) — after crypto.confirmVerificationSas resolves, if the returned summary has isSelfVerification: true, call client.trustOwnIdentityAfterSelfVerification?.(). Mirrors the pattern already in completeMatrixSelfVerification at line 224.

Tests

extensions/matrix/src/matrix/sdk/verification-manager.test.ts:

Flipped the existing "auto-confirmed self-verification SAS" test to assert trustOwnDeviceAfterSas IS called (was: NOT called).
New: confirmVerificationSas awaits the verifier's verify promise before resolving — uses a deferred verifyPromise to assert the verb blocks until the verifier settles.
New: confirmVerificationSas surfaces a verifier-promise rejection on session.error — asserts a rejecting verify() lands the error in summary.error rather than throwing.

extensions/matrix/src/matrix/actions/verification.test.ts:

New: confirmMatrixVerificationSas calls trustOwnIdentityAfterSelfVerification on a self-verification.
New: confirmMatrixVerificationSas does not call trustOwnIdentityAfterSelfVerification on a non-self verification.

pnpm test:extension matrix — 117 test files green; 23 tests in actions/verification.test.ts, 20 in verification-manager.test.ts.

What this PR explicitly does not do

Does not expose the QR-code verification flow as new CLI verbs. The manager methods exist (generateVerificationQr, scanVerificationQr, confirmVerificationReciprocateQr) but adding CLI surface for them is a separate change. SAS is sufficient for the failure mode addressed here.
Does not add cross-restart persistence for in-flight verification sessions. Sessions remain in-memory; if the bot restarts mid-verification the operator can re-initiate from their client and the bot will auto-accept the fresh request.
Does not change the cross-signing bootstrap path; that is the companion MSC3967 PR (filed separately).

Verified end-to-end

Confirmed against matrix.thepolycule.ca (Synapse 1.145.0+ess.1, MAS-fronted). After bootstrap and a SAS round-trip with an Element X session, the bot device shows the green shield in the operator's device list and encrypted DMs decrypt round-trip without UTD warnings.

AI-assisted disclosure

This change was authored with AI assistance (Claude). The author reviewed every line, ran pnpm test:extension matrix locally, and verified the closure end-to-end against a MAS-fronted Synapse 1.145.0+ess.1 deployment.

clawsweeper · 2026-04-29T18:29:05Z

Codex review: found issues before merge.

What this changes:

The PR updates Matrix SAS self-verification confirmation and auto-confirm handling to wait for verifier completion, cross-sign own device/identity paths, add regression tests, and document the fix in the Matrix changelog.

Maintainer follow-up before merge:

This is an open implementation PR that needs normal maintainer review and a small author-side changelog fix, not a separate automated repair branch.

Security review:

Security review cleared: The diff changes Matrix self-verification trust flow and tests only; it does not add dependencies, CI execution, permissions, secrets handling, or package-resolution changes, and the new trust calls remain gated to self-verification.

Review findings:

[P2] Add the required changelog credit — extensions/matrix/CHANGELOG.md:11

Review details

Best possible solution:

Keep the PR open, add the required changelog credit, then review and validate the Matrix self-verification behavior with the targeted Matrix extension tests and, if available, a live self-verification smoke.

Full review comments:

[P2] Add the required changelog credit — extensions/matrix/CHANGELOG.md:11
Every added changelog entry needs a credited Thanks @author attribution. This new Unreleased fix entry ends with [AI-assisted] but has no contributor credit, so it violates the repo changelog policy and should be amended before release.
Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.84

Acceptance criteria:

pnpm test:extension matrix
pnpm exec oxfmt --check --threads=1 extensions/matrix/src/matrix/sdk/verification-manager.ts extensions/matrix/src/matrix/sdk/verification-manager.test.ts extensions/matrix/src/matrix/actions/verification.ts extensions/matrix/src/matrix/actions/verification.test.ts extensions/matrix/CHANGELOG.md

What I checked:

Current main still skips own-device trust on inbound auto-confirm: maybeAutoConfirmSas still calls confirmSasForSession with { trustOwnDevice: false }, so the PR is not obsolete on current main. (extensions/matrix/src/matrix/sdk/verification-manager.ts:507, 68912111cfea)
Current main standalone confirm path lacks identity trust step: confirmMatrixVerificationSas returns crypto.confirmVerificationSas(...) directly, while the higher-level self-verification flow already calls trustOwnIdentityAfterSelfVerification when owner verification is incomplete. (extensions/matrix/src/matrix/actions/verification.ts:440, 68912111cfea)
Dependency contract supports waiting for verifier completion: The pinned matrix-js-sdk 41.4.0-rc.0 API documents Verifier.verify() as resolving when verification completes; the rust SAS verifier awaits a completion deferred that resolves when the inner verification is done, while SAS confirm() only sends queued MAC requests.
PR adds targeted regression coverage: The PR adds tests for awaiting the verifier promise, preserving rejection as session error, self-verification identity trust, and the auto-confirm self-device cross-sign path. (extensions/matrix/src/matrix/sdk/verification-manager.test.ts:541, 2be24f358a59)
Changelog entry violates repo attribution policy: The new Matrix changelog bullet has no Thanks @author attribution, while repo policy requires every added changelog entry to include one. (AGENTS.md:145, 68912111cfea)
Feature history points to Matrix E2EE maintainers: GitHub commit history for the Matrix verification files shows recent full identity trust, E2EE QA, and Matrix plugin migration work by @gumadeiras, with adjacent Matrix action/refactor and release/changelog maintenance by @steipete. (extensions/matrix/src/matrix/sdk/verification-manager.ts)

Likely related people:

gumadeiras: Recent history on the central Matrix verification/E2EE files includes full identity trust, E2EE QA stabilization, and the Matrix plugin migration branch. (role: recent Matrix E2EE and verification owner; confidence: high; commits: 72731a37d255, 99159f89da03, 94693f7ff036; files: extensions/matrix/src/matrix/sdk/verification-manager.ts, extensions/matrix/src/matrix/actions/verification.ts, extensions/matrix/CHANGELOG.md)
steipete: Recent commits touched Matrix action boundaries, Matrix test performance, and plugin changelog/release maintenance, making this a likely secondary routing candidate. (role: recent adjacent maintainer and release/changelog owner; confidence: medium; commits: 4336a7f3a9c6, 605cb60586eb, 7fcefd56b773; files: extensions/matrix/src/matrix/actions/verification.ts, extensions/matrix/src/matrix/actions/verification.test.ts, extensions/matrix/CHANGELOG.md)

Remaining risk / open question:

Tests were not run because this review was explicitly constrained to read-only inspection.
The Matrix behavior is homeserver/client timing-sensitive; maintainer review should weigh the author's reported live Element X proof against targeted CI results.
The changelog entry must be credited before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 68912111cfea.

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.

steipete · 2026-04-29T18:43:11Z

Landed in 86956f7.

I kept the contributor fix and added one maintainer-side guard before landing: confirmMatrixVerificationSas now calls trustOwnIdentityAfterSelfVerification only after a completed, error-free self-verification summary. That matches the matrix-js-sdk contract: ShowSasCallbacks.confirm() only queues MAC, while Verifier.verify() resolves when the rust verifier reaches done and rejects on cancel/timeout.

Local proof on the rebased head:

OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test extensions/matrix/src/matrix/actions/verification.test.ts extensions/matrix/src/matrix/sdk/verification-manager.test.ts -- --reporter=verbose
pnpm exec oxfmt --check --threads=1 extensions/matrix/CHANGELOG.md extensions/matrix/src/matrix/actions/verification.ts extensions/matrix/src/matrix/actions/verification.test.ts extensions/matrix/src/matrix/sdk/verification-manager.ts extensions/matrix/src/matrix/sdk/verification-manager.test.ts
git diff --check

Thanks @nklock.

@vincentkoc

Adds six missing entries for commits that landed without their own CHANGELOG.md update, picked from the last six hours of origin/main and attributed to the original contributors. Changes: - Control UI/i18n locale registry expansion + new docs glossaries (297f4c6, 0126692 by @vincentkoc). - Gateway/diagnostics opt-in startup timeline (097eed8, d001c34, e69da9d by @shakkernerd). Fixes: - Matrix `verify confirm-sas` cross-signing close (86956f7 by @nklock; #74542). - `openclaw status` channel context-window overrides (eb7d89f by @HemantSudarshan). - Sandbox Docker daemon graceful when sandbox mode is off (2dadc82 by @kaseonedge; #73671). - Control UI mobile chat settings persisted via Lit state (b1c5152 by @BunsDev). Skipped Peter-only commits with no external collaborator (per the maintainer-attribution rule against thanking @steipete) and the model list auth-index series (already covered by the existing "Models/UI: hide unauthenticated providers" entry).

…openclaw#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>

@vincentkoc

Adds six missing entries for commits that landed without their own CHANGELOG.md update, picked from the last six hours of origin/main and attributed to the original contributors. Changes: - Control UI/i18n locale registry expansion + new docs glossaries (297f4c6, 0126692 by @vincentkoc). - Gateway/diagnostics opt-in startup timeline (097eed8, d001c34, e69da9d by @shakkernerd). Fixes: - Matrix `verify confirm-sas` cross-signing close (86956f7 by @nklock; openclaw#74542). - `openclaw status` channel context-window overrides (eb7d89f by @HemantSudarshan). - Sandbox Docker daemon graceful when sandbox mode is off (2dadc82 by @kaseonedge; openclaw#73671). - Control UI mobile chat settings persisted via Lit state (b1c5152 by @BunsDev). Skipped Peter-only commits with no external collaborator (per the maintainer-attribution rule against thanking @steipete) and the model list auth-index series (already covered by the existing "Models/UI: hide unauthenticated providers" entry).

…openclaw#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>

@vincentkoc

Adds six missing entries for commits that landed without their own CHANGELOG.md update, picked from the last six hours of origin/main and attributed to the original contributors. Changes: - Control UI/i18n locale registry expansion + new docs glossaries (3562a87, ffe547a by @vincentkoc). - Gateway/diagnostics opt-in startup timeline (b6df431, e7119aa, ca63cda by @shakkernerd). Fixes: - Matrix `verify confirm-sas` cross-signing close (6dda69a by @nklock; openclaw#74542). - `openclaw status` channel context-window overrides (1b3a248 by @HemantSudarshan). - Sandbox Docker daemon graceful when sandbox mode is off (0336aa2 by @kaseonedge; openclaw#73671). - Control UI mobile chat settings persisted via Lit state (25b2db8 by @BunsDev). Skipped Peter-only commits with no external collaborator (per the maintainer-attribution rule against thanking @steipete) and the model list auth-index series (already covered by the existing "Models/UI: hide unauthenticated providers" entry).

openclaw-barnacle Bot added channel: matrix Channel integration: matrix size: S labels Apr 29, 2026

nklock and others added 2 commits April 29, 2026 19:41

fix(matrix): guard owner trust after failed SAS verification

f65e937

steipete force-pushed the fix/matrix-verify-owner-closure branch from 2be24f3 to f65e937 Compare April 29, 2026 18:42

steipete merged commit 86956f7 into openclaw:main Apr 29, 2026
12 checks passed

github-actions Bot mentioned this pull request Apr 29, 2026

📡 Upstream Digest — 2026-04-29 19:12 UTC curtismercier/openclaw-mods#719

Open

This was referenced May 1, 2026

Matrix: cross-signing bootstrap fails on MAS-fronted Synapse (MSC3967) #74504

Open

[Bug]: Matrix cannot be recovered using the recovery key. #73480

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(matrix): close owner-side device verification loop on SAS confirm#74542

fix(matrix): close owner-side device verification loop on SAS confirm#74542
steipete merged 2 commits intoopenclaw:mainfrom
nklock:fix/matrix-verify-owner-closure

nklock commented Apr 29, 2026

Uh oh!

clawsweeper Bot commented Apr 29, 2026

Uh oh!

Uh oh!

steipete commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nklock commented Apr 29, 2026

Close owner-side device verification loop on SAS confirm

What this PR does

Changes

extensions/matrix/src/matrix/sdk/verification-manager.ts

extensions/matrix/src/matrix/actions/verification.ts

Tests

What this PR explicitly does not do

Verified end-to-end

AI-assisted disclosure

Uh oh!

clawsweeper Bot commented Apr 29, 2026

Uh oh!

Uh oh!

steipete commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`extensions/matrix/src/matrix/sdk/verification-manager.ts`

`extensions/matrix/src/matrix/actions/verification.ts`