1. 🟠 Credential exfiltration / SSRF via plugin-controlled gatewayUrl in createExecApprovalChannelRuntime

Description

createExecApprovalChannelRuntime allows the caller (now exported via src/plugin-sdk/infra-runtime.ts) to supply an arbitrary adapter.gatewayUrl, which is forwarded to createOperatorApprovalsGatewayClient().

This is dangerous if third-party plugins are not fully trusted:

• Input: plugin-controlled adapter.gatewayUrl
• Auth material: createOperatorApprovalsGatewayClient resolves gateway auth (token / password) from host config/env and passes it into GatewayClient
• Sink: GatewayClient sends these credentials in its initial websocket connect payload to the supplied URL

As a result, a malicious plugin can point gatewayUrl to an attacker-controlled wss:// endpoint and capture the operator credentials (and then use them to access the real gateway), i.e. an SSRF-style outbound connection with credential leakage.

Vulnerable code:

const client = await createOperatorApprovalsGatewayClient({
  config: adapter.cfg,
  gatewayUrl: adapter.gatewayUrl,
  clientDisplayName: adapter.clientDisplayName,
  onEvent: handleGatewayEvent,
  ​// ...
});

Recommendation

Do not allow untrusted/plugin code to override the gateway URL when the client uses host credentials.

Options:

1. Remove gatewayUrl from the plugin-exposed runtime API (preferred) and always use the gateway URL from trusted host configuration.

2. Strictly validate/allowlist the URL if an override is truly needed (e.g., only allow loopback or an explicitly configured host), and reject all others.

Example (restrict to configured gateway origin):

import { buildGatewayConnectionDetails } from "../gateway/call.js";

const { url: configuredUrl } = buildGatewayConnectionDetails({ config: adapter.cfg });
if (adapter.gatewayUrl && new URL(adapter.gatewayUrl).origin !== new URL(configuredUrl).origin) {
  throw new Error("gatewayUrl override not permitted");
}

const client = await createOperatorApprovalsGatewayClient({
  config: adapter.cfg,
  gatewayUrl: configuredUrl,
  clientDisplayName: adapter.clientDisplayName,
  onEvent: handleGatewayEvent,
});

3. If overrides must remain, require explicit credentials to be passed in (do not load from env/config) so callers cannot trick the host into sending secrets to arbitrary endpoints.

2. 🟡 Unbounded pending map growth when deliverRequested rejects or hangs in exec approval channel runtime

Description

handleRequested() stores each approval request in the in-memory pending map before awaiting adapter.deliverRequested(request). If deliverRequested throws/rejects, the promise rejection bubbles up (only logged by spawn() when called from gateway events) and the pending entry is never cleared and no expiry timeout is scheduled.

This creates a denial-of-service risk:

• Inputs: approval request events from the gateway (exec.approval.requested / plugin.approval.requested)
• Failure mode: transient/permanent adapter delivery failure (network outage, downstream API error) or a hanging deliverRequested
• Impact: each failed/hung request leaves an entry in pending forever (delivering: true), causing unbounded memory growth over time; additionally, later handleResolved() will only set pendingResolution and still never schedule expiration while delivering remains true.

Vulnerable code:

pending.set(request.id, entry);
const entries = await adapter.deliverRequested(request);

Recommendation

Ensure pending entries are always cleaned up (or at least have a TTL) even when delivery fails/hangs.

A typical fix is to wrap delivery in try/catch/finally and delete the pending entry on error, optionally scheduling the expiration timer before awaiting delivery:

pending.set(request.id, entry);
const timeoutMs = Math.max(0, request.expiresAtMs - nowMs());
entry.timeoutId = setTimeout(() => spawn("error handling approval expiration", handleExpired(request.id)), timeoutMs);
entry.timeoutId.unref?.();

let entries: TPending[];
try {
  entries = await adapter.deliverRequested(request);
} catch (e) {
  clearPendingEntry(request.id);
  throw e;
}

Additionally consider:

• applying a maximum size for pending
• adding a separate "delivery timeout" so a hung deliverRequested cannot pin entries indefinitely
• ensuring handleResolved() can still finalize or expire entries even if delivery never completes

