Description

drainPendingDeliveries is re-exported from the public plugin SDK (src/plugin-sdk/infra-runtime.ts), giving plugin code a generic primitive that:

• Iterates over all pending deliveries (loadPendingDeliveries) and lets the caller decide which ones to drain via a caller-supplied selectEntry predicate.
• Can optionally bypass retry backoff (bypassBackoff) and immediately re-deliver entries.
• Uses a caller-supplied drainKey to gate concurrency; a malicious plugin can choose colliding keys to block other drains.

If third-party plugins are not fully trusted (or are intended to be capability-scoped), this expands the attack surface:

• Broken isolation / cross-channel impact: a plugin can match entries outside its own channel/account and force delivery attempts.
• Message flooding / cost amplification: bypassing backoff allows rapid repeated delivery attempts.
• DoS / operational interference: drainKey collisions can prevent legitimate drains from running.

Vulnerable exports/usage:

​// Public SDK re-export
export { drainPendingDeliveries };

const matchingEntries = (await loadPendingDeliveries(opts.stateDir))
  .map((entry) => ({ entry, decision: opts.selectEntry(entry, now) }))
  .filter((item) => item.decision.match);

if (!decision.bypassBackoff) {
  const retryEligibility = isEntryEligibleForRecoveryRetry(currentEntry, Date.now());
  ...
}

Recommendation

Do not expose a generic queue-draining primitive to plugins unless plugins are fully trusted. If plugins are intended to be capability-scoped, add guardrails:

• Remove drainPendingDeliveries from the public plugin SDK and keep it internal, or expose a narrow, channel-scoped API that enforces channel/account constraints server-side.
• If an SDK export is required, enforce that the caller can only drain entries for its own channel/account (e.g., pass an enforced channel/accountId scope rather than a free-form predicate).
• Remove/disable bypassBackoff for plugin callers, or gate it behind an internal-only token/capability.
• Namespace and validate drainKey (e.g., host-generated key), and consider per-plugin rate limiting.

Example of a safer, scoped API surface:

export async function drainChannelAccountDeliveries(opts: {
  channel: "whatsapp";
  accountId: string;
  ...
}) {
  return drainPendingDeliveries({
    drainKey: `plugin:${opts.channel}:${opts.accountId}`,
    logLabel: `${opts.channel} drain`,
    ...,
    selectEntry: (entry) => ({
      match: entry.channel === opts.channel && entry.accountId === opts.accountId,
      ​// never allow bypass from plugins
      bypassBackoff: false,
    }),
  });
}

Description

drainPendingDeliveries() computes decision = selectEntry(entry, now) from an initial snapshot of queued entries, then re-reads the entry from disk as currentEntry after claiming it.

However, the function continues to use the stale decision.bypassBackoff when deciding whether to enforce retry backoff for currentEntry:

• decision.bypassBackoff is derived from snapshot fields (commonly entry.lastError) before the claim.
• Between snapshot creation and loadPendingDelivery(...), another recovery path can update retryCount, lastAttemptAt, or lastError.
• If the snapshot decision indicates bypassBackoff: true, the code skips isEntryEligibleForRecoveryRetry(currentEntry, ...) entirely, potentially causing immediate retries even though currentEntry is within its backoff window.

This can create unintended aggressive retry loops during reconnect/startup, leading to self-induced denial of service (CPU/network churn) and possible upstream rate-limit bans.

Recommendation

Recompute the drain decision after reloading currentEntry (and using a fresh now) and base the backoff-bypass decision on the same entry that will actually be delivered.

For example:

const currentEntry = await loadPendingDelivery(entry.id, opts.stateDir);
if (!currentEntry) return;

const now2 = Date.now();
const decision2 = opts.selectEntry(currentEntry, now2);
if (!decision2.match) return; // entry no longer matches criteria

if (!decision2.bypassBackoff) {
  const retryEligibility = isEntryEligibleForRecoveryRetry(currentEntry, now2);
  if (!retryEligibility.eligible) return;
}

