🔒 Aisle Security Analysis

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

1. 🟠 Brute-forceable exec approval resolution via unrestricted prefix matching (and ID disclosure on ambiguity)

Description

The exec.approval.resolve endpoint now accepts case-insensitive ID prefixes of any length via ExecApprovalManager.lookupPendingId(). This substantially reduces the effective entropy of approval IDs and enables resolving approvals without knowing the full ID.

Impact:

• Brute-force / wrong-target approval: If only a small number of approvals are pending (common), many 1–2 character prefixes will uniquely identify an approval. An attacker with access to the operator.approvals surface can iterate prefixes until one uniquely matches and then allow/deny an approval they never received.
• Information disclosure: When a prefix is ambiguous, the error message returns up to 3 full matching approval IDs (matches: ...), which further accelerates guessing and leaks identifiers.

Why this is exploitable:

• Input id is only validated as NonEmptyString (no UUID format, no max length, no minimum prefix length).
• lookupPendingId() performs prefix matching with no minimum prefix size.
• exec.approval.resolve is not covered by any rate limiter like consumeControlPlaneWriteBudget() (only applied to config/update methods), so repeated guesses are not throttled at the gateway layer.

Concrete scenario:

• A backend integration (or chat approval client) has operator.approvals scope but is not supposed to approve requests it never saw (e.g., filtered delivery).
• With one pending approval whose UUID starts with a..., sending exec.approval.resolve { id: "a", decision: "deny" } will resolve it (unique prefix).
• If multiple approvals start with the same prefix, the attacker can intentionally trigger the ambiguous error to obtain candidate IDs, then resolve exactly.

Recommendation

Mitigate prefix-guessing and misbinding by tightening resolution semantics:

1. Enforce a minimum prefix length (e.g., 8+ chars) and/or require UUID formatting.
2. Do not echo full candidate IDs in ambiguous-prefix errors (return only a count, or masked IDs).
3. Add rate limiting for exec.approval.resolve attempts (per device/IP), especially for failed/unknown/ambiguous lookups.
4. Add a maxLength constraint for params.id to avoid oversized allocations.

Example (minimum prefix length + avoid leaking IDs):

​// exec.approval.resolve
const raw = p.id.trim();
if (raw.length < 8) {
  respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "approval id prefix too short"));
  return;
}
const resolved = manager.lookupPendingId(raw);
if (resolved.kind === "ambiguous") {
  respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "ambiguous approval id prefix; use full id"));
  return;
}

And in the schema:

export const ExecApprovalResolveParamsSchema = Type.Object({
  id: Type.String({ minLength: 8, maxLength: 128 }),
  decision: NonEmptyString,
}, { additionalProperties: false });

2. 🟡 Audit/notification evasion via user-controllable suppressNotifyOnExit in node system.run

Description

The new suppressNotifyOnExit flag allows a node.invoke caller to suppress gateway-side exec.* system event notifications, reducing operator awareness and enabling audit/notification evasion.

Data flow (all in this diff):

• Input: suppressNotifyOnExit can be provided by the node.invoke caller inside params for command: "system.run".
  • The gateway explicitly allowlists and forwards it in sanitizeSystemRunParamsForForwarding() via pickSystemRunParams().
• Propagation: node-host reads opts.params.suppressNotifyOnExit === true and includes it in the emitted exec.denied / exec.finished node events.
• Sink: gateway handleNodeEvent() drops all exec notifications (exec.started, exec.denied, exec.finished) when obj.suppressNotifyOnExit === true, preventing enqueueSystemEvent() and heartbeat wake.

Security impact:

• A compromised tool/plugin/agent (or any actor with access to node.invoke / system.run) can set this flag to hide command lifecycle notifications from the session’s system event stream.
• Because system events are ephemeral (in-memory) and not persisted elsewhere, this can materially reduce traceability/awareness in deployments that rely on these events for operational auditing.
• The suppression is not bound to exec-approval flows or privileged scopes; it is honored purely based on the payload flag.

Vulnerable code:

​// src/gateway/server-node-events.ts
const notifyOnExit = cfg.tools?.exec?.notifyOnExit !== false;
if (!notifyOnExit) {
  return;
}
if (obj.suppressNotifyOnExit === true) {
  return;
}

Recommendation

Do not allow untrusted callers to suppress exec notifications.

Options (pick one depending on intended feature semantics):

1. Strip/ignore suppressNotifyOnExit unless the request is trusted (recommended):

   • Remove suppressNotifyOnExit from the forwarded allowlist in pickSystemRunParams().
   • Or only forward it when the gateway can verify the request is part of a trusted internal exec-approval flow (e.g., internal client identity / elevated scope such as operator.admin).