3. 🟡 Chat command injection via unsanitized approvalCommandId in /approve button commands

Description

The exec approval UI helpers build interactive button command strings by directly interpolating approvalCommandId into the /approve ... command without enforcing an allowlist.

• buildExecApprovalActionDescriptors() only does .trim() on approvalCommandId and then passes it into buildExecApprovalCommandText().
• buildExecApprovalCommandText() concatenates the value into a chat command string.
• If an attacker can influence the approval ID used for chat approvals (e.g., by supplying a custom id to exec.approval.request, or any other path that propagates untrusted IDs), they can include whitespace/newlines/control characters.
• This can alter the rendered command, add extra arguments, or potentially inject additional commands when a user clicks/copies the button value, depending on the chat platform’s command parsing behavior.

Vulnerable code:

return `/approve ${params.approvalCommandId} ${...}`;

A related helper parseExecApprovalCommandText() does enforce a safe regex ([A-Za-z0-9][A-Za-z0-9._:-]*), but the builder does not, so the UI may emit commands that the parser wouldn’t accept (and may be interpreted differently by other consumers).

Recommendation

Enforce a strict allowlist for approvalCommandId at the point of command construction (and ideally at approval ID creation), matching the same constraints as the parser.

Example:

const APPROVAL_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._:-]*$/;

function normalizeApprovalCommandId(raw: string): string | null {
  const id = raw.trim();
  if (!id || !APPROVAL_ID_RE.test(id)) return null;
  return id;
}

export function buildExecApprovalActionDescriptors(params: {
  approvalCommandId: string;
  allowedDecisions?: readonly ExecApprovalReplyDecision[];
}): ExecApprovalActionDescriptor[] {
  const approvalCommandId = normalizeApprovalCommandId(params.approvalCommandId);
  if (!approvalCommandId) return [];
  ​// ...
}

Additionally, consider rejecting/normalizing custom IDs at the gateway boundary (exec.approval.request) so pending approval IDs cannot contain whitespace/control characters at all.

4. 🟡 Unbounded pending-approval tracking allows memory/timer exhaustion via gateway events

Description

The exec/plugin approval channel runtime tracks pending approvals in an unbounded Map and schedules a setTimeout per request based on the unvalidated expiresAtMs from the incoming event payload.

If an attacker can cause many *.approval.requested events to be delivered to this runtime (e.g., by abusing a gateway publishing path, a compromised gateway, or a credential with the operator.approvals scope), they can:

• Create arbitrarily many unique request.id values, growing pending without limit (memory exhaustion)
• Force creation of large numbers of timers (timer/resource exhaustion)
• Provide malformed payloads (because evt.payload is cast) leading to unexpected behavior such as immediate timeouts (NaN -> 0) and excessive churn

Vulnerable code:

const pending = new Map<string, PendingApprovalEntry<...>>();
...
pending.set(request.id, entry);
...
const timeoutMs = Math.max(0, request.expiresAtMs - nowMs());
const timeoutId = setTimeout(() => {
  spawn("error handling approval expiration", handleExpired(request.id));
}, timeoutMs);
...
spawn("error handling approval request", handleRequested(evt.payload as TRequest));

Recommendation

Add runtime validation and resource limits before accepting/scheduling a request.

Minimum hardening steps:

1. Validate event payload shape for the relevant events (e.g., with zod, typebox/AJV, or manual checks) before calling handleRequested/handleResolved.
2. Enforce bounds:
   • Maximum number of pending approvals (drop/evict oldest, or reject new ones)
   • Maximum approval ID length
   • Clamp expiresAtMs to a reasonable horizon (e.g., now + 15m) and clamp timeout to Node’s safe maximum

Example (illustrative):

const MAX_PENDING = 1000;
const MAX_ID_LEN = 128;
const MAX_TTL_MS = 15 * 60 * 1000;

function isApprovalRequestPayload(p: unknown): p is { id: string; expiresAtMs: number } {
  return (
    typeof p === "object" && p !== null &&
    typeof (p as any).id === "string" &&
    (p as any).id.length > 0 && (p as any).id.length <= MAX_ID_LEN &&
    typeof (p as any).expiresAtMs === "number" && Number.isFinite((p as any).expiresAtMs)
  );
}

