fix(matrix): multi-account identity routing, voice message support, and security hardening#33350
fix(matrix): multi-account identity routing, voice message support, and security hardening#33350efe-arv wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf9064bea4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
efe-arv
left a comment
There was a problem hiding this comment.
Good observation. You're correct that the current approach resolves boundAgentId once per account at monitor startup, which means all rooms on that account share one set of mention regexes.
Why this is acceptable for now: The multi-account architecture (channels.matrix.accounts) maps each account to a single agent identity — the 1:1 account→agent pattern is the intended usage. The boundAgentId resolution picks the first matching binding, which in practice is the only binding for that account's agent.
Where it would break: If someone binds the same Matrix account to multiple agents with different mentionPatterns across different rooms. In that edge case, non-first bindings would get the wrong regex set and messages could be incorrectly dropped as no-mention before routing runs.
Proper fix (out of scope for this PR): Move mention regex resolution into the per-event handler, after resolveAgentRoute() returns the actual agentId for that specific room+message. This would require buildMentionRegexes to accept the routed agentId and a per-agent regex cache. Filed as a follow-up concern — happy to address it in a subsequent PR if the maintainers want it.
Greptile SummaryThis PR is a well-structured multi-fix bundle addressing multi-account identity routing, voice message delivery, and several security hardening measures in the Matrix plugin. The vast majority of changes are correct and well-reasoned. Two correctness issues were identified:
The security hardening changes (SSRF guard, file permission modes, login error sanitization, autoJoin default, cache bounds) are all correctly implemented. The Confidence Score: 3/5
Last reviewed commit: cf9064b |
efe-arv
left a comment
There was a problem hiding this comment.
Both issues addressed in 5b09183:
-
tool-actions.ts— accountId propagation for edit/delete: Added{ accountId: accountId ?? undefined }to botheditMatrixMessageanddeleteMatrixMessagecalls. Good catch — these were the only two message actions missing the propagation. -
room-info.ts— spurious eviction on stale refresh: Changed the eviction guard fromroomInfoCache.size >= ROOM_INFO_CACHE_MAX_SIZEto!roomInfoCache.has(roomId) && roomInfoCache.size >= ROOM_INFO_CACHE_MAX_SIZE. When refreshing a stale entry for an existing key,Map.setoverwrites in-place without growing the map, so eviction is unnecessary.
Thanks for the thorough review.
Additional Comments (2)
The PR adds Looking at
The FIFO eviction check does not account for the case where
The net effect is the map shrinks from 500 to 499, having unnecessarily evicted a valid entry. The eviction guard should only trigger when a genuinely new key is being inserted. The condition should be: |
|
Re: codex-connector P1 (mention regexes per route) Acknowledged — known limitation. The 1:1 account→agent mapping means compiled regexes are correct today. Per-event resolution (after resolveAgentRoute) is the proper fix for multi-agent-per-account setups — tracking as follow-up. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab71a5eb55
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Re: codex-connector P1 (restore fallback mention regexes) Fixed in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3e4234fe7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Re: codex-connector P1 (allow trusted private homeservers) Fixed in |
7c83aa4 to
57f1a4d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57f1a4d1c1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c8494b298
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
419bc22 to
ce3ace8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce3ace8e7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f31f996ac4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…nd security hardening Rebased on upstream main. Squashed all prior fixup commits. Changes: - Add keyed-async-queue build entry to tsdown.config.ts - Resolve agentId for buildMentionRegexes in monitor init - Propagate accountId through complete tool action dispatch chain - Forward audioAsVoice and accountId through sendMatrixMessage wrapper - Apply SSRF-guarded fetch to directory API calls - Restrict file permissions on credentials and storage metadata - Change default autoJoin from 'always' to 'allowlist' - Sanitize login error responses - Add TTL and size bounds to room info cache
…entry - P1: reaction remove path now forwards accountId so non-default accounts can remove their own reactions - P2: keyed-async-queue explicit build entry removed — already covered by pluginSdkEntrypoints array since upstream update
Pass accountId to readMessages, listReactions, pinMessage, unpinMessage, listPins, memberInfo, and channelInfo — not just send/edit/delete/react. All action helpers accept MatrixActionClientOpts which includes accountId, but the dispatch was only forwarding it to a subset of actions.
… binding match - Merge accountConfig.autoJoin and autoJoinAllowlist into the resolved cfg so registerMatrixAutoJoin reads per-account settings, not just the top-level value - Support '*' wildcard in binding accountId filter, aligning with resolve-route.ts routing semantics
d688a0d to
f5ddeb8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5ddeb88cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
OpenClaw now uses a new Matrix plugin built on the official Closing this as potentially fixed in the new plugin. If this still reproduces after migrating, please open a new issue/PR against the new Matrix plugin and link back here. Migration/docs: |
fix(matrix): multi-account identity routing, voice message support, and security hardening
Context
Matrix is the only open-protocol, self-hostable channel in OpenClaw. For deployments operating on local infrastructure — home labs, air-gapped networks, VPN meshes, or multi-agent architectures where each agent requires a distinct identity — Matrix with a self-hosted homeserver is the canonical choice. No vendor lock-in, no cloud dependency, no recurring API costs.
OpenClaw's Matrix plugin supports multi-account configuration via
channels.matrix.accounts, enabling multiple bot identities on a single homeserver. However, during comprehensive testing of a production multi-account deployment, we identified that several code paths fail to propagate per-account context correctly. Additionally, voice message support (MSC3245) is fully implemented in the send layer but unreachable from every upstream caller. This PR resolves those routing deficiencies and introduces security hardening measures identified through a systematic audit of the Matrix plugin source.Changes
Bug Fixes
1. Add missing
keyed-async-queuebuild entry totsdown.config.tspackage.jsondeclares the subpath export./plugin-sdk/keyed-async-queuemapping todist/plugin-sdk/keyed-async-queue.js, andextensions/matrix/src/matrix/send-queue.tsimports from this path at runtime. However,tsdown.config.tscontains no corresponding entry forsrc/plugin-sdk/keyed-async-queue.ts, so the build pipeline never emits the output artifact. This produces a deterministic runtime import failure when the Matrix plugin initializes its send queue.The fix adds an entry block following the identical pattern established by the existing
account-id.tsentry. The source file and its vitest test alias both already exist upstream.Files:
tsdown.config.ts2. Resolve
agentIdforbuildMentionRegexesin Matrix monitor initializationIn
extensions/matrix/src/matrix/monitor/index.ts, the monitor startup invokesbuildMentionRegexes(cfg)without supplying anagentIdargument. Consequently, per-agentmentionPatterns— configured viaagents.list[].groupChat.mentionPatterns— are never resolved: the function cannot determine which agent's pattern set to compile into the regex cache.The Discord monitor handler correctly resolves the bound agent ID from channel bindings before constructing mention regexes. The Matrix monitor omits this resolution step entirely.
The fix resolves
boundAgentIdby performing a binding lookup against the current account's monitored rooms, then passes the result tobuildMentionRegexes(cfg, boundAgentId). This enables plain-text mention detection (e.g.,@agent-name) in shared multi-agent rooms configured withrequireMention: true.Files:
extensions/matrix/src/matrix/monitor/index.ts3. Propagate
accountIdthrough the complete tool action dispatch chainWhen an agent invokes the
messagetool to perform channel actions (react, edit, delete, send, pin, read, etc.), the execution path traversesactions.ts→handleMatrixAction()intool-actions.ts→ individual action functions. TheChannelMessageActionContextprovided by the core runtime includesaccountId, butactions.tsnever destructures this field andhandleMatrixActionnever accepts it as a parameter.The consequence: all tool-initiated actions resolve to
resolveActionClient()withundefinedaccountId, which deterministically selects the primary (default) account. In a multi-account deployment, this means every agent's tool actions — reactions, edits, deletions, message sends — execute under the default bot identity regardless of the originating agent.The fix comprises three coordinated changes:
actions.ts: DestructuresaccountIdfrom the action context and forwards it to all ninehandleMatrixActioninvocationstool-actions.ts: AcceptsaccountIdas an optional third parameter and propagates it to every downstream action functionmatrix/send.ts: ExtendsreactMatrixMessageto accept an options object containing{ client?, accountId? }alongside the existingMatrixClientparameter. Backward compatibility is preserved via runtime type detection ("doRequest" in clientOrOpts)Note: Typing indicators, read receipts, and outbound reply delivery are unaffected by this deficiency — the monitor handler passes the per-account
clientinstance directly for those operations, bypassing the action dispatch chain entirely.Files:
extensions/matrix/src/actions.ts,extensions/matrix/src/tool-actions.ts,extensions/matrix/src/matrix/send.ts4. Forward
audioAsVoiceandaccountIdthrough thesendMatrixMessagewrapperThe Matrix plugin implements full MSC3245 voice message support in its send layer:
sendMessageMatrix()inmatrix/send.tsacceptsaudioAsVoice, resolves voice compatibility viaresolveMatrixVoiceDecision(), and emits the requiredorg.matrix.msc3245.voiceandorg.matrix.msc1767.audioevent content fields. However, this capability is unreachable from any upstream caller due to three independent gaps in the call chain:actions/messages.ts— ThesendMatrixMessageconvenience wrapper explicitly enumerates which options to forward tosendMessageMatrix. The allowlist includesmediaUrl,replyToId,threadId,client, andtimeoutMs— but omits bothaudioAsVoiceandaccountId. Any caller passing these fields has them silently discarded.actions.ts— Thesendaction handler reads themediaparameter but does not check the alternative parameter names (mediaUrl,filePath,path) that agents may use depending on their tool call conventions. It also does not readasVoiceoraudioAsVoicefrom the incoming params.tool-actions.ts— ThesendMessagecase does not read or forwardaudioAsVoicefrom the action params object.The fix addresses all three layers:
actions/messages.ts: Extends the opts type to includeaudioAsVoice?: booleanandaccountId?: string, and forwards both tosendMessageMatrixactions.ts: ReadsasVoice/audioAsVoicefrom params, resolves media frommedia∥mediaUrl∥filePath∥path(matching the convention used by Telegram and Discord handlers), and passesaudioAsVoicetohandleMatrixActiontool-actions.ts: ReadsaudioAsVoicefrom the action params and forwards it tosendMatrixMessageVerified end-to-end: voice messages sent via the
messagetool now arrive with correct MSC3245 metadata across OGG/Opus and MP3 formats.Files:
extensions/matrix/src/matrix/actions/messages.ts,extensions/matrix/src/actions.ts,extensions/matrix/src/tool-actions.tsSecurity Hardening
5. Apply SSRF-guarded fetch to directory API calls
directory-live.tsissues HTTP requests to the configured homeserver URL using the unguardedfetch()API. The authentication flow inclient/config.tscorrectly employsfetchWithSsrFGuardfrom the plugin SDK, but the directory search and room listing endpoints do not.If an attacker can influence the homeserver configuration value — or if a misconfigured deployment points at an internal service URL (e.g., a cloud instance metadata endpoint) — the bot will transmit authenticated HTTP requests bearing the Matrix access token to that endpoint.
The fix replaces
fetch()withfetchWithSsrFGuardinfetchMatrixJson(), consistent with the pattern established in the login flow, specifyingauditContext: "matrix.directory"for traceability.Files:
extensions/matrix/src/directory-live.ts6. Restrict file permissions on credentials and storage metadata
credentials.tsandclient/storage.tspersist sensitive files (credentials.json,storage-meta.json) usingfs.writeFileSyncwithout specifying an explicit file mode. This inherits the process umask (typically0o022), yielding0o644permissions — world-readable on standard Linux configurations.These files contain homeserver URLs, user IDs, and access token hashes. While not full-entropy tokens, they reveal which credentials are active and can facilitate targeted credential-stuffing or session hijacking attacks when combined with other information disclosure vectors.
The fix enforces restrictive permissions:
fs.mkdirSynccalls specifymode: 0o700(owner-only directory traversal)fs.writeFileSynccalls specifymode: 0o600(owner-only read/write)saveMatrixCredentials,touchMatrixCredentialsincredentials.tswriteStorageMetaand the legacy migration path instorage.tsFiles:
extensions/matrix/src/matrix/credentials.ts,extensions/matrix/src/matrix/client/storage.ts7. Change default
autoJoinpolicy from"always"to"allowlist"In
auto-join.ts, the fallback behavior whenautoJoinis omitted from the configuration defaults to"always", which registers the SDK'sAutojoinRoomsMixin— causing the bot to unconditionally accept every room invitation.For isolated local deployments where the homeserver exists on a trusted network segment, this may be acceptable. However, for any deployment where the bot's Matrix user ID is discoverable (federated homeservers, public directories, leaked room invitations), an adversary can create a room, invite the bot, and — if
groupPolicyis also at its default value of"open"— interact directly with the agent.The fix changes the default to
"allowlist". Existing deployments that explicitly configureautoJoin: "always"are entirely unaffected — the new default only governs installations where the field is absent from the configuration.Files:
extensions/matrix/src/matrix/monitor/auto-join.ts8. Sanitize login error responses in exception messages
In
client/config.ts, when a Matrix login request returns a non-success HTTP status, the raw response body is interpolated directly into the thrownError:Depending on the homeserver implementation (Synapse, Conduit, Dendrite), this response body may contain internal diagnostic messages, partial stack traces, rate-limit metadata, or session identifiers. These error messages propagate through the exception chain and are typically persisted in application logs.
The fix replaces the raw error body with a deterministic status-based message:
"invalid credentials"for HTTP 403,"rate limited"for HTTP 429, or"HTTP {statusCode}"for all other failure modes.Files:
extensions/matrix/src/matrix/client/config.ts9. Add TTL and size bounds to the room info cache
room-info.tsemploys an unboundedMapas an in-memory cache for room name and alias lookups. Entries are inserted but never evicted. On long-running gateway processes with exposure to many rooms (multi-account deployments, rooms with frequent renames, or federated environments), this cache exhibits two pathological behaviors:The fix introduces:
ROOM_INFO_CACHE_TTL_MS = 300_000) with staleness checks on readCachedRoomInfotype wrapping entries with acachedAttimestampFiles:
extensions/matrix/src/matrix/monitor/room-info.tsSummary of Changed Files
tsdown.config.tsextensions/matrix/src/matrix/monitor/index.tsextensions/matrix/src/actions.tsextensions/matrix/src/tool-actions.tsextensions/matrix/src/matrix/send.tsextensions/matrix/src/matrix/actions/messages.tsextensions/matrix/src/directory-live.tsextensions/matrix/src/matrix/credentials.tsextensions/matrix/src/matrix/client/storage.tsextensions/matrix/src/matrix/monitor/auto-join.tsextensions/matrix/src/matrix/client/config.tsextensions/matrix/src/matrix/monitor/room-info.tsRelated Issues
messagetool ignoresaudioAsVoiceparameter #32489 — Voice messages via Matrix lack MSC3245 metadata (Fix 4)Testing
All fixes were verified on a live multi-account deployment with a self-hosted homeserver, multiple bot identities, and shared rooms using mention-based routing.
build-artifacts,install-smoke). Closed in favor of this combined PR.requireMention: true— plain-text@agent-namementions correctly routed to the addressed agent only. Verified with autonomous multi-agent conversation chain.sender=@default-bot; after fix,sender=@correct-agent. Verified for reactions, sends, and edits.message(action=send, media=file.ogg, asVoice=true)→ Matrix event containsorg.matrix.msc3245.voiceandorg.matrix.msc1767.audiowith correct duration. Triple-tested across OGG/Opus and MP3 formats — 4/4 success after fix, 0/3 before fix.fetchWithSsrFGuard.storage-meta.jsonwritten at0o600; parent directories at0o700.autoJoinis absent from config. Explicit"always"configs continue to function.