🔒 Aisle Security Analysis

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

1. 🟡 Denial of Service via unbounded string splitting in Slack directive parsing

Description

parseSlackDirectives builds interactive Slack blocks from directive bodies. The directive body is parsed using String.split() without a limit, which can allocate extremely large arrays and consume CPU/memory for adversarially large strings.

In particular:

• parseChoices() uses raw.split(",") and only applies .slice(0, maxItems) after splitting, so a body containing a very large number of commas (e.g., "a:a," repeated many times) will create a huge intermediate array.
• buildSelectBlock() uses raw.split("|") without limiting the number of parts, which can similarly allocate very large arrays if many | characters are present.
• parseSlackDirectives() iterates over all directive matches and accumulates generatedBlocks and visibleTextParts before enforcing the Slack block cap (50). If a reply contains many directives, the code can do substantial work and allocate large arrays even though it later returns the original payload when the cap is exceeded.

Vulnerable code:

function parseChoices(raw: string, maxItems: number): SlackChoice[] {
  return raw
    .split(",")
    .map((entry) => parseChoice(entry))
    .filter((entry): entry is SlackChoice => Boolean(entry))
    .slice(0, maxItems);
}

While replies are typically agent-generated, user prompt-injection or upstream integrations can induce very large outputs; when enableSlackInteractiveReplies is enabled, this becomes a practical resource-exhaustion vector (CPU and memory) during reply normalization.

Recommendation

Mitigate uncontrolled resource consumption by applying hard limits and avoiding unbounded split() intermediates.

Suggested changes:

1. Cap input sizes before parsing (defense-in-depth):

const MAX_SLACK_DIRECTIVE_TEXT = 20_000;
if (text.length > MAX_SLACK_DIRECTIVE_TEXT) return payload;

2. Limit split results at the source (avoid huge arrays):

function parseChoices(raw: string, maxItems: number): SlackChoice[] {
  return raw
    .split(",", maxItems) // limit output size
    .map(parseChoice)
    .filter((entry): entry is SlackChoice => Boolean(entry));
}

function buildSelectBlock(raw: string, index: number): SlackBlock | null {
  ​// only need at most 2 parts
  const [firstRaw, secondRaw] = raw.split("|", 2);
  const first = firstRaw?.trim();
  const second = secondRaw?.trim();
  ...
}

3. Early-abort once you exceed Slack block limits / directive count (avoid work that will be discarded):

if (existingBlocks.length + generatedBlocks.length >= SLACK_MAX_BLOCKS) break; // or return payload

Optionally, enforce a max directives count (e.g., 10) and a max directive body length to keep processing bounded.

2. 🔵 Potential DoS via unbounded JSON.parse of Slack blocks in routeReply empty-guard bypass

Description

routeReply() now attempts to detect Slack blocks before applying the empty-reply guard. For Slack payloads it calls parseSlackBlocksInput() on normalized.channelData.slack.blocks.

• parseSlackBlocksInput() accepts raw JSON strings and parses them with JSON.parse().
• There is no input-size limit prior to parsing.
• While the parsed array length is capped at 50 blocks, that validation happens after JSON.parse(), so a very large JSON string can still cause significant CPU/memory usage.
• This is newly reachable even when the reply is otherwise empty (text whitespace and no media), because block detection happens before the empty-reply early return.

Vulnerable flow:

• Input: ReplyPayload.channelData.slack.blocks (can be a string)
• Parsing sink: JSON.parse(raw) inside parseSlackBlocksInput()
• Trigger: routeReply() block detection to bypass empty reply guard

Vulnerable code (new call site):

hasSlackBlocks = Boolean(
  parseSlackBlocksInput((normalized.channelData.slack as { blocks?: unknown }).blocks)?.length,
);

Parsing sink:

return JSON.parse(raw);

Recommendation

Add strict size limits (and optionally type restrictions) before attempting to parse Slack blocks JSON strings.

Suggested hardening in parseSlackBlocksInput():

const SLACK_BLOCKS_JSON_MAX_CHARS = 100_000; // pick an appropriate limit

function parseBlocksJson(raw: string) {
  if (raw.length > SLACK_BLOCKS_JSON_MAX_CHARS) {
    throw new Error("blocks JSON too large");
  }
  try {
    return JSON.parse(raw);
  } catch {
    throw new Error("blocks must be valid JSON");
  }
}

export function parseSlackBlocksInput(raw: unknown) {
  if (raw == null) return undefined;
  if (typeof raw === "string") {
    ​// optional: require it to start with '[' to avoid parsing huge non-array JSON
    if (!raw.trimStart().startsWith("[")) throw new Error("blocks must be an array");
    return validateSlackBlocksArray(parseBlocksJson(raw));
  }
  return validateSlackBlocksArray(raw);
}

Additionally, consider avoiding JSON-string inputs entirely on internal code paths (require blocks to be an array at boundaries) so you never parse attacker-controlled JSON at runtime.

Analyzed PR: #44607 at commit aa6574d

_{Last updated on: 2026-03-13T21:29:18Z}

Greptile Summary

