🔒 Aisle Security Analysis

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

1. 🔵 Telegram API error-level logging suppressed on substring match, reducing monitoring visibility

Description

The new sendTelegramWithThreadFallback logic suppresses the error-level log for the first Telegram send failure when it intends to retry without message_thread_id.

This can weaken monitoring/alerting because:

• withTelegramApiErrorLogging logs failures via runtime.error / subsystem .error (high-signal) and then rethrows.
• For DM thread sends, the first failure is not logged at error level when the error message matches /message thread not found/i.
• The suppression predicate is partially based on formatErrorMessage(err) for non-GrammyError values. formatErrorMessage will include err.message (or a provided string), which can be influenced by upstream/transport layers and (in some architectures) potentially by attacker-controlled content if any thrown error message incorporates user input. This increases the risk of accidentally suppressing logs for unrelated failures whose formatted message contains the trigger substring.

Vulnerable/suppression logic:

function isTelegramThreadNotFoundError(err: unknown): boolean {
  if (err instanceof GrammyError) {
    return THREAD_NOT_FOUND_RE.test(err.description);
  }
  return THREAD_NOT_FOUND_RE.test(formatErrorMessage(err));
}

const shouldSuppressFirstErrorLog = (err: unknown) =>
  allowThreadlessRetry && hasThreadId && isTelegramThreadNotFoundError(err);
const mergedShouldLog = params.shouldLog
  ? (err: unknown) => params.shouldLog!(err) && !shouldSuppressFirstErrorLog(err)
  : (err: unknown) => !shouldSuppressFirstErrorLog(err);

Impact:

• Repeated triggering of “message thread not found” (legitimate or induced) will not show up in error-level logs if the retry succeeds, potentially hiding operational issues or abuse patterns from alerting tuned to error logs.
• Because the match is substring-based and can fall back to formatErrorMessage, suppression is less strictly tied to Telegram’s actual API error semantics (e.g., error_code=400).

Recommendation

Tighten suppression to only apply to verified Telegram API thread-not-found errors, and still emit a high-signal event when suppression occurs.

Suggested changes:

1. Only treat this as thread-not-found when the error is a Telegram API error with a known structure (prefer GrammyError fields):

function isTelegramThreadNotFoundError(err: unknown): boolean {
  return (
    err instanceof GrammyError &&
    err.error_code === 400 &&
    ​/\bmessage thread not found\b/i.test(err.description)
  );
}

(Optionally also handle HttpError from grammY by inspecting its structured payload, if available, rather than string matching.)

2. When suppressing the first error log, emit a warning (or a structured metric/event) instead of runtime.log, so it is visible to monitoring without causing a full error alert flood:

params.runtime.error?.(
  `telegram ${params.operation}: thread not found; will retry without message_thread_id`
);

Or add a dedicated counter/telemetry hook.

3. Consider narrowing the regex to match Telegram’s canonical message (as done elsewhere in the repo) to reduce accidental matches.

Analyzed PR: #30892 at commit 22b2eab

_{Last updated on: 2026-03-01T19:03:51Z}

Greptile Summary

Implements DM-specific thread fallback for Telegram sends that fail with message thread not found. The retry logic is correctly scoped to DM threads only (thread.scope === "dm"), preventing misrouting in forum topics. Error logging suppression for successful retries prevents false alarms.

• New sendTelegramWithThreadFallback() wrapper handles retry logic consistently across all send operations (text, media, voice, etc.)
• Retry only happens when all conditions are met: DM scope + thread ID present + thread-not-found error
• Forum threads remain fail-closed (no threadless retry), maintaining delivery safety
• Comprehensive test coverage for DM retry, forum fail-closed behavior, and media sends

Confidence Score: 4/5

• This PR is safe to merge with minimal risk
• Score reflects well-structured implementation with proper scope controls, comprehensive test coverage, and no security concerns. Point deducted for: (1) reliance on regex-based error message parsing which could break if Telegram changes error formats, and (2) lack of live production verification as noted by author
• No files require special attention - all changes are straightforward and well-tested

_{Last reviewed commit: 22b2eab}

Landed via temp rebase onto main.

• Gate: pnpm vitest run src/telegram/bot/delivery.test.ts
• Land commit: 3d77ab3
• Merge commit: e6e3a7b

Thanks @liuxiaopai-ai!