Description

In the Feishu reply dispatcher, partial snapshot updates are cleaned with stripReasoningTagsFromText(), but other streaming paths still forward raw model text into the user-visible streaming card.

Affected flows:

• Final streaming update: when a streamed reply receives info.kind === "final", the dispatcher sets streamText = text without stripping reasoning tags.
• Block-to-streaming fallback: when info.kind === "block" and streaming is active, the dispatcher calls queueStreamingUpdate(text, { mode: "delta" ... }) without stripping reasoning tags.

If an upstream runtime/model emits <thinking>...</thinking> / <think> / antml:thinking tags inside the final/block text (even accidentally), those tags (and their contents) can be rendered to end users through the streaming card update/close path.

Vulnerable code (final path):

if (info?.kind === "final") {
  streamText = text;
  snapshotBaseText = "";
  lastSnapshotTextLength = text.length;
  flushStreamingCardUpdate(buildCombinedStreamText(reasoningText, streamText));
}

Recommendation

Apply stripReasoningTagsFromText() consistently to all text that can reach streaming cards (partial snapshots, block deltas, and final texts) before it is assigned to streamText / passed into queueStreamingUpdate().

Example fix:

const cleaned = stripReasoningTagsFromText(text, { mode: "strict", trim: "both" });
if (!cleaned) return;

if (info?.kind === "block") {
  queueStreamingUpdate(cleaned, { mode: "delta", dedupeWithLastPartial: true });
}

if (info?.kind === "final") {
  streamText = cleaned;
  snapshotBaseText = "";
  lastSnapshotTextLength = cleaned.length;
  flushStreamingCardUpdate(buildCombinedStreamText(reasoningText, streamText));
}

Alternatively, enforce (and document) a strong upstream invariant that reply.text for block/final never contains reasoning tags, and add a defense-in-depth strip here anyway.

Description

The streaming status line appended to Feishu card markdown is built from payload.name / payload.phase without any escaping/sanitization.

• Input: payload.name or payload.phase provided to replyOptions.onToolStart.
• Transform: Interpolated directly into a markdown string: 🔧 **Using: ${...}...**.
• Sink: Included in the combined streaming text and sent to Feishu streaming card updates (streaming.update(...)), which updates a tag: "markdown" card element.
• Impact: If tool names/phases can be influenced by an attacker (e.g., user-configurable tool/plugin definitions, forwarded tool metadata, or untrusted integrations), they can inject Feishu card markdown/"lark_md" constructs such as <at id=all></at> (mention everyone), <at id=...></at> (mention specific users), or malicious links/formatting into the card.

Vulnerable code:

updateStreamingStatusLine(
  `🔧 **Using: ${payload.name ?? payload.phase ?? "tool"}...**`,
);

Recommendation

Treat tool/phase labels as untrusted when rendering into Feishu card markdown.

Options:

1. Escape/strip Feishu mention tags and markdown-special characters before interpolation.

function escapeLarkMdInline(input: string): string {
  return input
    .replace(/&/g, "&")
    .replace(/</g, "&lt;")
    .replace(/>/g, "&gt;")
    .replace(/[\r\n]+/g, " ")
    .trim();
}

const toolLabel = escapeLarkMdInline(payload.name ?? payload.phase ?? "tool");
updateStreamingStatusLine(`🔧 **Using: ${toolLabel}...**`);

2. Prefer a plain_text field/element for status (no markdown parsing), if supported by your card schema.

Also consider an allowlist for tool labels (e.g., /^[a-zA-Z0-9_\-.: ]{1,50}$/) to avoid unexpected rendering primitives.

Description

The Feishu streaming reply path maintains streamText across partial updates. Newly added snapshot-merging logic (snapshotBaseText/lastSnapshotTextLength) can preserve prior content and append new snapshot blocks when it detects a snapshot "reset".

• Input: payload.text from onPartialReply (model/runtime-provided partial content) is passed into queueStreamingUpdate().
• Issue: When startsNewSnapshotBlock triggers, the previous streamText is copied into snapshotBaseText and then concatenated with the new snapshot (streamText = ${snapshotBaseText}${nextText}``). Repeated resets can keep appending additional blocks, causing streamText to grow without bound.
• Impact: Large, repeated string concatenations (slice, template literal concatenations) and repeated flushStreamingCardUpdate() calls can create high CPU usage (O(n) copies) and increased memory pressure, enabling a denial-of-service when an attacker can induce many partial updates / pathological snapshot patterns (e.g., via crafted prompts causing streaming output patterns).

Vulnerable code:

const currentSnapshotText = snapshotBaseText
  ? streamText.slice(snapshotBaseText.length)
  : streamText;
...
if (startsNewSnapshotBlock) {
  snapshotBaseText = streamText;
  streamText = `${snapshotBaseText}${nextText}`;
} else {
  streamText = `${snapshotBaseText}${mergeStreamingText(currentSnapshotText, nextText)}`;
}

Recommendation

Introduce hard limits and/or trimming on streaming buffers, and avoid repeated full-string concatenations.

Suggested mitigations:

• Cap total streamed characters (e.g., 20k–100k) and truncate older content or stop streaming updates beyond the cap.
• Limit the number of snapshot blocks/resets preserved (e.g., keep only the most recent block).
• Consider building text via an array of chunks and joining only when sending updates (or use a rope/buffer strategy), to reduce O(n) copying.

Example (simple cap):

const MAX_STREAM_CHARS = 50_000;

function clampStream(text: string): string {
  if (text.length <= MAX_STREAM_CHARS) return text;
  ​// Keep tail to preserve most recent output
  return text.slice(text.length - MAX_STREAM_CHARS);
}

​// After updating streamText:
streamText = clampStream(streamText);
​// If you clamp, also adjust snapshotBaseText accordingly or reset it:
if (snapshotBaseText.length > streamText.length) snapshotBaseText = "";

Also consider dropping snapshotBaseText preservation when the combined length exceeds the cap (reset to empty) to prevent unbounded growth.

1. extensions/feishu/src/reply-dispatcher.ts, line 565-567 (link)

   onIdle/onError skip typing cleanup when closeStreaming throws

   If streaming.close() rejects, closeStreaming re-throws (the finally block doesn't suppress it). Both onIdle and onError have typingCallbacks?.onIdle?.() sequenced after the await closeStreaming() call, so the typing-indicator emoji reaction is never removed on a close failure. The streaming state itself is now correctly cleaned up by the finally block, but the reaction persists until the next turn's typing.stop runs.

   Consider wrapping the closeStreaming call in its own try/finally inside onIdle/onError so the typing callback is always reached.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/feishu/src/reply-dispatcher.ts
   Line: 565-567
   
   Comment:
   **`onIdle`/`onError` skip typing cleanup when `closeStreaming` throws**
   
   If `streaming.close()` rejects, `closeStreaming` re-throws (the `finally` block doesn't suppress it). Both `onIdle` and `onError` have `typingCallbacks?.onIdle?.()` sequenced *after* the `await closeStreaming()` call, so the typing-indicator emoji reaction is never removed on a close failure. The streaming state itself is now correctly cleaned up by the `finally` block, but the reaction persists until the next turn's `typing.stop` runs.
   
   Consider wrapping the `closeStreaming` call in its own `try`/`finally` inside `onIdle`/`onError` so the typing callback is always reached.
   
   How can I resolve this? If you propose a fix, please make it concise.