if (evt.event === "exec.approval.requested") {
  if (!isApprovalRequestPayload(evt.payload)) return;
  if (pending.size >= MAX_PENDING) { /* drop/evict */ return; }

  const expiresAtMs = Math.min(evt.payload.expiresAtMs, nowMs() + MAX_TTL_MS);
  await handleRequested({ ...evt.payload, expiresAtMs } as TRequest);
}

This prevents a single stream of events from creating unbounded memory/timer growth and avoids malformed payloads driving runtime behavior.

5. 🟡 Unvalidated gateway event payload passed to approval adapters

Description

handleGatewayEvent receives an EventFrame from the gateway client and type-asserts evt.payload to TRequest/TResolved before passing it to adapter code.

Although the gateway client validates the outer EventFrame shape (event is a string, payload is unknown), the payload itself is not schema-validated. As a result:

• A malformed or attacker-controlled gateway event can reach adapter.deliverRequested() / adapter.finalizeResolved() with unexpected types/fields.
• Downstream code uses fields like request.id, request.expiresAtMs, and resolved.decision without runtime checks, enabling logic bypass (e.g., immediate expiry via expiresAtMs being undefined/NaN) or process instability/DoS (exceptions in adapter implementations).
• If adapters render these fields into downstream message formats (chat, markdown, etc.), this becomes a broader injection surface.

Vulnerable code:

if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) {
  spawn("error handling approval request", handleRequested(evt.payload as TRequest));
  return;
}
...
if (evt.event === "exec.approval.resolved" && eventKinds.has("exec")) {
  spawn("error handling approval resolved", handleResolved(evt.payload as TResolved));
  return;
}

Recommendation

Add runtime validation for each expected payload type at this trust boundary, and drop/log invalid events.

Options:

1. Define explicit schemas for event payloads (e.g., TypeBox) and compile them with Ajv (similar to validateEventFrame).

2. If schemas already exist for approval request/resolve parameters, create corresponding schemas for the event payloads (ExecApprovalRequest, ExecApprovalResolved, PluginApprovalRequest, PluginApprovalResolved) and validate before dispatch.

Example (sketch):

import { validateExecApprovalRequestedEventPayload } from "../gateway/protocol/index.js";

const handleGatewayEvent = (evt: EventFrame): void => {
  if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) {
    if (!validateExecApprovalRequestedEventPayload(evt.payload)) {
      log.warn("dropping invalid exec.approval.requested payload");
      return;
    }
    spawn("error handling approval request", handleRequested(evt.payload));
    return;
  }
  ​// ... similarly for other event kinds
};

Also consider:

• Validating id as a non-empty string and expiresAtMs as a finite integer >= createdAtMs.
• Enforcing max lengths for user-controlled strings before handing to adapters (titles/descriptions/command text) to reduce downstream injection/DoS risk.

1. 🟠 Approval ID injection can flip /approve decision in generated interactive commands

Description

buildExecApprovalCommandText() interpolates approvalCommandId directly into a /approve slash-command string. Only .trim() is applied upstream (in buildExecApprovalActionDescriptors()), with no validation/allowlist.

If an attacker can influence the approval request ID (e.g., via the optional id parameter accepted by exec.approval.request), they can embed whitespace and decision-like tokens inside the ID. The generated button/command text then becomes ambiguous when parsed by the existing /approve text parser (parseApproveCommand() in src/auto-reply/reply/commands-approve.ts), which:

• splits on whitespace
• accepts decision aliases in either the first or second token
• ignores trailing tokens

This enables decision spoofing. Example malicious ID: "req-1 allow-always".

• The UI generates a Deny button with value: /approve req-1 allow-always deny
• parseApproveCommand() interprets the second token (allow-always) as the decision and uses id = "req-1"
• Result: clicking Deny submits allow-always instead.

Vulnerable code:

return `/approve ${params.approvalCommandId} ${params.decision === "allow-always" ? "always" : params.decision}`;

This is exploitable anywhere the system renders interactive approval actions using attacker-influenced IDs.

Recommendation

