fix(commands): restrict commands.allowFrom to sender principals

steipete · steipete · commit 08e2aa44e78a · 2026-02-24T02:01:01.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- Security/Commands: enforce sender-only matching for `commands.allowFrom` by blocking conversation-shaped `From` identities (`channel:`, `group:`, `thread:`, `@g.us`) while preserving direct-message fallback when sender fields are missing. Ships in the next npm release. Thanks @jiseoung.
 - Security/Config writes: block reserved prototype keys in account-id normalization and route account config resolution through own-key lookups, hardening `/allowlist` and account-scoped config paths against prototype-chain pollution.
 - Security/Exec: harden `safeBins` long-option validation by rejecting unknown/ambiguous GNU long-option abbreviations and denying sort filesystem-dependent flags (`--random-source`, `--temporary-directory`, `-T`), closing safe-bin denylist bypasses. Thanks @jiseoung.
 - Security/Channels: unify dangerous name-matching policy checks (`dangerouslyAllowNameMatching`) across core and extension channels, share mutable-allowlist detectors between `openclaw doctor` and `openclaw security audit`, and scan all configured accounts (not only the default account) in channel security audit findings.
diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts
@@ -176,6 +176,35 @@ function resolveCommandsAllowFromList(params: {
   });
 }
 
+function isConversationLikeIdentity(value: string): boolean {
+  const normalized = value.trim().toLowerCase();
+  if (!normalized) {
+    return false;
+  }
+  if (normalized.includes("@g.us")) {
+    return true;
+  }
+  if (normalized.startsWith("chat_id:")) {
+    return true;
+  }
+  return /(^|:)(channel|group|thread|topic|room|space|spaces):/.test(normalized);
+}
+
+function shouldUseFromAsSenderFallback(params: {
+  from?: string | null;
+  chatType?: string | null;
+}): boolean {
+  const from = (params.from ?? "").trim();
+  if (!from) {
+    return false;
+  }
+  const chatType = (params.chatType ?? "").trim().toLowerCase();
+  if (chatType && chatType !== "direct") {
+    return false;
+  }
+  return !isConversationLikeIdentity(from);
+}
+
 function resolveSenderCandidates(params: {
   dock?: ChannelDock;
   providerId?: ChannelId;
@@ -184,6 +213,7 @@ function resolveSenderCandidates(params: {
   senderId?: string | null;
   senderE164?: string | null;
   from?: string | null;
+  chatType?: string | null;
 }): string[] {
   const { dock, cfg, accountId } = params;
   const candidates: string[] = [];
@@ -201,7 +231,12 @@ function resolveSenderCandidates(params: {
     pushCandidate(params.senderId);
     pushCandidate(params.senderE164);
   }
-  pushCandidate(params.from);
+  if (
+    candidates.length === 0 &&
+    shouldUseFromAsSenderFallback({ from: params.from, chatType: params.chatType })
+  ) {
+    pushCandidate(params.from);
+  }
 
   const normalized: string[] = [];
   for (const sender of candidates) {
@@ -295,6 +330,7 @@ export function resolveCommandAuthorization(params: {
     senderId: ctx.SenderId,
     senderE164: ctx.SenderE164,
     from,
+    chatType: ctx.ChatType,
   });
   const matchedSender = ownerList.length
     ? senderCandidates.find((candidate) => ownerList.includes(candidate))
diff --git a/src/auto-reply/command-control.test.ts b/src/auto-reply/command-control.test.ts
@@ -343,6 +343,79 @@ describe("resolveCommandAuthorization", () => {
       expect(auth.isAuthorizedSender).toBe(true);
     });
 
+    it("does not treat conversation ids in From as sender identities", () => {
+      const cfg = {
+        commands: {
+          allowFrom: {
+            discord: ["channel:123456789012345678"],
+          },
+        },
+      } as OpenClawConfig;
+
+      const auth = resolveCommandAuthorization({
+        ctx: {
+          Provider: "discord",
+          Surface: "discord",
+          ChatType: "channel",
+          From: "discord:channel:123456789012345678",
+          SenderId: "999999999999999999",
+        } as MsgContext,
+        cfg,
+        commandAuthorized: false,
+      });
+
+      expect(auth.isAuthorizedSender).toBe(false);
+    });
+
+    it("still falls back to From for direct messages when sender fields are absent", () => {
+      const cfg = {
+        commands: {
+          allowFrom: {
+            discord: ["123456789012345678"],
+          },
+        },
+      } as OpenClawConfig;
+
+      const auth = resolveCommandAuthorization({
+        ctx: {
+          Provider: "discord",
+          Surface: "discord",
+          ChatType: "direct",
+          From: "discord:123456789012345678",
+          SenderId: " ",
+          SenderE164: " ",
+        } as MsgContext,
+        cfg,
+        commandAuthorized: false,
+      });
+
+      expect(auth.isAuthorizedSender).toBe(true);
+    });
+
+    it("does not fall back to conversation-shaped From when chat type is missing", () => {
+      const cfg = {
+        commands: {
+          allowFrom: {
+            "*": ["120363411111111111@g.us"],
+          },
+        },
+      } as OpenClawConfig;
+
+      const auth = resolveCommandAuthorization({
+        ctx: {
+          Provider: "whatsapp",
+          Surface: "whatsapp",
+          From: "120363411111111111@g.us",
+          SenderId: " ",
+          SenderE164: " ",
+        } as MsgContext,
+        cfg,
+        commandAuthorized: false,
+      });
+
+      expect(auth.isAuthorizedSender).toBe(false);
+    });
+
     it("normalizes Discord commands.allowFrom prefixes and mentions", () => {
       const cfg = {
         commands: {