2. Always log/audit regardless, only suppress user-facing delivery:

   • Keep enqueueSystemEvent() (or write to a persistent audit log) even if you suppress outbound/user notifications.

Example hardening (strip for normal callers):

​// src/gateway/node-invoke-system-run-approval.ts
function pickSystemRunParams(raw: Record<string, unknown>): Record<string, unknown> {
  const next: Record<string, unknown> = {};
  for (const key of [
    "command","rawCommand","systemRunPlan","cwd","env","timeoutMs",
    "needsScreenRecording","agentId","sessionKey","runId",
    ​// "suppressNotifyOnExit"  // do not forward from user input
  ]) {
    if (key in raw) next[key] = raw[key];
  }
  return next;
}

​// If needed, inject suppressNotifyOnExit only for vetted internal paths.

3. 🟡 Telegram exec approval prompts can be misdelivered to attacker-chosen chat via untrusted turnSourceTo

Description

The new TelegramExecApprovalHandler trusts routing metadata inside ExecApprovalRequest to decide where to send sensitive approval prompts (command text, cwd, nodeId).

If request.request.turnSourceChannel === "telegram", the handler uses turnSourceTo and turnSourceThreadId directly as the Telegram destination (only checking turnSourceAccountId when it is present). There is no validation that these fields actually correspond to the originating session/chat, nor that they match a session-store-derived delivery target.

Impact:

• A compromised or malicious component that can emit/call exec.approval.request (i.e., any gateway client with operator.approvals) can set turnSourceTo to an arbitrary Telegram chat ID (DM or group) that the bot can message.
• The handler will then send the approval prompt containing the command and metadata to that arbitrary chat, causing information disclosure/misdelivery.

Vulnerable code (target selection trusts request fields):

if (turnSourceChannel === "telegram" && turnSourceTo) {
  if (turnSourceAccountId && normalizeAccountId(turnSourceAccountId) !== normalizeAccountId(params.accountId)) {
    return null;
  }
  const threadId = /* parse */;
  return { to: turnSourceTo, threadId: Number.isFinite(threadId) ? threadId : undefined };
}

Sensitive content is then delivered to target.to:

await this.sendMessage(target.to, payload.text ?? "", { ... });

Notes:

• If turnSourceAccountId is omitted/empty, the account check is skipped, which can exacerbate misdelivery in multi-account deployments.

Recommendation

Do not treat turnSource* fields in ExecApprovalRequest as authoritative routing.

Hardening options (preferably combine 1 + 2):

1. Bind to session-store-derived target: when turnSourceChannel === "telegram", require that sessionKey exists and resolves (via session store) to the same to/accountId/threadId. If it doesn’t match, ignore channel routing (or fall back to DM approvers).

const direct = /* parse turnSourceTo/threadId */;
const sessionTarget = resolveRequestSessionTarget(params);
if (
  !sessionTarget ||
  sessionTarget.channel !== "telegram" ||
  sessionTarget.to !== direct.to ||
  (sessionTarget.threadId ?? undefined) !== (direct.threadId ?? undefined)
) {
  return null; // or fallbackToDm
}

2. Require account binding: if supporting multiple Telegram accounts, require a non-empty turnSourceAccountId and match it strictly.

3. Constrain acceptable destinations: optionally reject non-numeric chat IDs and/or destinations not present in an allowlist derived from session data.

Additionally, consider enforcing this upstream in the gateway: reject/strip turnSource* fields from exec.approval.request unless they are computed by trusted server-side context.

4. 🔵 Telegram exec approval prompts can be silently dropped due to unconditional local suppression

Description

The new Telegram suppression logic drops any ReplyPayload that contains channelData.execApproval metadata, without verifying that a Telegram exec-approval handler is actually configured/running (or that it will route this approval).

This causes a loss of the user-visible approval prompt (and can make the bot appear to “hang”) in situations such as:

• Telegram bot is running reply dispatch (createTelegramBot), but the separate monitor-side TelegramExecApprovalHandler is not running/connected (e.g., alternative runtime topology, transient gateway disconnect, or handler startup failure).
• TelegramExecApprovalHandler is enabled but its agentFilter / sessionFilter does not match a request (so it will not send prompts), while local dispatch still suppresses the only prompt payload.

Because the suppression paths also mark the reply as delivered (queuedFinal=true / deliveryState.delivered=true), Telegram fallback messages are skipped, increasing the likelihood of silent DoS (no approval instructions shown anywhere in the chat).

Vulnerable logic:

