🔒 Aisle Security Analysis

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

1. 🔵 DoS via unbounded sanitization of interactive card text before output truncation

Description

The new interactive-card text extraction introduces output caps (CARD_MAX_OUTPUT_CHARS), but it sanitizes attacker-controlled strings before clipping them, so very large card fields can still cause excessive CPU and memory usage (CWE-400).

• pushText() calls sanitizeCardText(s) on the entire string s (two global regex replaces) and only then slices to the remaining output budget.
• parseInteractiveCardContent() similarly sanitizes card.title and header.subtitle.content before slicing.
• An attacker who can send a message containing a very large interactive-card JSON payload (or very large string fields within it) can force the bot/plugin to do linear scans over multi-megabyte strings repeatedly, despite the intended output limit.

Vulnerable code:

const clean = sanitizeCardText(s);
const clipped = clean.slice(0, CARD_MAX_OUTPUT_CHARS - outChars);

Recommendation

Clip before running regex-based sanitization, and/or enforce a hard maximum input size per field and per message.

Example fix (clip then sanitize, keeping a small buffer to avoid cutting escape sequences mid-way):

const MAX_FIELD_SCAN = 16_384; // or another defensible bound

const pushText = (s: string) => {
  if (!s || outChars >= CARD_MAX_OUTPUT_CHARS) return;

  ​// Bound work even if attacker provides multi-megabyte strings
  const remaining = CARD_MAX_OUTPUT_CHARS - outChars;
  const scanLen = Math.min(s.length, Math.max(remaining + 256, 0), MAX_FIELD_SCAN);
  const clean = sanitizeCardText(s.slice(0, scanLen));

  const clipped = clean.slice(0, remaining);
  if (!clipped) return;
  out.push(clipped);
  outChars += clipped.length;
};

Also consider applying a maximum length guard on rawContent before JSON.parse() to prevent parsing extremely large payloads.

2. 🔵 CRLF/log injection via incomplete control-character sanitization in Feishu interactive card text

Description

The new Feishu interactive-card text sanitization is intended to prevent terminal/log injection, but it fails to remove carriage return (\r, 0x0D).

• CONTROL_CHAR_RE removes most C0 control characters except \r because it skips \x0d (range jumps from \x0c to \x0e).
• sanitizeCardText() is applied to attacker-influenced interactive card fields (e.g., card.title, plain_text.content, markdown.content) and returned as FeishuMessageInfo.content.
• That extracted content is used downstream in logs and in the agent prompt context:
  • Logged when fetching quoted messages: fetched quoted message: ${quotedContent?.slice(0, 100)} (in extensions/feishu/src/bot.ts). A \r can overwrite/forge log lines in many log viewers/terminals.
  • Injected into the agent body: [Replying to: "${quotedContent}"] (in buildFeishuAgentBody), which can also be displayed or logged elsewhere.

Vulnerable code:

const CONTROL_CHAR_RE = /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/g;

function sanitizeCardText(s: string): string {
  return s.replace(ANSI_ESCAPE_RE, "").replace(CONTROL_CHAR_RE, "");
}

Recommendation

Include \r in the stripped control characters (and consider neutralizing \n at log callsites).

Fix CONTROL_CHAR_RE to include CR:

​// keep \n and \t if desired, but strip \r as well
const CONTROL_CHAR_RE = /[\x00-\x08\x0b\x0c\x0d\x0e-\x1f\x7f]/g;

Additionally, harden log output (recommended) so multi-line values cannot forge log entries:

const safeForLog = (s: string) => s.replace(/[\r\n]/g, "\\n");
log(`... fetched quoted message: ${safeForLog(quotedContent ?? "").slice(0, 100)}`);

This preserves card text for LLM context while preventing CR/LF log forging in operational logs.

Analyzed PR: #32024 at commit 81921e4

_{Last updated on: 2026-03-07T07:06:55Z}

Greptile Summary

Improved Feishu interactive card text extraction to handle alert/notification cards. The PR extracts a new extractCardTextElements() helper that recursively processes multiple element types (div, markdown, plain_text, lark_md, header, note, column_set) instead of just div and markdown. The implementation:

• Handles nested structures via recursion for note and column_set elements
• Extracts top-level card header titles separately (outside the elements array)
• Falls back to JSON.stringify(parsed) when no text is found, giving agents the full card structure instead of [Interactive Card] placeholder
• Includes proper type guards to prevent runtime errors

The implementation is well-structured with defensive type checking. The fallback to JSON serialization is intentional per the inline comment, though it could produce large strings for complex cards.

Recommendation: Add test coverage for the new element types (plain_text, lark_md, header, note, column_set). The existing test at extensions/feishu/src/send.test.ts:35-70 only validates markdown and div extraction.

Confidence Score: 4/5

• Safe to merge with recommendation to add test coverage
• The implementation is solid with proper type guards, defensive programming, and correct handling of nested structures. No logical errors or security issues found. Score is 4 (not 5) because the PR is marked [WIP] and lacks test coverage for the newly supported element types (plain_text, lark_md, header, note, column_set)
• No files require special attention, but consider adding tests to extensions/feishu/src/send.test.ts for the new element types

_{Last reviewed commit: fddfc87}

Addressed all automated review feedback:

Codex (9 threads): all resolved — schema 2.0 body.elements support added, column_set node budget bounded, LIFO stack for correct element order, debug log removed, card.title/header.subtitle sanitized via sanitizeCardText, inline row loop consumes seenNodes budget, download_message_file removed (out of scope).

Aisle (CWE-400, CWE-117): column_set fanout now bounded by seenNodes; index-based traversal avoids O(n²); all extracted text paths go through sanitizeCardText stripping ANSI and control characters.

Greptile: test coverage added for all new element types (11 tests passing); JSON.stringify fallback replaced with [Interactive Card] placeholder.

@aisle review

@Takhoffman Could you please review this when you have a moment? Thanks.

Btw, I don't know why aisle can't not work and re-review again, but already fix the logic it mentioned.

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Closing due to inactivity.
If you believe this PR should be revived, post in #pr-thunderdome-dangerzone on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.