Treat approval IDs as untrusted when building human/interactive command strings.

1. Enforce a strict allowlist for approvalCommandId (and ideally enforce it at the source where approvals are created), e.g. only allow [A-Za-z0-9._:-] with no whitespace.
2. If an ID is invalid, do not render interactive commands/buttons.
3. Harden the /approve command parser to reject extra tokens and require an exact format.

Example fix (ID validation at render time):

const APPROVAL_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._:-]*$/;

export function buildExecApprovalCommandText(params: {
  approvalCommandId: string;
  decision: ExecApprovalReplyDecision;
}): string {
  const id = params.approvalCommandId.trim();
  if (!APPROVAL_ID_RE.test(id)) {
    throw new Error("Invalid approval id");
  }
  const decisionToken = params.decision === "allow-always" ? "always" : params.decision;
  return `/approve ${id} ${decisionToken}`;
}

Also consider updating parseApproveCommand() to require exactly 2 tokens after /approve (or to reject IDs containing whitespace) to eliminate ambiguity.

2. 🟠 Untrusted gatewayUrl override can exfiltrate stored device token via operator approvals runtime

Description

The exported createExecApprovalChannelRuntime() allows the caller (now re-exported from the plugin SDK) to provide an arbitrary adapter.gatewayUrl, which is forwarded into createOperatorApprovalsGatewayClient().

Because createOperatorApprovalsGatewayClient() builds a GatewayClient without requiring an allowlisted host or TLS pinning, a plugin can point gatewayUrl at an attacker-controlled wss:// endpoint.

Key security impact:

• GatewayClient will automatically load a stored device auth token from disk (loadDeviceAuthToken(...)) when no explicit token/password is provided.
• That stored device token is then sent to the remote gateway in the initial connect request (auth.token = resolvedDeviceToken).
• Therefore, an untrusted plugin can trigger credential exfiltration of the device token to an attacker-controlled server by supplying a malicious gatewayUrl.

Vulnerable flow:

• Input: adapter.gatewayUrl (plugin-controlled)
• Propagation: createOperatorApprovalsGatewayClient({ gatewayUrl: adapter.gatewayUrl })
• Sink: GatewayClient.sendConnect() sends stored device token in the connect request

Vulnerable code (entry point):

const client = await createOperatorApprovalsGatewayClient({
  config: adapter.cfg,
  gatewayUrl: adapter.gatewayUrl,
  clientDisplayName: adapter.clientDisplayName,
  onEvent: handleGatewayEvent,
  ...
});

Recommendation

Treat gatewayUrl as a privileged/host-controlled setting, not plugin-provided input.

Options (choose one or combine):

1. Remove/stop exporting createExecApprovalChannelRuntime (or at least do not expose gatewayUrl override) from the plugin SDK unless plugins are fully trusted.

2. If plugins must be able to set the URL, enforce strict validation:

• Allowlist hosts (e.g., only loopback) or require it matches config.gateway.remote.url
• Require TLS pinning (tlsFingerprint) for non-loopback URLs
• Refuse connecting when a stored device token would be used with a non-trusted endpoint

Example mitigation (deny non-loopback override unless explicitly pinned):

import { isLoopbackHost } from "../gateway/net.js";

function assertSafeGatewayUrlOverride(url: string, tlsFingerprint?: string) {
  const u = new URL(url);
  if (!isLoopbackHost(u.hostname) && !tlsFingerprint) {
    throw new Error("Refusing non-loopback gatewayUrl override without TLS pinning");
  }
}

​// before createOperatorApprovalsGatewayClient(...)
if (adapter.gatewayUrl) {
  assertSafeGatewayUrlOverride(adapter.gatewayUrl, adapter.cfg.gateway?.remote?.tlsFingerprint);
}

Also consider changing GatewayClient to never send stored device tokens to non-loopback endpoints unless they are explicitly trusted/pinned.

3. 🟡 Denial-of-service risk from unbounded pending approvals Map and per-request timers

Description

The new createExecApprovalChannelRuntime() implementation stores every approval request in an unbounded in-memory Map and creates a setTimeout() per request until expiresAtMs.

