🔒 Aisle Security Analysis

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

1. 🟡 Unredacted recipient identifier (to) logged at warn level in outbound media fallback path

Description

The new media-fallback branch logs the raw outbound target identifier (to) at warn level.

• to is the delivery target passed through deliverOutboundPayloads* and is used as conversationId/recipient across channels; in practice it can be a phone number, email, chat/room ID, etc.
• The subsystem logger writes meta to the file logger without automatic redaction/scrubbing.
• This introduces PII leakage into logs whenever a plugin outbound adapter lacks sendMedia and a payload contains media URLs.

Vulnerable code (new):

log.warn(
  "Plugin outbound adapter does not implement sendMedia; media URLs will be dropped and text fallback will be used",
  {
    channel,
    to,
    mediaCount: payloadSummary.mediaUrls.length,
  },
);

Recommendation

Avoid logging raw recipient identifiers.

• Remove to entirely from this warning, or
• Log a stable, non-reversible identifier (hash prefix) using the existing helper redactIdentifier().

Example fix:

import { redactIdentifier } from "../../logging/redact-identifier.js";

log.warn(
  "Plugin outbound adapter does not implement sendMedia; media URLs will be dropped and text fallback will be used",
  {
    channel,
    to: redactIdentifier(to),
    mediaCount: payloadSummary.mediaUrls.length,
  },
);

Optionally also downgrade to debug or add rate limiting if this can occur frequently in production.

2. 🔵 Information disclosure via detailed plugin capability error (sendMedia missing)

Description

deliverOutboundPayloadsCore throws a very specific error when a plugin outbound adapter lacks sendMedia and the outbound payload contains only media URLs (no text fallback). This error string discloses internal implementation/capability details (that a plugin outbound adapter is being used and it lacks sendMedia).

Why this matters:

• The error is thrown from core delivery code and is likely propagated to external API clients.
• For example, the gateway send method catches errors and returns errorShape(ErrorCodes.UNAVAILABLE, String(err)), which will include this message (typically prefixed with "Error: ").
• This can enable fingerprinting of installed/configured plugins and their capabilities and may aid targeted abuse.

Vulnerable code (throws detailed internal message):

throw new Error(
  "Plugin outbound adapter does not implement sendMedia and no text fallback is available for media payload",
);

Recommendation

Avoid returning detailed internal capability/architecture messages to untrusted callers.

Recommended approach:

1. Throw a generic, user-facing error message from delivery code (or throw a typed error with an internal code), e.g.:

​// Option A: generic message
throw new Error("Media delivery is not supported for this channel");

​// Option B: typed error
class OutboundCapabilityError extends Error {
  code = "OUTBOUND_MEDIA_UNSUPPORTED" as const;
}
throw new OutboundCapabilityError("Media delivery is not supported for this channel");

2. At the API boundary (e.g., gateway send handler), map internal errors to sanitized client messages and keep detailed diagnostics only in server logs/telemetry.

3. 🔵 Incorrect message_sent messageId/content in media fallback path can reference previous payload

Description

In deliverOutboundPayloadsCore, the new !handler.supportsMedia fallback path emits a message_sent event with a messageId derived from results.at(-1) even when no new delivery result was added for this payload.

• beforeCount is recorded, but messageId is not derived from the new results (those after beforeCount).
• If sendTextChunks() completes without pushing any new result (e.g., a plugin-provided chunker returns [] in length mode, causing the loop to send nothing), then:
  • success becomes false (results.length > beforeCount)
  • but messageId will still be set to the previous payload’s last messageId (results.at(-1)), corrupting audit/hook metadata.
• Additionally, the event’s content is emitted as the untrimmed payloadSummary.text, even though the actual delivered text is fallbackText = payloadSummary.text.trim(). This can create discrepancies in auditing/hook processing.

Security impact (integrity):

• Downstream consumers of message_sent (plugins/internal hooks) may associate a failure (or content) with the wrong message ID, leading to misleading audit trails, incorrect correlation, or erroneous automated actions keyed off messageId.

Vulnerable code:

const beforeCount = results.length;
await sendTextChunks(fallbackText, sendOverrides);
const messageId = results.at(-1)?.messageId;
emitMessageSent({
  success: results.length > beforeCount,
  content: payloadSummary.text,
  messageId,
});

Recommendation

Only attach a messageId when the current payload actually produced new delivery results, and derive it from the newly appended slice. Also emit the actual delivered content.

Example fix:

const beforeCount = results.length;
await sendTextChunks(fallbackText, sendOverrides);
const newResults = results.slice(beforeCount);
const messageId = newResults.at(-1)?.messageId;

emitMessageSent({
  success: newResults.length > 0,
  content: fallbackText,
  messageId: newResults.length > 0 ? messageId : undefined,
});

Additionally, consider hardening sendTextChunks to ensure non-empty text always produces at least one send (or throws) even if a plugin chunker returns an empty array.

Analyzed PR: #32788 at commit c77bfc3

_{Last updated on: 2026-03-04T19:06:44Z}

Greptile Summary

This PR relaxes the createPluginHandler guard so plugin outbound adapters that only implement sendText (without sendMedia) are treated as valid direct-delivery handlers, and adds a sendMedia fallback that degrades to sendText for media payloads when sendMedia is absent.

The core change is small and reasonable, but there is one functional concern and one observability gap:

• Silent empty-string messages on multi-URL payloads — the delivery loop in deliverOutboundPayloadsCore iterates every mediaUrl in a payload and calls handler.sendMedia(caption, url). Only the first URL receives the real caption; subsequent ones pass caption = "". With the new fallback, each of those empty-caption calls reaches sendText({ text: "" }), meaning a text-only adapter will emit blank messages for every media URL after the first.
• No logging when mediaUrl is dropped — the fallback silently discards the mediaUrl without any warn/debug log entry, making it hard for plugin authors or operators to notice that media is not being delivered.
• The regression test only covers the single-URL case; a multi-URL test case would pin the intended semantics and guard against future regressions.

Confidence Score: 3/5

• Safe to merge for single-media-URL payloads, but carries a regression risk for multi-URL payloads where the fallback path will emit spurious empty-text messages.
• The guard relaxation and single-URL fallback are correct. The multi-URL edge case (empty-caption calls to sendText) is a genuine logic gap not covered by tests, and the lack of any log when media is silently dropped reduces observability. These issues lower confidence from an otherwise straightforward change.
• src/infra/outbound/deliver.ts — specifically the sendMedia fallback block (lines 180–192) and how it interacts with the multi-URL iteration loop in deliverOutboundPayloadsCore.

_{Last reviewed commit: 357f81a}

Addressed the review feedback in dc7240ca4:

• Avoided multi-URL empty-message regressions when sendMedia is missing.
  • deliverOutboundPayloadsCore now detects adapters without media support and performs a single text fallback instead of iterating media URLs through sendMedia fallback.
• Added observability for media drop fallback.
  • Emits a warning when media URLs are dropped because plugin outbound only implements sendText.
• Added regression coverage for multi-media fallback behavior.
  • New test asserts one text send for two media URLs and validates warning metadata.

Please recheck when convenient.