🔒 Aisle Security Analysis

We found 3 potential security issue(s) in this PR:

1. 🟡 Potential DoS via unbounded answer segment accumulation and full recomposition on each partial stream update

Description

The new answer-segment buffering system can consume excessive CPU and memory during streaming, because it repeatedly recomputes the full concatenated answer text across all accumulated segments on every partial update.

Key behaviors:

• answerSegments grows by pushing a new AnswerSegmentState each time an assistant-message boundary is observed (answerBoundaryPending) or a partial arrives after a finalized segment.
• For every partial update, updateAnswerSegmentFromPartial() calls composeAnswerSegmentsText(), which reduces over all answerSegments and concatenates strings.
• With many partial updates (long streaming) and many assistant-message boundaries (tool loops/provider ordering bugs), this can become uncontrolled resource consumption:
  • CPU: repeated reduce + string concatenations over an ever-growing segment list.
  • Memory: all segment texts remain stored until dispatch completion; additionally, intermediate concatenated strings are allocated repeatedly.

Vulnerable code path:

• Input: untrusted/user-influenced model output streaming via replyOptions.onPartialReply → ingestDraftLaneSegments(payload.text).
• Hot loop sink: updateAnswerSegmentFromPartial() → composeAnswerSegmentsText() → updateDraftFromPartial().

Vulnerable code:

const composeAnswerSegmentsText = () =>
  answerSegments.reduce((acc, segment) => appendAnswerSegment(acc, segment.text), "");

const updateAnswerSegmentFromPartial = (text: string) => {
  ...
  segment.text = text;
  updateDraftFromPartial(answerLane, composeAnswerSegmentsText());
};

Notes on bounds:

• These arrays are scoped to a single dispatchTelegramMessage() invocation (so they reset per inbound user message), but within a single request there is no cap on:
  • answerSegments
  • pendingAnswerFinals
  • auxiliaryAnswerFinals

This means a single crafted request that induces extensive tool-calling / many assistant message boundaries plus high-frequency partial updates can degrade the bot process (DoS).

Recommendation

Mitigate uncontrolled CPU/memory usage by avoiding full recomposition and by bounding segment growth.

Suggested changes (pick a combination):

1. Incremental composition cache

• Maintain a cached answerPrefixText representing all finalized/prior segments.
• On partial updates, only compose answerPrefixText + currentSegmentText.

Example:

let answerPrefixText = ""; // finalized segments only

function finalizeSegmentText(segText: string) {
  answerPrefixText = appendAnswerSegment(answerPrefixText, segText);
}

function currentComposedText(currentSegText: string) {
  return appendAnswerSegment(answerPrefixText, currentSegText);
}

2. Hard caps

• Cap the number of segments (e.g., 20) and/or total buffered characters (e.g., 50k).
• If exceeded, stop streaming previews and/or drop older segments.

3. Stop composing once preview cannot be updated

• If composed length exceeds draftMaxChars (min(textLimit, 4096)), stop calling composeAnswerSegmentsText() for streaming preview updates; optionally show a truncated tail.

4. Bound final queues

• Add defensive limits for pendingAnswerFinals/auxiliaryAnswerFinals and flush/merge early when counts exceed a safe threshold.

These changes reduce the risk of CPU spikes from repeated reductions and large transient string allocations during streaming.

2. 🔵 Stale Telegram preview retained when final edit fails (treated as delivered), preventing cleanup

Description

In createLaneTextDeliverer, final preview edits can fail yet be treated as delivered ("keeping existing preview"), causing the system to:

• set lane.lastPartialText to the intended final text even though Telegram still shows the old preview content
• return success to the caller, which then sets finalizedPreviewByLane[lane] = true
• skip stream cleanup (stream.clear()), leaving the stale/partial preview message visible in the chat

Security impact (privacy/integrity):

• If streaming previews can contain transient or sensitive text that the final message would have replaced/redacted, an edit failure leaves that transient content visible indefinitely.
• The system may believe the final response was delivered when it was not, preventing fallback send and cleanup.

Vulnerable flow:

• sink: params.editPreview(...) fails
• error handling: treatEditFailureAsDelivered path returns true
• state: finalizedPreviewByLane later prevents deletion in dispatch cleanup

Vulnerable code (final edit failure treated as delivered):

if (args.treatEditFailureAsDelivered) {
  if (args.context === "final") {
    args.lane.lastPartialText = args.text;
  }
  params.log(`... edit failed; keeping existing preview ...`);
  params.markDelivered();
  return true;
}

Cleanup then skips deleting the preview message:

const shouldClear = !finalizedPreviewByLane[laneState.laneName];
...
if (cleanupState.shouldClear) {
  await stream.clear();
}

Recommendation

Do not mark a final preview as delivered/finalized when the edit operation fails.

Options (choose one):

1. Fallback to standard send on final edit failure (recommended):

   • Return false for final edit failures (except message is not modified), allowing deliverLaneText(...infoKind:"final") to send a new final message.
   • After successfully sending, clear/delete the preview stream message.

2. If you must keep the existing preview on error, do not set finalizedPreviewByLane[lane] = true, so cleanup still clears the preview message, and also send an explicit error/fallback final message.

Example adjustment (option 1):

​// in tryEditPreviewMessage
if (args.treatEditFailureAsDelivered) {
  if (isMessageNotModifiedError(err)) return true;
  if (isMissingPreviewMessageError(err)) return false;
  ​// for final: do NOT treat as delivered
  return false;
}

