fix: cover slack open policy gating (#1563) (thanks @itsjaydesu)

Address reviewer feedback: slash commands now use the same
hasExplicitConfig check as regular messages, so unlisted
channels are allowed under groupPolicy: "open" for both
message handling and slash commands.

CHANGELOG.md

@@ -19,6 +19,7 @@ Docs: https://docs.clawd.bot
 - Channels: allow per-group tool allow/deny policies across built-in + plugin channels. (#1546) Thanks @adam91holt.
 
 ### Fixes
+- Slack: honor open groupPolicy for unlisted channels in message + slash gating. (#1563) Thanks @itsjaydesu.
 - Agents: ignore IDENTITY.md template placeholders when parsing identity to avoid placeholder replies. (#1556)
 - Docker: update gateway command in docker-compose and Hetzner guide. (#1514)
 - Sessions: reject array-backed session stores to prevent silent wipes. (#1469)

src/agents/anthropic-payload-log.ts

@@ -90,6 +90,18 @@ function safeJsonStringify(value: unknown): string | null {
   }
 }
 
+function formatError(error: unknown): string | undefined {
+  if (error instanceof Error) return error.message;
+  if (typeof error === "string") return error;
+  if (typeof error === "number" || typeof error === "boolean" || typeof error === "bigint") {
+    return String(error);
+  }
+  if (error && typeof error === "object") {
+    return safeJsonStringify(error) ?? "unknown error";
+  }
+  return undefined;
+}
+
 function digest(value: unknown): string | undefined {
   const serialized = safeJsonStringify(value);
   if (!serialized) return undefined;
@@ -163,7 +175,7 @@ export function createAnthropicPayloadLogger(params: {
         options?.onPayload?.(payload);
       };
       return streamFn(model, context, {
-        ...(options ?? {}),
+        ...options,
         onPayload: nextOnPayload,
       });
     };
@@ -172,13 +184,14 @@ export function createAnthropicPayloadLogger(params: {
 
   const recordUsage: AnthropicPayloadLogger["recordUsage"] = (messages, error) => {
     const usage = findLastAssistantUsage(messages);
+    const errorMessage = formatError(error);
     if (!usage) {
-      if (error) {
+      if (errorMessage) {
         record({
           ...base,
           ts: new Date().toISOString(),
           stage: "usage",
-          error: String(error),
+          error: errorMessage,
         });
       }
       return;
@@ -188,7 +201,7 @@ export function createAnthropicPayloadLogger(params: {
       ts: new Date().toISOString(),
       stage: "usage",
       usage,
-      error: error ? String(error) : undefined,
+      error: errorMessage,
     });
     log.info("anthropic usage", {
       runId: params.runId,

src/plugins/commands.ts

@@ -195,7 +195,13 @@ function sanitizeArgs(args: string | undefined): string | undefined {
   }
 
   // Remove control characters (except newlines and tabs which may be intentional)
-  return args.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, "");
+  let sanitized = "";
+  for (const char of args) {
+    const code = char.charCodeAt(0);
+    const isControl = (code <= 0x1f && code !== 0x09 && code !== 0x0a) || code === 0x7f;
+    if (!isControl) sanitized += char;
+  }
+  return sanitized;
 }
 
 /**

src/slack/monitor/context.test.ts

@@ -60,3 +60,61 @@ describe("resolveSlackSystemEventSessionKey", () => {
     );
   });
 });
+
+describe("isChannelAllowed with groupPolicy and channelsConfig", () => {
+  it("allows unlisted channels when groupPolicy is open even with channelsConfig entries", () => {
+    // Bug fix: when groupPolicy="open" and channels has some entries,
+    // unlisted channels should still be allowed (not blocked)
+    const ctx = createSlackMonitorContext({
+      ...baseParams(),
+      groupPolicy: "open",
+      channelsConfig: {
+        C_LISTED: { requireMention: true },
+      },
+    });
+    // Listed channel should be allowed
+    expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true);
+    // Unlisted channel should ALSO be allowed when policy is "open"
+    expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(true);
+  });
+
+  it("blocks unlisted channels when groupPolicy is allowlist", () => {
+    const ctx = createSlackMonitorContext({
+      ...baseParams(),
+      groupPolicy: "allowlist",
+      channelsConfig: {
+        C_LISTED: { requireMention: true },
+      },
+    });
+    // Listed channel should be allowed
+    expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true);
+    // Unlisted channel should be blocked when policy is "allowlist"
+    expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(false);
+  });
+
+  it("blocks explicitly denied channels even when groupPolicy is open", () => {
+    const ctx = createSlackMonitorContext({
+      ...baseParams(),
+      groupPolicy: "open",
+      channelsConfig: {
+        C_ALLOWED: { allow: true },
+        C_DENIED: { allow: false },
+      },
+    });
+    // Explicitly allowed channel
+    expect(ctx.isChannelAllowed({ channelId: "C_ALLOWED", channelType: "channel" })).toBe(true);
+    // Explicitly denied channel should be blocked even with open policy
+    expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
+    // Unlisted channel should be allowed with open policy
+    expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(true);
+  });
+
+  it("allows all channels when groupPolicy is open and channelsConfig is empty", () => {
+    const ctx = createSlackMonitorContext({
+      ...baseParams(),
+      groupPolicy: "open",
+      channelsConfig: undefined,
+    });
+    expect(ctx.isChannelAllowed({ channelId: "C_ANY", channelType: "channel" })).toBe(true);
+  });
+});

src/slack/monitor/context.ts

@@ -327,7 +327,11 @@ export function createSlackMonitorContext(params: {
         );
         return false;
       }
-      if (!channelAllowed) {
+      // When groupPolicy is "open", only block channels that are EXPLICITLY denied
+      // (i.e., have a matching config entry with allow:false). Channels not in the
+      // config (matchSource undefined) should be allowed under open policy.
+      const hasExplicitConfig = Boolean(channelConfig?.matchSource);
+      if (!channelAllowed && (params.groupPolicy !== "open" || hasExplicitConfig)) {
         logVerbose(`slack: drop channel ${p.channelId} (${channelMatchMeta})`);
         return false;
       }

src/slack/monitor/slash.policy.test.ts

@@ -0,0 +1,186 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { registerSlackMonitorSlashCommands } from "./slash.js";
+
+const dispatchMock = vi.fn();
+const readAllowFromStoreMock = vi.fn();
+const upsertPairingRequestMock = vi.fn();
+const resolveAgentRouteMock = vi.fn();
+
+vi.mock("../../auto-reply/reply/provider-dispatcher.js", () => ({
+  dispatchReplyWithDispatcher: (...args: unknown[]) => dispatchMock(...args),
+}));
+
+vi.mock("../../pairing/pairing-store.js", () => ({
+  readChannelAllowFromStore: (...args: unknown[]) => readAllowFromStoreMock(...args),
+  upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args),
+}));
+
+vi.mock("../../routing/resolve-route.js", () => ({
+  resolveAgentRoute: (...args: unknown[]) => resolveAgentRouteMock(...args),
+}));
+
+vi.mock("../../agents/identity.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../../agents/identity.js")>();
+  return {
+    ...actual,
+    resolveEffectiveMessagesConfig: () => ({ responsePrefix: "" }),
+  };
+});
+
+function createHarness(overrides?: {
+  groupPolicy?: "open" | "allowlist";
+  channelsConfig?: Record<string, { allow?: boolean; requireMention?: boolean }>;
+  channelId?: string;
+  channelName?: string;
+}) {
+  const commands = new Map<unknown, (args: unknown) => Promise<void>>();
+  const postEphemeral = vi.fn().mockResolvedValue({ ok: true });
+  const app = {
+    client: { chat: { postEphemeral } },
+    command: (name: unknown, handler: (args: unknown) => Promise<void>) => {
+      commands.set(name, handler);
+    },
+  };
+
+  const channelId = overrides?.channelId ?? "C_UNLISTED";
+  const channelName = overrides?.channelName ?? "unlisted";
+
+  const ctx = {
+    cfg: { commands: { native: false } },
+    runtime: {},
+    botToken: "bot-token",
+    botUserId: "bot",
+    teamId: "T1",
+    allowFrom: ["*"],
+    dmEnabled: true,
+    dmPolicy: "open",
+    groupDmEnabled: false,
+    groupDmChannels: [],
+    defaultRequireMention: true,
+    groupPolicy: overrides?.groupPolicy ?? "open",
+    useAccessGroups: true,
+    channelsConfig: overrides?.channelsConfig,
+    slashCommand: { enabled: true, name: "clawd", ephemeral: true, sessionPrefix: "slack:slash" },
+    textLimit: 4000,
+    app,
+    isChannelAllowed: () => true,
+    resolveChannelName: async () => ({ name: channelName, type: "channel" }),
+    resolveUserName: async () => ({ name: "Ada" }),
+  } as unknown;
+
+  const account = { accountId: "acct", config: { commands: { native: false } } } as unknown;
+
+  return { commands, ctx, account, postEphemeral, channelId, channelName };
+}
+
+beforeEach(() => {
+  dispatchMock.mockReset().mockResolvedValue({ counts: { final: 1, tool: 0, block: 0 } });
+  readAllowFromStoreMock.mockReset().mockResolvedValue([]);
+  upsertPairingRequestMock.mockReset().mockResolvedValue({ code: "PAIRCODE", created: true });
+  resolveAgentRouteMock.mockReset().mockReturnValue({
+    agentId: "main",
+    sessionKey: "session:1",
+    accountId: "acct",
+  });
+});
+
+describe("slack slash commands channel policy", () => {
+  it("allows unlisted channels when groupPolicy is open", async () => {
+    const { commands, ctx, account, channelId, channelName } = createHarness({
+      groupPolicy: "open",
+      channelsConfig: { C_LISTED: { requireMention: true } },
+      channelId: "C_UNLISTED",
+      channelName: "unlisted",
+    });
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = [...commands.values()][0];
+    if (!handler) throw new Error("Missing slash handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    await handler({
+      command: {
+        user_id: "U1",
+        user_name: "Ada",
+        channel_id: channelId,
+        channel_name: channelName,
+        text: "hello",
+        trigger_id: "t1",
+      },
+      ack: vi.fn().mockResolvedValue(undefined),
+      respond,
+    });
+
+    expect(dispatchMock).toHaveBeenCalledTimes(1);
+    expect(respond).not.toHaveBeenCalledWith(
+      expect.objectContaining({ text: "This channel is not allowed." }),
+    );
+  });
+
+  it("blocks explicitly denied channels when groupPolicy is open", async () => {
+    const { commands, ctx, account, channelId, channelName } = createHarness({
+      groupPolicy: "open",
+      channelsConfig: { C_DENIED: { allow: false } },
+      channelId: "C_DENIED",
+      channelName: "denied",
+    });
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = [...commands.values()][0];
+    if (!handler) throw new Error("Missing slash handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    await handler({
+      command: {
+        user_id: "U1",
+        user_name: "Ada",
+        channel_id: channelId,
+        channel_name: channelName,
+        text: "hello",
+        trigger_id: "t1",
+      },
+      ack: vi.fn().mockResolvedValue(undefined),
+      respond,
+    });
+
+    expect(dispatchMock).not.toHaveBeenCalled();
+    expect(respond).toHaveBeenCalledWith({
+      text: "This channel is not allowed.",
+      response_type: "ephemeral",
+    });
+  });
+
+  it("blocks unlisted channels when groupPolicy is allowlist", async () => {
+    const { commands, ctx, account, channelId, channelName } = createHarness({
+      groupPolicy: "allowlist",
+      channelsConfig: { C_LISTED: { requireMention: true } },
+      channelId: "C_UNLISTED",
+      channelName: "unlisted",
+    });
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = [...commands.values()][0];
+    if (!handler) throw new Error("Missing slash handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    await handler({
+      command: {
+        user_id: "U1",
+        user_name: "Ada",
+        channel_id: channelId,
+        channel_name: channelName,
+        text: "hello",
+        trigger_id: "t1",
+      },
+      ack: vi.fn().mockResolvedValue(undefined),
+      respond,
+    });
+
+    expect(dispatchMock).not.toHaveBeenCalled();
+    expect(respond).toHaveBeenCalledWith({
+      text: "This channel is not allowed.",
+      response_type: "ephemeral",
+    });
+  });
+});

src/slack/monitor/slash.ts

@@ -262,8 +262,7 @@ export function registerSlackMonitorSlashCommands(params: {
               groupPolicy: ctx.groupPolicy,
               channelAllowlistConfigured,
               channelAllowed,
-            }) ||
-            !channelAllowed
+            })
           ) {
             await respond({
               text: "This channel is not allowed.",
@@ -271,13 +270,17 @@ export function registerSlackMonitorSlashCommands(params: {
             });
             return;
           }
-        }
-        if (ctx.useAccessGroups && channelConfig?.allowed === false) {
-          await respond({
-            text: "This channel is not allowed.",
-            response_type: "ephemeral",
-          });
-          return;
+          // When groupPolicy is "open", only block channels that are EXPLICITLY denied
+          // (i.e., have a matching config entry with allow:false). Channels not in the
+          // config (matchSource undefined) should be allowed under open policy.
+          const hasExplicitConfig = Boolean(channelConfig?.matchSource);
+          if (!channelAllowed && (ctx.groupPolicy !== "open" || hasExplicitConfig)) {
+            await respond({
+              text: "This channel is not allowed.",
+              response_type: "ephemeral",
+            });
+            return;
+          }
         }
       }