• Each exec.approval.requested / plugin.approval.requested event that passes adapter.shouldHandle() results in:
  • entries = await adapter.deliverRequested(request) retained in memory
  • pending.set(request.id, { request, entries, timeoutId })
  • a dedicated timer scheduled for request.expiresAtMs - nowMs()
• There is no cap on:
  • number of pending requests stored in pending
  • size of entries returned by deliverRequested()
  • the timer duration derived from expiresAtMs

If an attacker can trigger many approval requests (e.g., by abusing the gateway method exec.approval.request / plugin.approval.request with operator.approvals scope, or via a compromised gateway/client), this can exhaust memory and timer resources in the process running this runtime, causing degraded performance or process crash.

Recommendation

Add resource limits and defensive bounds to prevent untrusted or compromised sources from exhausting memory/timers.

Suggested mitigations (combine as appropriate):

1. Cap pending approvals (drop/expire oldest or reject new ones once limit reached):

const MAX_PENDING = 1000;
if (pending.size >= MAX_PENDING) {
  log.error(`too many pending approvals (${pending.size}), dropping ${request.id}`);
  return;
}

2. Bound timeout duration (ignore/clip unreasonable expiresAtMs values):

const MAX_TIMEOUT_MS = 30 * 60 * 1000; // 30m
const timeoutMs = Math.min(MAX_TIMEOUT_MS, Math.max(0, request.expiresAtMs - nowMs()));

3. Avoid retaining large entries arrays (store only minimal metadata, or stream/serialize entries elsewhere).

4. Rate limit / authenticate event sources: ensure only trusted, rate-limited actors can generate approval requests and that request timeouts are server-capped.

4. 🟡 Unvalidated gateway event payload cast to approval request/resolution objects

Description

createExecApprovalChannelRuntime accepts EventFrame objects from the gateway client and blindly casts evt.payload to TRequest/TResolved based only on the evt.event string.

• EventFrameSchema validates only { type, event, payload: unknown } (payload is Type.Unknown()), so payload structure is not guaranteed.
• The code then passes the unvalidated payload into adapter hooks (deliverRequested / finalizeResolved), which are likely to perform side effects (send DMs, write state, resolve approvals).
• If an attacker can influence the gateway event stream (stolen gateway credentials, compromised gateway, or misconfigured gateway access), malformed or malicious payloads can:
  • crash adapter logic or cause unexpected behavior (DoS)
  • trigger unintended side effects based on forged fields (e.g., wrong approval id/decision/targets)

Vulnerable code:

if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) {
  spawn("error handling approval request", handleRequested(evt.payload as TRequest));
  return;
}
...
if (evt.event === "exec.approval.resolved" && eventKinds.has("exec")) {
  spawn("error handling approval resolved", handleResolved(evt.payload as TResolved));
  return;
}

Recommendation

Validate evt.payload against the expected schema before invoking adapter hooks, and drop/alert on invalid payloads.

Example using existing AJV validators from src/gateway/protocol/index.ts:

import {
  validateExecApprovalRequestParams,
  validateExecApprovalResolveParams,
  validatePluginApprovalRequestParams,
  validatePluginApprovalResolveParams,
  formatValidationErrors,
} from "../gateway/protocol/index.js";

const handleGatewayEvent = (evt: EventFrame): void => {
  if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) {
    if (!validateExecApprovalRequestParams(evt.payload)) {
      log.error(`invalid exec approval request payload: ${formatValidationErrors(validateExecApprovalRequestParams.errors)}`);
      return;
    }
    spawn("error handling approval request", handleRequested(evt.payload as TRequest));
    return;
  }

  if (evt.event === "exec.approval.resolved" && eventKinds.has("exec")) {
    if (!validateExecApprovalResolveParams(evt.payload)) {
      log.error(`invalid exec approval resolved payload: ${formatValidationErrors(validateExecApprovalResolveParams.errors)}`);
      return;
    }
    spawn("error handling approval resolved", handleResolved(evt.payload as TResolved));
    return;
  }

  ​// ...same for plugin events
};

If TRequest/TResolved are not identical to the gateway protocol params types, introduce explicit runtime schemas/guards for the adapter-facing types and validate against those instead.