And ensure callers only set finalizedPreviewByLane[lane] = true when the edit actually succeeded (or was truly not modified).

3. 🔵 Stale Telegram inline buttons can be preserved across assistant-message boundaries

Description

In dispatchTelegramMessage, the final answer payload is buffered and may retain earlier reply metadata (media/buttons) when a later final only changes text.

• bufferAnswerFinal() chooses the previously buffered payload when it contains channelData.telegram.buttons/media and the new final payload does not.
• This can cause inline buttons from an earlier assistant segment (e.g., before/after a tool call) to be attached to the collapsed, later “final” message text, even if the later segment did not intend to expose those actions.
• Telegram callback_data is executed by the bot’s callback handler by converting it into a synthetic message (text: data) with forceWasMentioned: true, meaning stale buttons can continue to trigger bot commands/skills without a fresh mention/context.

While callback queries are still subject to sender authorization (inlineButtonsScope + group policy), this behavior can lead to state confusion / confused-deputy UX where users click buttons that no longer correspond to the final content, potentially triggering unintended (and possibly privileged) bot actions.

Vulnerable logic:

const bufferedPayload =
  bufferedAnswerFinal &&
  hasBufferedAnswerPayloadMetadata(bufferedAnswerFinal.payload) &&
  !hasBufferedAnswerPayloadMetadata(payload)
    ? bufferedAnswerFinal.payload
    : payload;
bufferedAnswerFinal = { payload: bufferedPayload, text: composeAnswerSegmentsText() };

Recommendation

Gate metadata preservation so inline buttons/media cannot cross assistant-message boundaries unless explicitly re-specified.

Options:

1. Reset buffered metadata on assistant message start (recommended default for safety):

onAssistantMessageStart: () => enqueueDraftLaneEvent(async () => {
  ​// ...existing...
  bufferedAnswerFinal = undefined; // or clear just telegram.buttons/media fields
  answerBoundaryPending = true;
})

2. Track a segmentId/generation with bufferedAnswerFinal and only preserve metadata when the new final is for the same segment.

3. If you want “absence means remove” semantics for the collapsed final message, explicitly pass buttons: [] when a later final omits buttons but you are collapsing messages.

Additionally, consider binding callback_data to a nonce/run-id and validating it in the callback handler to prevent stale interactions from affecting a different turn/segment.

Analyzed PR: #40883 at commit d5d708b

_{Last updated on: 2026-03-09T11:57:23Z}

Greptile Summary

This PR fixes duplicate reply bubbles in Telegram streaming by replacing the old forceNewMessage() / archived-preview mechanism with two targeted changes:

1. Single answer preview lane per turn — onAssistantMessageStart no longer forces a new stream message. Instead, a pendingAnswerFinalSlots counter tracks expected finals. Each restart with existing text increments the counter and records a text boundary via answerSegmentPrefixText; arriving finals decrement it and compose cumulative text into one buffered final that is edited into the existing preview. Surplus "auxiliary" finals (slots exhausted) are sent as standalone payloads.

2. Edit failures no longer fall back to a second bubble — treatEditFailureAsDelivered is now true for all final-context preview edits. When a Telegram editMessageText call fails (e.g. transient API error), the existing preview is kept in place and the delivery is marked complete, rather than sending a replacement text message that produced the visible duplicate.

Additionally, the ArchivedPreview type and its associated consumeArchivedAnswerPreviewForFinal function are removed entirely, along with the BufferedFinalAnswer state from reasoning-lane-coordinator.ts, substantially simplifying the code path. lane.lastPartialText is now consistently updated in all final-delivery branches (successful edit, MESSAGE_NOT_MODIFIED, treatEditFailureAsDelivered, and sendPayload fallback), preventing stale snapshots from corrupting subsequent regressive-skip decisions.

Key changes:

• bot-message-dispatch.ts: replaced forceNewMessage with pendingAnswerFinalSlots + rememberAnswerBoundary in onAssistantMessageStart
• lane-delivery-text-deliverer.ts: treatEditFailureAsDelivered = context === "final" for existing-preview finalization; lane.lastPartialText updated in all error and fallback branches
• reasoning-lane-coordinator.ts: dead BufferedFinalAnswer state removed
• Tests updated and extended with "collapses multi-message finals", "keeps finalized text when media-only follow-up", and edit failure handling scenarios

Confidence Score: 4/5

• Safe to merge; the logic is well-reasoned, simplifies complex duplicate-handling code, and the new test suite covers the critical paths including multi-restart collapse and edit failure handling.
• The two-part fix (single-lane accumulation + treat-edit-failure-as-delivered) is coherent and the test suite exercises the critical paths (multi-restart collapse, early partial ordering, media follow-up, edit failure handling). The removed archived-preview machinery was the previous source of complexity and is fully replaced. Score kept at 4 rather than 5 because the pendingAnswerFinalSlots accumulation logic is non-trivial and relies on correct fire-and-forget ordering of onAssistantMessageStart vs deliver(final) callbacks.
• The pendingAnswerFinalSlots / bufferAnswerFinal path in bot-message-dispatch.ts (lines 257–302, 560–567, 635–643) is the most complex new logic and is the area to scrutinize during any future regression investigation.

_{Last reviewed commit: 70eea02}