Additionally, consider making bypassBackoff depend on immutable/claimed state (or verifying currentEntry.lastError still matches the bypass condition) to avoid bypass based on stale error strings.

Description

The delivery queue storage builds file paths by directly concatenating the caller-provided id into a filename and joining it to the queue directory:

• resolveQueueEntryPaths(id) uses path.join(queueDir, ${id}.json) and ${id}.delivered with no validation
• path.join will normalize .. segments, so an id like ../../otherdir/secret escapes queueDir
• loadPendingDelivery then performs filesystem operations on the resulting path (stat, readFile, and potentially writeFile during migration), enabling arbitrary file read and, in some flows, file overwrite/clobber if an attacker can call these APIs with a crafted id

Vulnerable code:

function resolveQueueEntryPaths(id: string, stateDir?: string) {
  const queueDir = resolveQueueDir(stateDir);
  return {
    jsonPath: path.join(queueDir, `${id}.json`),
    deliveredPath: path.join(queueDir, `${id}.delivered`),
  };
}

export async function loadPendingDelivery(id: string, stateDir?: string) {
  const { jsonPath } = resolveQueueEntryPaths(id, stateDir);
  const stat = await fs.promises.stat(jsonPath);
  ​// ... readQueueEntry(jsonPath) ...
  ​// ... writeQueueEntry(jsonPath, entry) on migration ...
}

While current internal callers appear to use IDs generated by generateSecureUuid(), the functions are exported (via src/infra/outbound/delivery-queue.ts), so any untrusted caller/plugin that can reach these exports could supply a malicious id.

Recommendation

Constrain id to a safe filename and enforce that resolved paths stay within queueDir.

1. Validate id against an allowlist (e.g., UUID v4):

const SAFE_ID_RE = /^[a-f0-9-]{36}$/i;
if (!SAFE_ID_RE.test(id)) throw new Error("Invalid queue entry id");

2. Additionally, enforce containment using path.resolve:

const queueDir = resolveQueueDir(stateDir);
const jsonPath = path.resolve(queueDir, `${id}.json`);
if (!jsonPath.startsWith(path.resolve(queueDir) + path.sep)) {
  throw new Error("Path traversal");
}

3. If you want to harden against symlink tricks inside the queue directory, prefer lstat and refuse symlinks, or open files with O_NOFOLLOW (where available) before reading/writing.

Apply the same validation/containment in all functions that accept an id (ackDelivery, failDelivery, moveToFailed, loadPendingDelivery).

Description

drainPendingDeliveries() interpolates opts.drainKey (and opts.logLabel) directly into log message strings.

• drainKey is caller-provided. In the WhatsApp reconnect path it is derived from accountId with only .trim() normalization.
• If an attacker can influence accountId (e.g., via configuration, provisioning API, or any external input path that ultimately sets the account id), they may include control characters such as \r/\n to forge additional log lines, obscure true log origin, or inject misleading content.
• This is particularly relevant when logs are used for incident response/auditing.

Vulnerable code:

opts.log.info(`${opts.logLabel}: already in progress for ${opts.drainKey}, skipping`);
...
opts.log.info(`${opts.logLabel}: ${matchingEntries.length} pending message(s) matched ${opts.drainKey}`);

Recommendation

Neutralize untrusted values before placing them into log message text, or log them as structured fields so the logger/runtime can escape them.

Option A: sanitize to a safe printable subset (strip control chars):

function sanitizeForLog(value: string): string {
  return value.replace(/[\r\n\t\0\f\v]/g, " ");
}

const drainKeySafe = sanitizeForLog(opts.drainKey);
const labelSafe = sanitizeForLog(opts.logLabel);
opts.log.info(`${labelSafe}: already in progress for ${drainKeySafe}, skipping`);

Option B: structured logging (preferred when supported):

(opts.log as any).info?.({ drainKey: opts.drainKey }, `${opts.logLabel}: already in progress, skipping`);

Also consider constraining accountId/drainKey to an allowlist character set (e.g., /^[a-z0-9._-]+$/i).