This PR adds opt-in Slack interactive reply directives ([[slack_buttons: ...]] and [[slack_select: ...]]) that agents can use to emit Slack Block Kit action blocks, gated behind channels.slack.capabilities.interactiveReplies (disabled by default). It mirrors the existing Telegram capabilities pattern, adds schema/type support, wires the capability flag through the reply normalization pipeline, and updates agent prompt hints to reflect the current config.

Key changes:

• New src/auto-reply/reply/slack-directives.ts — parses directives from reply text and compiles them into Slack actions blocks (buttons and static select menus).
• New src/slack/interactive-replies.ts — resolves whether the capability is enabled for a given account/config.
• normalize-reply.ts / reply-dispatcher.ts / route-reply.ts / reply-prefix.ts — thread the enableSlackInteractiveReplies flag through the reply normalization pipeline.
• Schema and type updates (types.slack.ts, zod-schema.providers-core.ts) add SlackCapabilitiesConfig as a string-array or object union, backward-compatible with the legacy string[] format.
• Agent prompt hints in extensions/slack/src/channel.ts conditionally surface the directive syntax only when the capability is enabled.

Issues found:

• Multi-account fallback in isSlackInteractiveRepliesEnabled: When accountId is null/undefined and multiple Slack accounts are configured, the function returns true if any account has interactiveReplies: true. This could inadvertently enable interactive reply compilation for a conversation routed through an account that does not have the capability set.
• Text visibility with pre-existing blocks: When channelData.slack.blocks is already populated, parseSlackDirectives omits a section block for the cleaned text, leaving it only as a Slack notification/accessibility fallback (not visible in the channel). This is likely intentional but is undocumented in code comments, making it easy to misread as a bug.

Confidence Score: 3/5

• Safe to merge with low risk if only single Slack accounts are deployed; higher risk in multi-account configurations due to the .some() fallback logic.
• The feature is well-tested, disabled by default, and backward-compatible. The primary logic concern is the multi-account .some() fallback in isSlackInteractiveRepliesEnabled — in multi-account setups without a resolved accountId, enabling the capability on any one account can bleed into others. All primary call sites pass accountId when available, mitigating this in the common path, but the fallback remains a correctness risk worth addressing before this ships to production multi-account environments.
• src/slack/interactive-replies.ts — review the .some() fallback semantics for multi-account configurations.

This is a comment left during a code review.
Path: src/slack/interactive-replies.ts
Line: 31-34

Comment:
**`.some()` fallback may over-enable interactive replies across accounts**

When `accountId` is `null`/`undefined` and multiple Slack accounts are configured, the function returns `true` if **any** account has `interactiveReplies: true`. This means that if Account A has the capability enabled and Account B does not, a message routed through Account B (with an unresolved `accountId`) would still have Slack directives compiled into blocks — even though Account B's operator explicitly left the capability off.

The primary call sites (`route-reply.ts`, `reply-prefix.ts`, `channel.ts`) all pass `accountId` when it is available, so the fallback should be rare in practice. However, if the `accountId` cannot be resolved (e.g., during certain initialization paths), the overly-broad `.some()` semantics would silently enable the feature for the wrong account.

A safer fallback would be to return `false` (opt-out) when `accountId` is unknown and there are multiple accounts, since `interactiveReplies` is documented as disabled-by-default:

```typescript
// When accountId is unknown and multiple accounts exist, we cannot determine
// which account's config applies — default to disabled.
return accountIds.some((accountId) => {
  const account = resolveSlackAccount({ cfg: params.cfg, accountId });
  return resolveInteractiveRepliesFromCapabilities(account.config.capabilities);
});
```

Consider replacing `.some()` with a check that requires all accounts to have the capability (`.every()`), or simply returning `false` in the multi-account-without-accountId case.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/auto-reply/reply/slack-directives.ts
Line: 167-175

Comment:
**Cleaned text is silently dropped from visual rendering when pre-existing blocks are present**

When `existingBlocks.length > 0`, the code intentionally skips creating a section block for `cleanedText` — that text is only returned as `result.text`. In Slack, when a message has `blocks`, the top-level `text` field is used **only** as a notification/accessibility fallback and is **not** displayed in the channel. This means that if an agent writes:

```
"Act now [[slack_buttons: Retry:retry]]"
```

and `channelData.slack.blocks` is already populated (e.g., from a prior Block Kit step), the string `"Act now"` will be invisible to the user in the Slack channel — it will appear only in push notification previews and screen readers.

The current test explicitly covers this path (see `reply-flow.test.ts` "appends Slack interactive blocks to existing slack blocks") and accepts `result.text === "Act now"` without a corresponding section block. This confirms the behavior is intentional, but it's worth documenting explicitly in code or a comment, since the silent drop is non-obvious to callers:

```typescript
// When there are pre-existing blocks, we do NOT inject a section block for
// cleanedText — the caller is responsible for ensuring the text content is
// already represented in existingBlocks. cleanedText is still returned as
// result.text for Slack's notification/accessibility fallback.
if (existingBlocks.length === 0) {
```

Without this comment, a future maintainer may introduce a regression by adding a section block in both branches, duplicating the text. Alternatively, if the lost-text scenario is actually undesirable, consider always prepending a section block when `cleanedText` is non-empty, even when blocks already exist.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: df353a9}