fix(approval-auth): prevent empty approver list from granting explicit approval authorization [AI] (#65714)

* fix: address issue

* fix: address PR review feedback

* fix: address PR review feedback

* docs: add changelog entry for PR merge

CHANGELOG.md

@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- fix(approval-auth): prevent empty approver list from granting explicit approval authorization [AI]. (#65714) Thanks @pgondhi987.
 - fix(security): broaden shell-wrapper detection and block env-argv assignment injection [AI-assisted]. (#65717) Thanks @pgondhi987.
 - Gateway/startup: defer scheduled services until sidecars finish, gate chat history and model listing during sidecar resume, and let Control UI retry startup-gated history loads so Sandbox wake resumes channels first. (#65365) Thanks @lml2468.
 - Control UI/chat: load the live gateway slash-command catalog into the composer and command palette so dock commands, plugin commands, and direct skill aliases appear in chat, while keeping trusted local commands authoritative and bounding remote command metadata. (#65620) Thanks @BunsDev.

src/auto-reply/reply/commands-approve.test.ts

@@ -115,7 +115,8 @@ const signalApproveTestPlugin: ChannelPlugin = {
   approvalCapability: createResolvedApproverActionAuthAdapter({
     channelLabel: "Signal",
     resolveApprovers: ({ cfg, accountId }) => {
-      const signal = accountId ? cfg.channels?.signal?.accounts?.[accountId] : cfg.channels?.signal;
+      const scopedSignal = accountId ? cfg.channels?.signal?.accounts?.[accountId] : undefined;
+      const signal = scopedSignal ?? cfg.channels?.signal;
       return resolveApprovalApprovers({
         allowFrom: signal?.allowFrom,
         defaultTo: signal?.defaultTo,
@@ -643,6 +644,62 @@ describe("handleApproveCommand", () => {
     expect(callGatewayMock).not.toHaveBeenCalled();
   });
 
+  it("does not allow empty helper approvers to bypass unauthorized sender checks", async () => {
+    const params = buildApproveParams(
+      "/approve abc12345 allow-once",
+      {
+        commands: { text: true },
+        channels: {
+          signal: {
+            allowFrom: [],
+          },
+        },
+      } as OpenClawConfig,
+      {
+        Provider: "signal",
+        Surface: "signal",
+        SenderId: "+15551239999",
+      },
+    );
+    params.command.isAuthorizedSender = false;
+
+    const result = await handleApproveCommand(params, true);
+    expect(result?.shouldContinue).toBe(false);
+    expect(result?.reply).toBeUndefined();
+    expect(callGatewayMock).not.toHaveBeenCalled();
+  });
+
+  it("keeps same-chat /approve available to authorized senders when helper approvers are empty", async () => {
+    callGatewayMock.mockResolvedValue({ ok: true });
+    const params = buildApproveParams(
+      "/approve abc12345 allow-once",
+      {
+        commands: { text: true },
+        channels: {
+          signal: {
+            allowFrom: [],
+          },
+        },
+      } as OpenClawConfig,
+      {
+        Provider: "signal",
+        Surface: "signal",
+        SenderId: "+15551239999",
+      },
+    );
+    params.command.isAuthorizedSender = true;
+
+    const result = await handleApproveCommand(params, true);
+    expect(result?.shouldContinue).toBe(false);
+    expect(result?.reply?.text).toContain("Approval allow-once submitted");
+    expect(callGatewayMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        method: "exec.approval.resolve",
+        params: { id: "abc12345", decision: "allow-once" },
+      }),
+    );
+  });
+
   it("accepts Telegram /approve from exec target recipients when native approvals are disabled", async () => {
     const params = buildApproveParams(
       "/approve abc12345 allow-once",

src/infra/channel-approval-auth.test.ts

@@ -1,4 +1,5 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createResolvedApproverActionAuthAdapter } from "../plugin-sdk/approval-auth-helpers.js";
 
 const getChannelPluginMock = vi.hoisted(() => vi.fn());
 
@@ -116,4 +117,42 @@ describe("resolveApprovalCommandAuthorization", () => {
       approvalKind: "exec",
     });
   });
+
+  it("keeps empty approver fallback implicit without bypassing channel sender auth", () => {
+    getChannelPluginMock.mockReturnValue({
+      approvalCapability: createResolvedApproverActionAuthAdapter({
+        channelLabel: "Signal",
+        resolveApprovers: () => [],
+      }),
+    });
+
+    expect(
+      resolveApprovalCommandAuthorization({
+        cfg: {} as never,
+        channel: "signal",
+        accountId: "work",
+        senderId: "uuid:attacker",
+        kind: "exec",
+      }),
+    ).toEqual({ authorized: true, explicit: false });
+  });
+
+  it("keeps configured approvers explicit when sender matches", () => {
+    getChannelPluginMock.mockReturnValue({
+      approvalCapability: createResolvedApproverActionAuthAdapter({
+        channelLabel: "Signal",
+        resolveApprovers: () => ["uuid:owner"],
+      }),
+    });
+
+    expect(
+      resolveApprovalCommandAuthorization({
+        cfg: {} as never,
+        channel: "signal",
+        accountId: "work",
+        senderId: "uuid:owner",
+        kind: "exec",
+      }),
+    ).toEqual({ authorized: true, explicit: true });
+  });
 });

src/infra/channel-approval-auth.ts

@@ -1,5 +1,6 @@
 import { getChannelPlugin, resolveChannelApprovalCapability } from "../channels/plugins/index.js";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
+import { isImplicitSameChatApprovalAuthorization } from "../plugin-sdk/approval-auth-helpers.js";
 import { normalizeMessageChannel } from "../utils/message-channel.js";
 
 export type ApprovalCommandAuthorization = {
@@ -30,6 +31,9 @@ export function resolveApprovalCommandAuthorization(params: {
   if (!resolved) {
     return { authorized: true, explicit: false };
   }
+  // Keep `resolved` by reference; cloning before this check would drop the
+  // non-enumerable implicit-fallback marker.
+  const implicitSameChatAuthorization = isImplicitSameChatApprovalAuthorization(resolved);
   const availability = approvalCapability?.getActionAvailabilityState?.({
     cfg: params.cfg,
     accountId: params.accountId,
@@ -39,6 +43,8 @@ export function resolveApprovalCommandAuthorization(params: {
   return {
     authorized: resolved.authorized,
     reason: resolved.reason,
-    explicit: resolved.authorized ? availability?.kind !== "disabled" : true,
+    explicit: resolved.authorized
+      ? !implicitSameChatAuthorization && availability?.kind !== "disabled"
+      : true,
   };
 }

src/plugin-sdk/approval-auth-helpers.test.ts

@@ -1,5 +1,8 @@
 import { describe, expect, it } from "vitest";
-import { createResolvedApproverActionAuthAdapter } from "./approval-auth-helpers.js";
+import {
+  createResolvedApproverActionAuthAdapter,
+  isImplicitSameChatApprovalAuthorization,
+} from "./approval-auth-helpers.js";
 
 describe("createResolvedApproverActionAuthAdapter", () => {
   it.each([
@@ -55,4 +58,36 @@ describe("createResolvedApproverActionAuthAdapter", () => {
       ).toEqual(testCase.expected);
     }
   });
+
+  it("marks empty-approver fallback auth as implicit", () => {
+    const auth = createResolvedApproverActionAuthAdapter({
+      channelLabel: "Signal",
+      resolveApprovers: () => [],
+    });
+    const result = auth.authorizeActorAction({
+      cfg: {},
+      senderId: "uuid:attacker",
+      action: "approve",
+      approvalKind: "exec",
+    });
+
+    expect(result).toEqual({ authorized: true });
+    expect(isImplicitSameChatApprovalAuthorization(result)).toBe(true);
+  });
+
+  it("does not mark configured-approver auth as implicit", () => {
+    const auth = createResolvedApproverActionAuthAdapter({
+      channelLabel: "Signal",
+      resolveApprovers: () => ["uuid:owner"],
+    });
+    const result = auth.authorizeActorAction({
+      cfg: {},
+      senderId: "uuid:owner",
+      action: "approve",
+      approvalKind: "exec",
+    });
+
+    expect(result).toEqual({ authorized: true });
+    expect(isImplicitSameChatApprovalAuthorization(result)).toBe(false);
+  });
 });

src/plugin-sdk/approval-auth-helpers.ts

@@ -2,6 +2,40 @@ import { normalizeOptionalString } from "../shared/string-coerce.js";
 import type { OpenClawConfig } from "./config-runtime.js";
 
 type ApprovalKind = "exec" | "plugin";
+type ApprovalAuthorizationResult = {
+  authorized: boolean;
+  reason?: string;
+};
+const IMPLICIT_SAME_CHAT_APPROVAL_AUTHORIZATION = Symbol(
+  "openclaw.implicitSameChatApprovalAuthorization",
+);
+
+function markImplicitSameChatApprovalAuthorization(
+  result: ApprovalAuthorizationResult,
+): ApprovalAuthorizationResult {
+  // Keep this non-enumerable to avoid changing auth payload shape.
+  // Consumers must pass the same object reference to
+  // `isImplicitSameChatApprovalAuthorization`; spread/Object.assign/JSON clones
+  // drop this marker.
+  Object.defineProperty(result, IMPLICIT_SAME_CHAT_APPROVAL_AUTHORIZATION, {
+    value: true,
+    enumerable: false,
+  });
+  return result;
+}
+
+export function isImplicitSameChatApprovalAuthorization(
+  result: ApprovalAuthorizationResult | null | undefined,
+): boolean {
+  return Boolean(
+    result &&
+    (
+      result as ApprovalAuthorizationResult & {
+        [IMPLICIT_SAME_CHAT_APPROVAL_AUTHORIZATION]?: true;
+      }
+    )[IMPLICIT_SAME_CHAT_APPROVAL_AUTHORIZATION],
+  );
+}
 
 export function createResolvedApproverActionAuthAdapter(params: {
   channelLabel: string;
@@ -25,7 +59,8 @@ export function createResolvedApproverActionAuthAdapter(params: {
     }) {
       const approvers = params.resolveApprovers({ cfg, accountId });
       if (approvers.length === 0) {
-        return { authorized: true } as const;
+        // Empty approver sets are implicit same-chat fallback, not explicit approver bypass.
+        return markImplicitSameChatApprovalAuthorization({ authorized: true });
       }
       const normalizedSenderId = senderId ? normalizeSenderId(senderId) : undefined;
       if (normalizedSenderId && approvers.includes(normalizedSenderId)) {