export function shouldSuppressLocalTelegramExecApprovalPrompt(...) {
  void params.cfg;
  void params.accountId;
  return getExecApprovalReplyMetadata(params.payload) !== null;
}

and its usage in Telegram dispatch paths:

• dispatchTelegramMessage returns early and sets queuedFinal = true
• native command dispatcher returns early and sets deliveryState.delivered = true

Recommendation

Only suppress the local approval prompt when you can ensure an alternative delivery path exists for this approval request.

Minimum hardening (match Discord behavior): gate suppression on Telegram exec-approvals being enabled for the account.

import { isTelegramExecApprovalClientEnabled } from "./exec-approvals.js";

export function shouldSuppressLocalTelegramExecApprovalPrompt(params: {
  cfg: OpenClawConfig;
  accountId?: string | null;
  payload: ReplyPayload;
}): boolean {
  return (
    isTelegramExecApprovalClientEnabled({ cfg: params.cfg, accountId: params.accountId }) &&
    getExecApprovalReplyMetadata(params.payload) !== null
  );
}

Stronger fix (prevents silent hangs):

• If suppression triggers, send a minimal notice to the originating chat (without channelData.execApproval) such as “Approval required; check approver DMs / web UI”, unless you are sending to the same chat (target: channel|both).
• Alternatively, avoid setting queuedFinal/delivered when suppressing unless you have confirmed that the monitor-side handler actually delivered a prompt.

This preserves availability and prevents missing security-relevant prompts due to handler outages or filter misconfiguration.

Analyzed PR: #37233 at commit f243379

_{Last updated on: 2026-03-10T05:10:33Z}

Greptile Summary

This PR fixes a multi-part regression in Telegram exec-approval flows: approval-id mismatch/rotation, missing approver gating on Telegram's /approve command, missing inline approval buttons, and post-approval follow-up messages landing in the wrong topic. The fix is well-scoped and comes with extensive new tests covering approval ID prefix resolution, button injection, forwarder Telegram delivery, and the suppressed-notify-on-exit path.

Key changes:

• ExecApprovalManager.lookupPendingId adds prefix-match + ambiguity resolution, consumed by the gateway resolve handler.
• New src/telegram/exec-approvals.ts module centralises Telegram approval config/approver checks used consistently across the forwarder, delivery layer, and /approve command handler.
• buildExecApprovalPendingReplyPayload emits structured channelData.execApproval metadata, enabling deterministic button injection without relying on text-regex extraction.
• sendExecApprovalFollowup replaces heartbeat/system-event delivery for post-approval results, preserving the originating topic/thread ID for Telegram forum topics.
• suppressNotifyOnExit flag prevents the legacy node-exit heartbeat path from firing a duplicate notification alongside the new direct follow-up.

Two issues found:

1. Misleading follow-up prompt preamble (src/agents/bash-tools.exec-approval-followup.ts): buildExecApprovalFollowupPrompt always opens with "An async command the user already approved has completed," even when called with denial result text such as "Exec denied (..., approval-request-failed): ..." where no approval ever occurred.
2. Duplicate forwarding for shared targets (src/infra/exec-approval-forwarder.ts): handleRequested and handleResolved concatenate resolveForwardTargets and resolveTelegramForwardTargets without cross-set deduplication. An approver present in both approvals.exec.targets and channels.telegram.execApprovals.approvers would receive two copies of every approval/resolution message.

Confidence Score: 3/5

• Safe to merge for core approval-ID and Telegram routing fixes; two logic issues should be addressed before relying on multi-channel forwarder configs in production.
• The approval-ID resolution, Telegram approver gating, and follow-up routing fixes address the stated regressions and are well-tested. Two verified issues remain: (1) misleading preamble in post-approval follow-up prompts for denial flows could nudge LLM toward incorrect framing, (2) missing cross-deduplication in forwarder targets can cause duplicate notifications if the same approver is configured in both general and Telegram-specific settings. Neither blocks the primary happy-path scenarios, but both should be fixed before multi-channel forwarder deployments.
• src/agents/bash-tools.exec-approval-followup.ts (preamble accuracy for denial flows) and src/infra/exec-approval-forwarder.ts (cross-deduplication of combined forward targets)

_{Last reviewed commit: 12025dd}

@greptile-apps please review latest code

❌ I'm refactoring to remove the heartbeat logic while preserving the ability for the notification of completion after the approval.

Merged via squash.

• Prepared head SHA: f2433790941841ade0efe6292ff4909b2edd6f18
• Merge commit: de49a8b72c12e89170f36143ac30aaa4e938aafc

Thanks @huntharo!