From f4f0b171d3bcdfd88f051e1d2f8b852ff1f0eafa Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Thu, 19 Mar 2026 10:30:12 -0400 Subject: [PATCH] Matrix: isolate credential write runtime --- extensions/matrix/src/matrix/accounts.test.ts | 2 +- extensions/matrix/src/matrix/accounts.ts | 2 +- extensions/matrix/src/matrix/client.test.ts | 28 +-- extensions/matrix/src/matrix/client/config.ts | 17 +- .../matrix/src/matrix/credentials-read.ts | 150 +++++++++++++++++ .../src/matrix/credentials-write.runtime.ts | 18 ++ extensions/matrix/src/matrix/credentials.ts | 159 ++---------------- 7 files changed, 206 insertions(+), 170 deletions(-) create mode 100644 extensions/matrix/src/matrix/credentials-read.ts create mode 100644 extensions/matrix/src/matrix/credentials-write.runtime.ts diff --git a/extensions/matrix/src/matrix/accounts.test.ts b/extensions/matrix/src/matrix/accounts.test.ts index 45db29362ce..8480ef0e94b 100644 --- a/extensions/matrix/src/matrix/accounts.test.ts +++ b/extensions/matrix/src/matrix/accounts.test.ts @@ -7,7 +7,7 @@ import { resolveMatrixAccount, } from "./accounts.js"; -vi.mock("./credentials.js", () => ({ +vi.mock("./credentials-read.js", () => ({ loadMatrixCredentials: () => null, credentialsMatchConfig: () => false, })); diff --git a/extensions/matrix/src/matrix/accounts.ts b/extensions/matrix/src/matrix/accounts.ts index d0039664ac8..13e33a259a6 100644 --- a/extensions/matrix/src/matrix/accounts.ts +++ b/extensions/matrix/src/matrix/accounts.ts @@ -10,7 +10,7 @@ import { import type { CoreConfig, MatrixConfig } from "../types.js"; import { findMatrixAccountConfig, resolveMatrixBaseConfig } from "./account-config.js"; import { resolveMatrixConfigForAccount } from "./client.js"; -import { credentialsMatchConfig, loadMatrixCredentials } from "./credentials.js"; +import { credentialsMatchConfig, loadMatrixCredentials } from "./credentials-read.js"; /** Merge account config with top-level defaults, preserving nested objects. */ function mergeAccountConfig(base: MatrixConfig, account: MatrixConfig): MatrixConfig { diff --git a/extensions/matrix/src/matrix/client.test.ts b/extensions/matrix/src/matrix/client.test.ts index fc89a4944e7..663e5715daf 100644 --- a/extensions/matrix/src/matrix/client.test.ts +++ b/extensions/matrix/src/matrix/client.test.ts @@ -9,16 +9,20 @@ import { resolveMatrixAuthContext, validateMatrixHomeserverUrl, } from "./client/config.js"; -import * as credentialsModule from "./credentials.js"; +import * as credentialsReadModule from "./credentials-read.js"; import * as sdkModule from "./sdk.js"; const saveMatrixCredentialsMock = vi.hoisted(() => vi.fn()); +const touchMatrixCredentialsMock = vi.hoisted(() => vi.fn()); -vi.mock("./credentials.js", () => ({ +vi.mock("./credentials-read.js", () => ({ loadMatrixCredentials: vi.fn(() => null), - saveMatrixCredentials: saveMatrixCredentialsMock, credentialsMatchConfig: vi.fn(() => false), - touchMatrixCredentials: vi.fn(), +})); + +vi.mock("./credentials-write.runtime.js", () => ({ + saveMatrixCredentials: saveMatrixCredentialsMock, + touchMatrixCredentials: touchMatrixCredentialsMock, })); describe("resolveMatrixConfig", () => { @@ -414,14 +418,14 @@ describe("resolveMatrixAuth", () => { }); it("uses cached matching credentials when access token is not configured", async () => { - vi.mocked(credentialsModule.loadMatrixCredentials).mockReturnValue({ + vi.mocked(credentialsReadModule.loadMatrixCredentials).mockReturnValue({ homeserver: "https://matrix.example.org", userId: "@bot:example.org", accessToken: "cached-token", deviceId: "CACHEDDEVICE", createdAt: "2026-01-01T00:00:00.000Z", }); - vi.mocked(credentialsModule.credentialsMatchConfig).mockReturnValue(true); + vi.mocked(credentialsReadModule.credentialsMatchConfig).mockReturnValue(true); const cfg = { channels: { @@ -464,13 +468,13 @@ describe("resolveMatrixAuth", () => { }); it("falls back to config deviceId when cached credentials are missing it", async () => { - vi.mocked(credentialsModule.loadMatrixCredentials).mockReturnValue({ + vi.mocked(credentialsReadModule.loadMatrixCredentials).mockReturnValue({ homeserver: "https://matrix.example.org", userId: "@bot:example.org", accessToken: "tok-123", createdAt: "2026-01-01T00:00:00.000Z", }); - vi.mocked(credentialsModule.credentialsMatchConfig).mockReturnValue(true); + vi.mocked(credentialsReadModule.credentialsMatchConfig).mockReturnValue(true); const cfg = { channels: { @@ -533,8 +537,8 @@ describe("resolveMatrixAuth", () => { }); it("uses named-account password auth instead of inheriting the base access token", async () => { - vi.mocked(credentialsModule.loadMatrixCredentials).mockReturnValue(null); - vi.mocked(credentialsModule.credentialsMatchConfig).mockReturnValue(false); + vi.mocked(credentialsReadModule.loadMatrixCredentials).mockReturnValue(null); + vi.mocked(credentialsReadModule.credentialsMatchConfig).mockReturnValue(false); const doRequestSpy = vi.spyOn(sdkModule.MatrixClient.prototype, "doRequest").mockResolvedValue({ access_token: "ops-token", user_id: "@ops:example.org", @@ -615,13 +619,13 @@ describe("resolveMatrixAuth", () => { }); it("uses config deviceId with cached credentials when token is loaded from cache", async () => { - vi.mocked(credentialsModule.loadMatrixCredentials).mockReturnValue({ + vi.mocked(credentialsReadModule.loadMatrixCredentials).mockReturnValue({ homeserver: "https://matrix.example.org", userId: "@bot:example.org", accessToken: "tok-123", createdAt: "2026-01-01T00:00:00.000Z", }); - vi.mocked(credentialsModule.credentialsMatchConfig).mockReturnValue(true); + vi.mocked(credentialsReadModule.credentialsMatchConfig).mockReturnValue(true); const cfg = { channels: { diff --git a/extensions/matrix/src/matrix/client/config.ts b/extensions/matrix/src/matrix/client/config.ts index 6d137677657..e4be059ccc5 100644 --- a/extensions/matrix/src/matrix/client/config.ts +++ b/extensions/matrix/src/matrix/client/config.ts @@ -19,6 +19,7 @@ import { listNormalizedMatrixAccountIds, } from "../account-config.js"; import { resolveMatrixConfigFieldPath } from "../config-update.js"; +import { credentialsMatchConfig, loadMatrixCredentials } from "../credentials-read.js"; import { MatrixClient } from "../sdk.js"; import { ensureMatrixSdkLoggingConfigured } from "./logging.js"; import type { MatrixAuth, MatrixResolvedConfig } from "./types.js"; @@ -338,13 +339,11 @@ export async function resolveMatrixAuth(params?: { }): Promise { const { cfg, env, accountId, resolved } = resolveMatrixAuthContext(params); const homeserver = validateMatrixHomeserverUrl(resolved.homeserver); - - const { - loadMatrixCredentials, - saveMatrixCredentials, - credentialsMatchConfig, - touchMatrixCredentials, - } = await import("../credentials.js"); + let credentialsWriter: typeof import("../credentials-write.runtime.js") | undefined; + const loadCredentialsWriter = async () => { + credentialsWriter ??= await import("../credentials-write.runtime.js"); + return credentialsWriter; + }; const cached = loadMatrixCredentials(env, accountId); const cachedCredentials = @@ -391,6 +390,7 @@ export async function resolveMatrixAuth(params?: { cachedCredentials.userId !== userId || (cachedCredentials.deviceId || undefined) !== knownDeviceId; if (shouldRefreshCachedCredentials) { + const { saveMatrixCredentials } = await loadCredentialsWriter(); await saveMatrixCredentials( { homeserver, @@ -402,6 +402,7 @@ export async function resolveMatrixAuth(params?: { accountId, ); } else if (hasMatchingCachedToken) { + const { touchMatrixCredentials } = await loadCredentialsWriter(); await touchMatrixCredentials(env, accountId); } return { @@ -418,6 +419,7 @@ export async function resolveMatrixAuth(params?: { } if (cachedCredentials) { + const { touchMatrixCredentials } = await loadCredentialsWriter(); await touchMatrixCredentials(env, accountId); return { accountId, @@ -474,6 +476,7 @@ export async function resolveMatrixAuth(params?: { encryption: resolved.encryption, }; + const { saveMatrixCredentials } = await loadCredentialsWriter(); await saveMatrixCredentials( { homeserver: auth.homeserver, diff --git a/extensions/matrix/src/matrix/credentials-read.ts b/extensions/matrix/src/matrix/credentials-read.ts new file mode 100644 index 00000000000..e297072fea4 --- /dev/null +++ b/extensions/matrix/src/matrix/credentials-read.ts @@ -0,0 +1,150 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; +import { + requiresExplicitMatrixDefaultAccount, + resolveMatrixDefaultOrOnlyAccountId, +} from "../account-selection.js"; +import { getMatrixRuntime } from "../runtime.js"; +import { + resolveMatrixCredentialsDir as resolveSharedMatrixCredentialsDir, + resolveMatrixCredentialsPath as resolveSharedMatrixCredentialsPath, +} from "../storage-paths.js"; + +export type MatrixStoredCredentials = { + homeserver: string; + userId: string; + accessToken: string; + deviceId?: string; + createdAt: string; + lastUsedAt?: string; +}; + +function resolveStateDir(env: NodeJS.ProcessEnv): string { + return getMatrixRuntime().state.resolveStateDir(env, os.homedir); +} + +function resolveLegacyMatrixCredentialsPath(env: NodeJS.ProcessEnv): string | null { + return path.join(resolveMatrixCredentialsDir(env), "credentials.json"); +} + +function shouldReadLegacyCredentialsForAccount(accountId?: string | null): boolean { + const normalizedAccountId = normalizeAccountId(accountId); + const cfg = getMatrixRuntime().config.loadConfig(); + if (!cfg.channels?.matrix || typeof cfg.channels.matrix !== "object") { + return normalizedAccountId === DEFAULT_ACCOUNT_ID; + } + if (requiresExplicitMatrixDefaultAccount(cfg)) { + return false; + } + return normalizeAccountId(resolveMatrixDefaultOrOnlyAccountId(cfg)) === normalizedAccountId; +} + +function resolveLegacyMigrationSourcePath( + env: NodeJS.ProcessEnv, + accountId?: string | null, +): string | null { + if (!shouldReadLegacyCredentialsForAccount(accountId)) { + return null; + } + const legacyPath = resolveLegacyMatrixCredentialsPath(env); + return legacyPath === resolveMatrixCredentialsPath(env, accountId) ? null : legacyPath; +} + +function parseMatrixCredentialsFile(filePath: string): MatrixStoredCredentials | null { + const raw = fs.readFileSync(filePath, "utf-8"); + const parsed = JSON.parse(raw) as Partial; + if ( + typeof parsed.homeserver !== "string" || + typeof parsed.userId !== "string" || + typeof parsed.accessToken !== "string" + ) { + return null; + } + return parsed as MatrixStoredCredentials; +} + +export function resolveMatrixCredentialsDir( + env: NodeJS.ProcessEnv = process.env, + stateDir?: string, +): string { + const resolvedStateDir = stateDir ?? resolveStateDir(env); + return resolveSharedMatrixCredentialsDir(resolvedStateDir); +} + +export function resolveMatrixCredentialsPath( + env: NodeJS.ProcessEnv = process.env, + accountId?: string | null, +): string { + const resolvedStateDir = resolveStateDir(env); + return resolveSharedMatrixCredentialsPath({ stateDir: resolvedStateDir, accountId }); +} + +export function loadMatrixCredentials( + env: NodeJS.ProcessEnv = process.env, + accountId?: string | null, +): MatrixStoredCredentials | null { + const credPath = resolveMatrixCredentialsPath(env, accountId); + try { + if (fs.existsSync(credPath)) { + return parseMatrixCredentialsFile(credPath); + } + + const legacyPath = resolveLegacyMigrationSourcePath(env, accountId); + if (!legacyPath || !fs.existsSync(legacyPath)) { + return null; + } + + const parsed = parseMatrixCredentialsFile(legacyPath); + if (!parsed) { + return null; + } + + try { + fs.mkdirSync(path.dirname(credPath), { recursive: true }); + fs.renameSync(legacyPath, credPath); + } catch { + // Keep returning the legacy credentials even if migration fails. + } + + return parsed; + } catch { + return null; + } +} + +export function clearMatrixCredentials( + env: NodeJS.ProcessEnv = process.env, + accountId?: string | null, +): void { + const paths = [ + resolveMatrixCredentialsPath(env, accountId), + resolveLegacyMigrationSourcePath(env, accountId), + ]; + for (const filePath of paths) { + if (!filePath) { + continue; + } + try { + if (fs.existsSync(filePath)) { + fs.unlinkSync(filePath); + } + } catch { + // ignore + } + } +} + +export function credentialsMatchConfig( + stored: MatrixStoredCredentials, + config: { homeserver: string; userId: string; accessToken?: string }, +): boolean { + if (!config.userId) { + if (!config.accessToken) { + return false; + } + return stored.homeserver === config.homeserver && stored.accessToken === config.accessToken; + } + return stored.homeserver === config.homeserver && stored.userId === config.userId; +} diff --git a/extensions/matrix/src/matrix/credentials-write.runtime.ts b/extensions/matrix/src/matrix/credentials-write.runtime.ts new file mode 100644 index 00000000000..5e773861e42 --- /dev/null +++ b/extensions/matrix/src/matrix/credentials-write.runtime.ts @@ -0,0 +1,18 @@ +import type { + saveMatrixCredentials as saveMatrixCredentialsType, + touchMatrixCredentials as touchMatrixCredentialsType, +} from "./credentials.js"; + +export async function saveMatrixCredentials( + ...args: Parameters +): ReturnType { + const runtime = await import("./credentials.js"); + return runtime.saveMatrixCredentials(...args); +} + +export async function touchMatrixCredentials( + ...args: Parameters +): ReturnType { + const runtime = await import("./credentials.js"); + return runtime.touchMatrixCredentials(...args); +} diff --git a/extensions/matrix/src/matrix/credentials.ts b/extensions/matrix/src/matrix/credentials.ts index eaccd0ed487..7fb71715ddf 100644 --- a/extensions/matrix/src/matrix/credentials.ts +++ b/extensions/matrix/src/matrix/credentials.ts @@ -1,119 +1,15 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "openclaw/plugin-sdk/account-id"; -import { - requiresExplicitMatrixDefaultAccount, - resolveMatrixDefaultOrOnlyAccountId, -} from "../account-selection.js"; import { writeJsonFileAtomically } from "../runtime-api.js"; -import { getMatrixRuntime } from "../runtime.js"; -import { - resolveMatrixCredentialsDir as resolveSharedMatrixCredentialsDir, - resolveMatrixCredentialsPath as resolveSharedMatrixCredentialsPath, -} from "../storage-paths.js"; +import { loadMatrixCredentials, resolveMatrixCredentialsPath } from "./credentials-read.js"; +import type { MatrixStoredCredentials } from "./credentials-read.js"; -export type MatrixStoredCredentials = { - homeserver: string; - userId: string; - accessToken: string; - deviceId?: string; - createdAt: string; - lastUsedAt?: string; -}; - -function resolveStateDir(env: NodeJS.ProcessEnv): string { - return getMatrixRuntime().state.resolveStateDir(env, os.homedir); -} - -function resolveLegacyMatrixCredentialsPath(env: NodeJS.ProcessEnv): string | null { - return path.join(resolveMatrixCredentialsDir(env), "credentials.json"); -} - -function shouldReadLegacyCredentialsForAccount(accountId?: string | null): boolean { - const normalizedAccountId = normalizeAccountId(accountId); - const cfg = getMatrixRuntime().config.loadConfig(); - if (!cfg.channels?.matrix || typeof cfg.channels.matrix !== "object") { - return normalizedAccountId === DEFAULT_ACCOUNT_ID; - } - if (requiresExplicitMatrixDefaultAccount(cfg)) { - return false; - } - return normalizeAccountId(resolveMatrixDefaultOrOnlyAccountId(cfg)) === normalizedAccountId; -} - -function resolveLegacyMigrationSourcePath( - env: NodeJS.ProcessEnv, - accountId?: string | null, -): string | null { - if (!shouldReadLegacyCredentialsForAccount(accountId)) { - return null; - } - const legacyPath = resolveLegacyMatrixCredentialsPath(env); - return legacyPath === resolveMatrixCredentialsPath(env, accountId) ? null : legacyPath; -} - -function parseMatrixCredentialsFile(filePath: string): MatrixStoredCredentials | null { - const raw = fs.readFileSync(filePath, "utf-8"); - const parsed = JSON.parse(raw) as Partial; - if ( - typeof parsed.homeserver !== "string" || - typeof parsed.userId !== "string" || - typeof parsed.accessToken !== "string" - ) { - return null; - } - return parsed as MatrixStoredCredentials; -} - -export function resolveMatrixCredentialsDir( - env: NodeJS.ProcessEnv = process.env, - stateDir?: string, -): string { - const resolvedStateDir = stateDir ?? resolveStateDir(env); - return resolveSharedMatrixCredentialsDir(resolvedStateDir); -} - -export function resolveMatrixCredentialsPath( - env: NodeJS.ProcessEnv = process.env, - accountId?: string | null, -): string { - const resolvedStateDir = resolveStateDir(env); - return resolveSharedMatrixCredentialsPath({ stateDir: resolvedStateDir, accountId }); -} - -export function loadMatrixCredentials( - env: NodeJS.ProcessEnv = process.env, - accountId?: string | null, -): MatrixStoredCredentials | null { - const credPath = resolveMatrixCredentialsPath(env, accountId); - try { - if (fs.existsSync(credPath)) { - return parseMatrixCredentialsFile(credPath); - } - - const legacyPath = resolveLegacyMigrationSourcePath(env, accountId); - if (!legacyPath || !fs.existsSync(legacyPath)) { - return null; - } - - const parsed = parseMatrixCredentialsFile(legacyPath); - if (!parsed) { - return null; - } - - try { - fs.mkdirSync(path.dirname(credPath), { recursive: true }); - fs.renameSync(legacyPath, credPath); - } catch { - // Keep returning the legacy credentials even if migration fails. - } - - return parsed; - } catch { - return null; - } -} +export { + clearMatrixCredentials, + credentialsMatchConfig, + loadMatrixCredentials, + resolveMatrixCredentialsDir, + resolveMatrixCredentialsPath, +} from "./credentials-read.js"; +export type { MatrixStoredCredentials } from "./credentials-read.js"; export async function saveMatrixCredentials( credentials: Omit, @@ -147,38 +43,3 @@ export async function touchMatrixCredentials( const credPath = resolveMatrixCredentialsPath(env, accountId); await writeJsonFileAtomically(credPath, existing); } - -export function clearMatrixCredentials( - env: NodeJS.ProcessEnv = process.env, - accountId?: string | null, -): void { - const paths = [ - resolveMatrixCredentialsPath(env, accountId), - resolveLegacyMigrationSourcePath(env, accountId), - ]; - for (const filePath of paths) { - if (!filePath) { - continue; - } - try { - if (fs.existsSync(filePath)) { - fs.unlinkSync(filePath); - } - } catch { - // ignore - } - } -} - -export function credentialsMatchConfig( - stored: MatrixStoredCredentials, - config: { homeserver: string; userId: string; accessToken?: string }, -): boolean { - if (!config.userId) { - if (!config.accessToken) { - return false; - } - return stored.homeserver === config.homeserver && stored.accessToken === config.accessToken; - } - return stored.homeserver === config.homeserver && stored.userId === config.userId; -}