fix: prevent Telegram DM sandbox bypass in resolveTelegramConversationBaseSessionKey#29
Open
EronFan wants to merge 2646 commits into
Open
fix: prevent Telegram DM sandbox bypass in resolveTelegramConversationBaseSessionKey#29EronFan wants to merge 2646 commits into
EronFan wants to merge 2646 commits into
Conversation
…ve sends Reconnect drain (drainPendingDeliveries) matches fresh pending entries by design to preserve crash-replay, but the live delivery path in deliverOutboundPayloads held no in-memory claim while the send was running. A reconnect firing mid-send therefore re-drove the same queue entry and produced duplicate outbound messages (e.g. WhatsApp cron sends going out 7-12x when the 30-minute inbound-silence watchdog fired during delivery). Claim the queueId against the existing entriesInProgress set right after enqueueDelivery and release it in the finally branch around ack/fail. Drain already skips claimed ids via claimRecoveryEntry, so no drain-side change is needed. The claim is process-local on purpose: a crashed owner leaves no claim behind, so startup recovery still reclaims orphaned entries. Fixes openclaw#70386. Made-with: Cursor
If a reconnect/startup drain observes the newly enqueued queue entry and calls claimRecoveryEntry before the live delivery path reaches tryClaimActiveDelivery, tryClaimActiveDelivery returns false. Previously the live path still proceeded to deliverOutboundPayloadsCore and then ack/fail, which would race the drain's own delivery and ack/fail for the same entry id and produce duplicate outbound messages. Treat a failed claim acquisition as "another in-process owner is already handling this queue entry" and bail out with an empty result array, leaving the queue entry in place for the drain to deliver and clean up. This closes the narrow residual race called out by the Aisle security review on openclaw#70428. Made-with: Cursor
… (openclaw#49588) updateLastRoute() used mergeSessionEntry which bumps updatedAt to Date.now() on every inbound message. This prevented session idle and daily reset from ever firing, since evaluateSessionFreshness() always saw a fresh updatedAt. The fix from openclaw#32379 patched recordSessionMetaFromInbound to use mergeSessionEntryPreserveActivity, but missed updateLastRoute() in the same inbound pipeline. Changes: - Remove explicit updatedAt from updateLastRoute basePatch - Switch from mergeSessionEntry to mergeSessionEntryPreserveActivity - Add regression test verifying updatedAt is preserved - Update existing test assertion to match corrected behavior Fixes openclaw#49515
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: prevent Telegram DM sandbox bypass in resolveTelegramConversationBaseSessionKey
Problem
Telegram direct-message (DM) sessions for the default account returned
agent:main:mainfromresolveTelegramConversationBaseSessionKey(). Since this matches the agent's main session key,shouldSandboxSession()returnedfalse(sandbox mode=all), causing Telegram DM executions to bypass the sandbox entirely.Root Cause
The previous implementation only applied a peer-scoped session key (e.g.
agent:main:telegram:default:direct:<senderId>) for named account DMs (whenrouteAccountId !== defaultAccountId). Default-account DMs fell through to returnparams.route.sessionKeydirectly, which wasagent:main:main(the route's default session key for the main agent on the default account).Fix
resolveTelegramConversationBaseSessionKey()now returns a peer-scoped session key for all direct (non-group) chats that are NOT explicitly bound, ensuring the key differs from the main session key and sandboxing is active.Changes
extensions/telegram/src/conversation-route.ts: Rewrote
resolveTelegramConversationBaseSessionKey()to:route.sessionKeyimmediately for explicitly bound sessions (matchedBy === 'binding.channel') or group chats (isGroup === true)buildAgentSessionKey()withdmScopefrom config (falling back to'per-account-channel-peer'ifdmScope === 'main'or unset) to ensure sandboxing is activeextensions/telegram/src/conversation-route.base-session-key.test.ts: Updated two tests that previously expected
agent:main:mainfor default-account DMs to now expect sandboxed peer-scoped keysTest Results
pnpm test -- --run extensions/telegram/src/conversation-route.base-session-key.test.ts— 5 tests passedpnpm test -- --run src/agents/sandbox/tool-policy.test.ts— 10 tests passedCloses openclaw#70342