From 637f26cdca7d8af3f60bbada477810c12106977c Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sat, 14 Mar 2026 03:46:05 +0000 Subject: [PATCH] Core: harden matrix migration and runner wiring --- extensions/matrix/src/matrix/client.test.ts | 53 +++++++++++++++++++ extensions/matrix/src/matrix/client/config.ts | 46 ++++------------ .../run/attempt.imports.test.ts | 19 +++++++ src/agents/pi-embedded-runner/run/attempt.ts | 1 + .../reply/session-target-resolution.test.ts | 33 ++++++++++++ .../reply/session-target-resolution.ts | 9 ++-- src/infra/matrix-env-vars.ts | 30 +++++++++++ src/infra/matrix-migration-config.test.ts | 30 +++++++++++ src/infra/matrix-migration-config.ts | 16 ++---- src/sessions/session-id.ts | 5 ++ 10 files changed, 191 insertions(+), 51 deletions(-) create mode 100644 src/agents/pi-embedded-runner/run/attempt.imports.test.ts create mode 100644 src/auto-reply/reply/session-target-resolution.test.ts create mode 100644 src/infra/matrix-env-vars.ts diff --git a/extensions/matrix/src/matrix/client.test.ts b/extensions/matrix/src/matrix/client.test.ts index b9800dc79be..f8d09629bd5 100644 --- a/extensions/matrix/src/matrix/client.test.ts +++ b/extensions/matrix/src/matrix/client.test.ts @@ -241,6 +241,27 @@ describe("resolveMatrixConfig", () => { expect(resolved.deviceId).toBeUndefined(); }); + it("does not inherit the base userId for non-default accounts", () => { + const cfg = { + channels: { + matrix: { + homeserver: "https://base.example.org", + userId: "@base:example.org", + accessToken: "base-token", + accounts: { + ops: { + homeserver: "https://ops.example.org", + accessToken: "ops-token", + }, + }, + }, + }, + } as CoreConfig; + + const resolved = resolveMatrixConfigForAccount(cfg, "ops", {} as NodeJS.ProcessEnv); + expect(resolved.userId).toBe(""); + }); + it("rejects insecure public http Matrix homeservers", () => { expect(() => validateMatrixHomeserverUrl("http://matrix.example.org")).toThrow( "Matrix homeserver must use https:// unless it targets a private or loopback host", @@ -426,6 +447,38 @@ describe("resolveMatrixAuth", () => { ); }); + it("resolves token-only non-default account userId from whoami instead of inheriting the base user", async () => { + const doRequestSpy = vi.spyOn(sdkModule.MatrixClient.prototype, "doRequest").mockResolvedValue({ + user_id: "@ops:example.org", + device_id: "OPSDEVICE", + }); + + const cfg = { + channels: { + matrix: { + userId: "@base:example.org", + homeserver: "https://matrix.example.org", + accounts: { + ops: { + homeserver: "https://matrix.example.org", + accessToken: "ops-token", + }, + }, + }, + }, + } as CoreConfig; + + const auth = await resolveMatrixAuth({ + cfg, + env: {} as NodeJS.ProcessEnv, + accountId: "ops", + }); + + expect(doRequestSpy).toHaveBeenCalledWith("GET", "/_matrix/client/v3/account/whoami"); + expect(auth.userId).toBe("@ops:example.org"); + expect(auth.deviceId).toBe("OPSDEVICE"); + }); + it("resolves missing whoami identity fields for token auth", async () => { const doRequestSpy = vi.spyOn(sdkModule.MatrixClient.prototype, "doRequest").mockResolvedValue({ user_id: "@bot:example.org", diff --git a/extensions/matrix/src/matrix/client/config.ts b/extensions/matrix/src/matrix/client/config.ts index 3230e82fd77..9d4dbc734fd 100644 --- a/extensions/matrix/src/matrix/client/config.ts +++ b/extensions/matrix/src/matrix/client/config.ts @@ -7,6 +7,7 @@ import { requiresExplicitMatrixDefaultAccount, resolveMatrixDefaultOrOnlyAccountId, } from "openclaw/plugin-sdk/matrix"; +import { getMatrixScopedEnvVarNames } from "../../../../../src/infra/matrix-env-vars.js"; import { getMatrixRuntime } from "../../runtime.js"; import type { CoreConfig } from "../../types.js"; import { @@ -91,34 +92,7 @@ function resolveGlobalMatrixEnvConfig(env: NodeJS.ProcessEnv): MatrixEnvConfig { }; } -function resolveMatrixEnvAccountToken(accountId: string): string { - return Array.from(normalizeAccountId(accountId)) - .map((char) => - /[a-z0-9]/.test(char) - ? char.toUpperCase() - : `_X${char.codePointAt(0)?.toString(16).toUpperCase() ?? "00"}_`, - ) - .join(""); -} - -export function getMatrixScopedEnvVarNames(accountId: string): { - homeserver: string; - userId: string; - accessToken: string; - password: string; - deviceId: string; - deviceName: string; -} { - const token = resolveMatrixEnvAccountToken(accountId); - return { - homeserver: `MATRIX_${token}_HOMESERVER`, - userId: `MATRIX_${token}_USER_ID`, - accessToken: `MATRIX_${token}_ACCESS_TOKEN`, - password: `MATRIX_${token}_PASSWORD`, // pragma: allowlist secret - deviceId: `MATRIX_${token}_DEVICE_ID`, - deviceName: `MATRIX_${token}_DEVICE_NAME`, - }; -} +export { getMatrixScopedEnvVarNames } from "../../../../../src/infra/matrix-env-vars.js"; export function resolveScopedMatrixEnvConfig( accountId: string, @@ -274,13 +248,13 @@ export function resolveMatrixConfigForAccount( scopedEnvValue: scopedEnv.homeserver, globalEnvValue: globalEnv.homeserver, }); - const userId = resolveMatrixStringField({ - matrix, - field: "userId", - accountValue: accountField("userId"), - scopedEnvValue: scopedEnv.userId, - globalEnvValue: globalEnv.userId, - }); + const userIdSource = + accountField("userId") || + scopedEnv.userId || + (normalizedAccountId === DEFAULT_ACCOUNT_ID + ? readMatrixBaseConfigField(matrix, "userId") || globalEnv.userId || "" + : ""); + const userId = userIdSource; const accessToken = resolveMatrixStringField({ matrix, @@ -367,7 +341,7 @@ export function resolveMatrixAuthContext(params?: { !hasScopedMatrixEnvConfig(explicitAccountId, env) ) { throw new Error( - `Matrix account "${explicitAccountId}" is not configured. Add channels.matrix.accounts.${explicitAccountId} or define scoped MATRIX_${resolveMatrixEnvAccountToken(explicitAccountId)}_* variables.`, + `Matrix account "${explicitAccountId}" is not configured. Add channels.matrix.accounts.${explicitAccountId} or define scoped ${getMatrixScopedEnvVarNames(explicitAccountId).accessToken.replace(/_ACCESS_TOKEN$/, "")}_* variables.`, ); } const resolved = resolveMatrixConfigForAccount(cfg, effectiveAccountId, env); diff --git a/src/agents/pi-embedded-runner/run/attempt.imports.test.ts b/src/agents/pi-embedded-runner/run/attempt.imports.test.ts new file mode 100644 index 00000000000..cd84bb2d098 --- /dev/null +++ b/src/agents/pi-embedded-runner/run/attempt.imports.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it, vi } from "vitest"; + +const compactionFailuresImported = vi.hoisted(() => vi.fn()); + +vi.mock("../compaction-failures.js", () => { + compactionFailuresImported(); + return {}; +}); + +describe("run attempt module wiring", () => { + it("loads the compaction failure bridge during runner init", async () => { + vi.resetModules(); + compactionFailuresImported.mockClear(); + + await import("./attempt.js"); + + expect(compactionFailuresImported).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 9a46beca5d2..c22c527a602 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -142,6 +142,7 @@ import { selectCompactionTimeoutSnapshot, shouldFlagCompactionTimeout, } from "./compaction-timeout.js"; +import "../compaction-failures.js"; import { pruneProcessedHistoryImages } from "./history-image-prune.js"; import { detectAndLoadPromptImages } from "./images.js"; import type { EmbeddedRunAttemptParams, EmbeddedRunAttemptResult } from "./types.js"; diff --git a/src/auto-reply/reply/session-target-resolution.test.ts b/src/auto-reply/reply/session-target-resolution.test.ts new file mode 100644 index 00000000000..50f3cb2446b --- /dev/null +++ b/src/auto-reply/reply/session-target-resolution.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; + +const listAcpSessionEntriesMock = vi.hoisted(() => vi.fn()); + +vi.mock("../../acp/runtime/session-meta.js", () => ({ + listAcpSessionEntries: listAcpSessionEntriesMock, +})); + +vi.mock("../../gateway/call.js", () => ({ + callGateway: vi.fn(async () => { + throw new Error("gateway unavailable"); + }), +})); + +import { resolveSessionKeyByReference } from "./session-target-resolution.js"; + +describe("resolveSessionKeyByReference", () => { + it("matches ACP fallback session references case-insensitively", async () => { + listAcpSessionEntriesMock.mockResolvedValueOnce([ + { + sessionKey: "user:alice:acp:982649c1-1234-4abc-8123-0123456789ab", + }, + ]); + + const resolved = await resolveSessionKeyByReference({ + cfg: {} as OpenClawConfig, + token: "acp:982649C1-1234-4ABC-8123-0123456789AB", + }); + + expect(resolved).toBe("user:alice:acp:982649c1-1234-4abc-8123-0123456789ab"); + }); +}); diff --git a/src/auto-reply/reply/session-target-resolution.ts b/src/auto-reply/reply/session-target-resolution.ts index 90d9ca21cbe..da0fc6ecb42 100644 --- a/src/auto-reply/reply/session-target-resolution.ts +++ b/src/auto-reply/reply/session-target-resolution.ts @@ -1,22 +1,23 @@ import { listAcpSessionEntries } from "../../acp/runtime/session-meta.js"; import type { OpenClawConfig } from "../../config/config.js"; import { callGateway } from "../../gateway/call.js"; -import { SESSION_ID_RE } from "../../sessions/session-id.js"; +import { normalizeSessionId, SESSION_ID_RE } from "../../sessions/session-id.js"; function resolveAcpSessionKeySuffixToken(token: string): string | null { const trimmed = token.trim(); if (!trimmed) { return null; } - if (SESSION_ID_RE.test(trimmed)) { - return trimmed; + const normalizedSessionId = normalizeSessionId(trimmed); + if (normalizedSessionId) { + return normalizedSessionId; } const lower = trimmed.toLowerCase(); if (!lower.startsWith("acp:")) { return null; } const suffix = trimmed.slice("acp:".length).trim(); - return SESSION_ID_RE.test(suffix) ? suffix : null; + return normalizeSessionId(suffix); } async function resolveSessionKeyViaGateway(token: string): Promise { diff --git a/src/infra/matrix-env-vars.ts b/src/infra/matrix-env-vars.ts new file mode 100644 index 00000000000..b5c437d4859 --- /dev/null +++ b/src/infra/matrix-env-vars.ts @@ -0,0 +1,30 @@ +import { normalizeAccountId } from "../routing/session-key.js"; + +export function resolveMatrixEnvAccountToken(accountId: string): string { + return Array.from(normalizeAccountId(accountId)) + .map((char) => + /[a-z0-9]/.test(char) + ? char.toUpperCase() + : `_X${char.codePointAt(0)?.toString(16).toUpperCase() ?? "00"}_`, + ) + .join(""); +} + +export function getMatrixScopedEnvVarNames(accountId: string): { + homeserver: string; + userId: string; + accessToken: string; + password: string; + deviceId: string; + deviceName: string; +} { + const token = resolveMatrixEnvAccountToken(accountId); + return { + homeserver: `MATRIX_${token}_HOMESERVER`, + userId: `MATRIX_${token}_USER_ID`, + accessToken: `MATRIX_${token}_ACCESS_TOKEN`, + password: `MATRIX_${token}_PASSWORD`, + deviceId: `MATRIX_${token}_DEVICE_ID`, + deviceName: `MATRIX_${token}_DEVICE_NAME`, + }; +} diff --git a/src/infra/matrix-migration-config.test.ts b/src/infra/matrix-migration-config.test.ts index 624dfd8beec..bedf21d9d49 100644 --- a/src/infra/matrix-migration-config.test.ts +++ b/src/infra/matrix-migration-config.test.ts @@ -136,4 +136,34 @@ describe("resolveMatrixMigrationAccountTarget", () => { expect(target).toBeNull(); }); }); + + it("uses the same scoped env token encoding as runtime account auth", async () => { + await withTempHome(async () => { + const cfg: OpenClawConfig = { + channels: { + matrix: { + accounts: { + "ops-prod": {}, + }, + }, + }, + }; + const env = { + MATRIX_OPS_X2D_PROD_HOMESERVER: "https://matrix.example.org", + MATRIX_OPS_X2D_PROD_USER_ID: "@ops-prod:example.org", + MATRIX_OPS_X2D_PROD_ACCESS_TOKEN: "tok-ops-prod", + } as NodeJS.ProcessEnv; + + const target = resolveMatrixMigrationAccountTarget({ + cfg, + env, + accountId: "ops-prod", + }); + + expect(target).not.toBeNull(); + expect(target?.homeserver).toBe("https://matrix.example.org"); + expect(target?.userId).toBe("@ops-prod:example.org"); + expect(target?.accessToken).toBe("tok-ops-prod"); + }); + }); }); diff --git a/src/infra/matrix-migration-config.ts b/src/infra/matrix-migration-config.ts index dc2ad7b1da2..31753a3ccea 100644 --- a/src/infra/matrix-migration-config.ts +++ b/src/infra/matrix-migration-config.ts @@ -10,6 +10,7 @@ import { resolveMatrixChannelConfig, resolveMatrixDefaultOrOnlyAccountId, } from "./matrix-account-selection.js"; +import { getMatrixScopedEnvVarNames } from "./matrix-env-vars.js"; import { resolveMatrixAccountStorageRoot, resolveMatrixCredentialsPath, @@ -41,13 +42,6 @@ function clean(value: unknown): string { return typeof value === "string" ? value.trim() : ""; } -function resolveMatrixEnvAccountToken(accountId: string): string { - return normalizeAccountId(accountId) - .replace(/[^a-z0-9]+/gi, "_") - .replace(/^_+|_+$/g, "") - .toUpperCase(); -} - function resolveScopedMatrixEnvConfig( accountId: string, env: NodeJS.ProcessEnv, @@ -56,11 +50,11 @@ function resolveScopedMatrixEnvConfig( userId: string; accessToken: string; } { - const token = resolveMatrixEnvAccountToken(accountId); + const keys = getMatrixScopedEnvVarNames(accountId); return { - homeserver: clean(env[`MATRIX_${token}_HOMESERVER`]), - userId: clean(env[`MATRIX_${token}_USER_ID`]), - accessToken: clean(env[`MATRIX_${token}_ACCESS_TOKEN`]), + homeserver: clean(env[keys.homeserver]), + userId: clean(env[keys.userId]), + accessToken: clean(env[keys.accessToken]), }; } diff --git a/src/sessions/session-id.ts b/src/sessions/session-id.ts index 475d017832b..c3e28c5640b 100644 --- a/src/sessions/session-id.ts +++ b/src/sessions/session-id.ts @@ -3,3 +3,8 @@ export const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[ export function looksLikeSessionId(value: string): boolean { return SESSION_ID_RE.test(value.trim()); } + +export function normalizeSessionId(value: string): string | null { + const trimmed = value.trim(); + return SESSION_ID_RE.test(trimmed) ? trimmed.toLowerCase() : null; +}