From 8a7eb2329ac6907f8ad233d966aa6473e18fc617 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 6 Apr 2026 00:18:30 -0400 Subject: [PATCH] fix(matrix): harden health probe storage metadata --- .../matrix/src/channel.account-paths.test.ts | 2 +- .../src/matrix/client/create-client.test.ts | 85 +++++++++++++++++++ .../matrix/src/matrix/client/create-client.ts | 18 ++-- extensions/matrix/src/matrix/probe.test.ts | 31 ++++--- extensions/matrix/src/matrix/probe.ts | 1 + 5 files changed, 119 insertions(+), 18 deletions(-) create mode 100644 extensions/matrix/src/matrix/client/create-client.test.ts diff --git a/extensions/matrix/src/channel.account-paths.test.ts b/extensions/matrix/src/channel.account-paths.test.ts index 36a78896856..6e9681f3567 100644 --- a/extensions/matrix/src/channel.account-paths.test.ts +++ b/extensions/matrix/src/channel.account-paths.test.ts @@ -67,7 +67,7 @@ describe("matrix account path propagation", () => { ); }); - it("forwards accountId to matrix probes", async () => { + it("forwards accountId and deviceId to matrix probes", async () => { await matrixPlugin.status!.probeAccount?.({ cfg: {} as never, timeoutMs: 500, diff --git a/extensions/matrix/src/matrix/client/create-client.test.ts b/extensions/matrix/src/matrix/client/create-client.test.ts new file mode 100644 index 00000000000..b26b2005d96 --- /dev/null +++ b/extensions/matrix/src/matrix/client/create-client.test.ts @@ -0,0 +1,85 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const ensureMatrixSdkLoggingConfiguredMock = vi.hoisted(() => vi.fn()); +const resolveValidatedMatrixHomeserverUrlMock = vi.hoisted(() => vi.fn()); +const maybeMigrateLegacyStorageMock = vi.hoisted(() => vi.fn(async () => undefined)); +const resolveMatrixStoragePathsMock = vi.hoisted(() => vi.fn()); +const writeStorageMetaMock = vi.hoisted(() => vi.fn()); +const MatrixClientMock = vi.hoisted(() => vi.fn()); + +vi.mock("./logging.js", () => ({ + ensureMatrixSdkLoggingConfigured: ensureMatrixSdkLoggingConfiguredMock, +})); + +vi.mock("./config.js", () => ({ + resolveValidatedMatrixHomeserverUrl: resolveValidatedMatrixHomeserverUrlMock, +})); + +vi.mock("./storage.js", () => ({ + maybeMigrateLegacyStorage: maybeMigrateLegacyStorageMock, + resolveMatrixStoragePaths: resolveMatrixStoragePathsMock, + writeStorageMeta: writeStorageMetaMock, +})); + +vi.mock("../sdk.js", () => ({ + MatrixClient: MatrixClientMock, +})); + +let createMatrixClient: typeof import("./create-client.js").createMatrixClient; + +describe("createMatrixClient", () => { + const storagePaths = { + rootDir: "/tmp/openclaw-matrix-create-client-test", + storagePath: "/tmp/openclaw-matrix-create-client-test/storage.json", + recoveryKeyPath: "/tmp/openclaw-matrix-create-client-test/recovery.key", + idbSnapshotPath: "/tmp/openclaw-matrix-create-client-test/idb.snapshot", + metaPath: "/tmp/openclaw-matrix-create-client-test/storage-meta.json", + accountKey: "default", + tokenHash: "token-hash", + }; + + beforeAll(async () => { + ({ createMatrixClient } = await import("./create-client.js")); + }); + + beforeEach(() => { + vi.clearAllMocks(); + ensureMatrixSdkLoggingConfiguredMock.mockReturnValue(undefined); + resolveValidatedMatrixHomeserverUrlMock.mockResolvedValue("https://matrix.example.org"); + resolveMatrixStoragePathsMock.mockReturnValue(storagePaths); + MatrixClientMock.mockImplementation(function MockMatrixClient() { + return { + stop: vi.fn(), + }; + }); + }); + + it("persists storage metadata by default", async () => { + await createMatrixClient({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + }); + + expect(writeStorageMetaMock).toHaveBeenCalledWith({ + storagePaths, + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accountId: undefined, + deviceId: undefined, + }); + expect(MatrixClientMock).toHaveBeenCalledTimes(1); + }); + + it("skips storage metadata writes when persistence is disabled", async () => { + await createMatrixClient({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + persistStorageMeta: false, + }); + + expect(writeStorageMetaMock).not.toHaveBeenCalled(); + expect(MatrixClientMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/extensions/matrix/src/matrix/client/create-client.ts b/extensions/matrix/src/matrix/client/create-client.ts index cc53a835c05..6a28e446f69 100644 --- a/extensions/matrix/src/matrix/client/create-client.ts +++ b/extensions/matrix/src/matrix/client/create-client.ts @@ -33,6 +33,7 @@ export async function createMatrixClient(params: { accessToken: string; password?: string; deviceId?: string; + persistStorageMeta?: boolean; encryption?: boolean; localTimeoutMs?: number; initialSyncLimit?: number; @@ -66,13 +67,16 @@ export async function createMatrixClient(params: { }); fs.mkdirSync(storagePaths.rootDir, { recursive: true }); - writeStorageMeta({ - storagePaths, - homeserver, - userId, - accountId: params.accountId, - deviceId: params.deviceId, - }); + // Health probes still need validated paths, but they must not rewrite durable identity metadata. + if (params.persistStorageMeta !== false) { + writeStorageMeta({ + storagePaths, + homeserver, + userId, + accountId: params.accountId, + deviceId: params.deviceId, + }); + } const cryptoDatabasePrefix = `openclaw-matrix-${storagePaths.accountKey}-${storagePaths.tokenHash}`; diff --git a/extensions/matrix/src/matrix/probe.test.ts b/extensions/matrix/src/matrix/probe.test.ts index e08d2f07e9f..04c62865591 100644 --- a/extensions/matrix/src/matrix/probe.test.ts +++ b/extensions/matrix/src/matrix/probe.test.ts @@ -34,6 +34,7 @@ describe("probeMatrix", () => { homeserver: "https://matrix.example.org", userId: undefined, accessToken: "tok", + persistStorageMeta: false, localTimeoutMs: 1234, }); }); @@ -50,6 +51,7 @@ describe("probeMatrix", () => { homeserver: "https://matrix.example.org", userId: "@bot:example.org", accessToken: "tok", + persistStorageMeta: false, localTimeoutMs: 500, }); }); @@ -67,6 +69,7 @@ describe("probeMatrix", () => { homeserver: "https://matrix.example.org", userId: "@bot:example.org", accessToken: "tok", + persistStorageMeta: false, localTimeoutMs: 500, accountId: "ops", }); @@ -87,6 +90,7 @@ describe("probeMatrix", () => { homeserver: "https://matrix.example.org", userId: undefined, accessToken: "tok", + persistStorageMeta: false, localTimeoutMs: 500, dispatcherPolicy: { mode: "explicit-proxy", @@ -105,11 +109,15 @@ describe("probeMatrix", () => { accountId: "ops", }); - expect(createMatrixClientMock).toHaveBeenCalledWith( - expect.objectContaining({ - deviceId: "ABCDEF", - }), - ); + expect(createMatrixClientMock).toHaveBeenCalledWith({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok", + deviceId: "ABCDEF", + persistStorageMeta: false, + localTimeoutMs: 500, + accountId: "ops", + }); }); it("omits deviceId when not provided", async () => { @@ -119,11 +127,14 @@ describe("probeMatrix", () => { timeoutMs: 500, }); - expect(createMatrixClientMock).toHaveBeenCalledWith( - expect.objectContaining({ - deviceId: undefined, - }), - ); + expect(createMatrixClientMock).toHaveBeenCalledWith({ + homeserver: "https://matrix.example.org", + userId: undefined, + accessToken: "tok", + deviceId: undefined, + persistStorageMeta: false, + localTimeoutMs: 500, + }); }); it("returns client validation errors for insecure public http homeservers", async () => { diff --git a/extensions/matrix/src/matrix/probe.ts b/extensions/matrix/src/matrix/probe.ts index 55b3968a487..91591d0a103 100644 --- a/extensions/matrix/src/matrix/probe.ts +++ b/extensions/matrix/src/matrix/probe.ts @@ -67,6 +67,7 @@ export async function probeMatrix(params: { userId: inputUserId, accessToken: params.accessToken, deviceId: params.deviceId, + persistStorageMeta: false, localTimeoutMs: params.timeoutMs, accountId: params.accountId, allowPrivateNetwork: params.allowPrivateNetwork,