fix(security): enforce explicit ingress owner context

2026-05-09 05:17:40 +00:00 · 2026-03-02 23:50:30 +00:00
parent ea3b7dfde5
commit b8b8a5f314
17 changed files with 471 additions and 92 deletions
--- a/src/discord/monitor/agent-components.ts
+++ b/src/discord/monitor/agent-components.ts
@@ -61,6 +61,7 @@ import {
  resolveDiscordChannelConfigWithFallback,
  resolveDiscordGuildEntry,
  resolveDiscordMemberAccessState,
+  resolveDiscordOwnerAccess,
  resolveDiscordOwnerAllowFrom,
 } from "./allow-list.js";
 import { formatDiscordUserTag } from "./format.js";
@@ -764,18 +765,15 @@ function resolveComponentCommandAuthorized(params: {
    return true;
  }

-  const ownerAllowList = normalizeDiscordAllowList(ctx.allowFrom, ["discord:", "user:", "pk:"]);
-  const ownerOk = ownerAllowList
-    ? resolveDiscordAllowListMatch({
-        allowList: ownerAllowList,
-        candidate: {
-          id: interactionCtx.user.id,
-          name: interactionCtx.user.username,
-          tag: formatDiscordUserTag(interactionCtx.user),
-        },
-        allowNameMatching: params.allowNameMatching,
-      }).allowed
-    : false;
+  const { ownerAllowList, ownerAllowed: ownerOk } = resolveDiscordOwnerAccess({
+    allowFrom: ctx.allowFrom,
+    sender: {
+      id: interactionCtx.user.id,
+      name: interactionCtx.user.username,
+      tag: formatDiscordUserTag(interactionCtx.user),
+    },
+    allowNameMatching: params.allowNameMatching,
+  });

  const { hasAccessRestrictions, memberAllowed } = resolveDiscordMemberAccessState({
    channelConfig,
--- a/src/discord/monitor/allow-list.ts
+++ b/src/discord/monitor/allow-list.ts
@@ -16,6 +16,8 @@ export type DiscordAllowList = {

 export type DiscordAllowListMatch = AllowlistMatch<"wildcard" | "id" | "name" | "tag">;

+const DISCORD_OWNER_ALLOWLIST_PREFIXES = ["discord:", "user:", "pk:"];
+
 export type DiscordGuildEntryResolved = {
  id?: string;
  slug?: string;
@@ -265,6 +267,32 @@ export function resolveDiscordOwnerAllowFrom(params: {
  return [match.matchKey];
 }

+export function resolveDiscordOwnerAccess(params: {
+  allowFrom?: string[];
+  sender: { id: string; name?: string; tag?: string };
+  allowNameMatching?: boolean;
+}): {
+  ownerAllowList: DiscordAllowList | null;
+  ownerAllowed: boolean;
+} {
+  const ownerAllowList = normalizeDiscordAllowList(
+    params.allowFrom,
+    DISCORD_OWNER_ALLOWLIST_PREFIXES,
+  );
+  const ownerAllowed = ownerAllowList
+    ? allowListMatches(
+        ownerAllowList,
+        {
+          id: params.sender.id,
+          name: params.sender.name,
+          tag: params.sender.tag,
+        },
+        { allowNameMatching: params.allowNameMatching },
+      )
+    : false;
+  return { ownerAllowList, ownerAllowed };
+}
+
 export function resolveDiscordCommandAuthorized(params: {
  isDirectMessage: boolean;
  allowFrom?: string[];
--- a/src/discord/monitor/message-handler.preflight.ts
+++ b/src/discord/monitor/message-handler.preflight.ts
@@ -30,13 +30,12 @@ import { DEFAULT_ACCOUNT_ID, resolveAgentIdFromSessionKey } from "../../routing/
 import { fetchPluralKitMessageInfo } from "../pluralkit.js";
 import { sendMessageDiscord } from "../send.js";
 import {
-  allowListMatches,
  isDiscordGroupAllowedByPolicy,
-  normalizeDiscordAllowList,
  normalizeDiscordSlug,
  resolveDiscordChannelConfigWithFallback,
  resolveDiscordGuildEntry,
  resolveDiscordMemberAccessState,
+  resolveDiscordOwnerAccess,
  resolveDiscordShouldRequireMention,
  resolveGroupDmAllow,
 } from "./allow-list.js";
@@ -549,22 +548,15 @@ export async function preflightDiscordMessage(
  });

  if (!isDirectMessage) {
-    const ownerAllowList = normalizeDiscordAllowList(params.allowFrom, [
-      "discord:",
-      "user:",
-      "pk:",
-    ]);
-    const ownerOk = ownerAllowList
-      ? allowListMatches(
-          ownerAllowList,
-          {
-            id: sender.id,
-            name: sender.name,
-            tag: sender.tag,
-          },
-          { allowNameMatching },
-        )
-      : false;
+    const { ownerAllowList, ownerAllowed: ownerOk } = resolveDiscordOwnerAccess({
+      allowFrom: params.allowFrom,
+      sender: {
+        id: sender.id,
+        name: sender.name,
+        tag: sender.tag,
+      },
+      allowNameMatching,
+    });
    const commandGate = resolveControlCommandGate({
      useAccessGroups,
      authorizers: [
--- a/src/discord/monitor/native-command.ts
+++ b/src/discord/monitor/native-command.ts
@@ -54,13 +54,12 @@ import { withTimeout } from "../../utils/with-timeout.js";
 import { loadWebMedia } from "../../web/media.js";
 import { chunkDiscordTextWithMode } from "../chunk.js";
 import {
-  allowListMatches,
  isDiscordGroupAllowedByPolicy,
-  normalizeDiscordAllowList,
  normalizeDiscordSlug,
  resolveDiscordChannelConfigWithFallback,
  resolveDiscordGuildEntry,
  resolveDiscordMemberAccessState,
+  resolveDiscordOwnerAccess,
  resolveDiscordOwnerAllowFrom,
 } from "./allow-list.js";
 import { resolveDiscordDmCommandAccess } from "./dm-command-auth.js";
@@ -1270,22 +1269,15 @@ async function dispatchDiscordCommandInteraction(params: {
    ? interaction.rawData.member.roles.map((roleId: string) => String(roleId))
    : [];
  const allowNameMatching = isDangerousNameMatchingEnabled(discordConfig);
-  const ownerAllowList = normalizeDiscordAllowList(
-    discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? [],
-    ["discord:", "user:", "pk:"],
-  );
-  const ownerOk =
-    ownerAllowList && user
-      ? allowListMatches(
-          ownerAllowList,
-          {
-            id: sender.id,
-            name: sender.name,
-            tag: sender.tag,
-          },
-          { allowNameMatching },
-        )
-      : false;
+  const { ownerAllowList, ownerAllowed: ownerOk } = resolveDiscordOwnerAccess({
+    allowFrom: discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? [],
+    sender: {
+      id: sender.id,
+      name: sender.name,
+      tag: sender.tag,
+    },
+    allowNameMatching,
+  });
  const guildInfo = resolveDiscordGuildEntry({
    guild: interaction.guild ?? undefined,
    guildEntries: discordConfig?.guilds,
--- a/src/discord/voice/command.ts
+++ b/src/discord/voice/command.ts
@@ -15,10 +15,9 @@ import type { OpenClawConfig } from "../../config/config.js";
 import { isDangerousNameMatchingEnabled } from "../../config/dangerous-name-matching.js";
 import type { DiscordAccountConfig } from "../../config/types.js";
 import {
-  allowListMatches,
  isDiscordGroupAllowedByPolicy,
-  normalizeDiscordAllowList,
  normalizeDiscordSlug,
+  resolveDiscordOwnerAccess,
  resolveDiscordChannelConfigWithFallback,
  resolveDiscordGuildEntry,
  resolveDiscordMemberAccessState,
@@ -160,21 +159,15 @@ async function authorizeVoiceCommand(
    allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig),
  });

-  const ownerAllowList = normalizeDiscordAllowList(
-    params.discordConfig.allowFrom ?? params.discordConfig.dm?.allowFrom ?? [],
-    ["discord:", "user:", "pk:"],
-  );
-  const ownerOk = ownerAllowList
-    ? allowListMatches(
-        ownerAllowList,
-        {
-          id: sender.id,
-          name: sender.name,
-          tag: sender.tag,
-        },
-        { allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig) },
-      )
-    : false;
+  const { ownerAllowList, ownerAllowed: ownerOk } = resolveDiscordOwnerAccess({
+    allowFrom: params.discordConfig.allowFrom ?? params.discordConfig.dm?.allowFrom ?? [],
+    sender: {
+      id: sender.id,
+      name: sender.name,
+      tag: sender.tag,
+    },
+    allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig),
+  });

  const authorizers = params.useAccessGroups
    ? [
--- a/src/discord/voice/manager.e2e.test.ts
+++ b/src/discord/voice/manager.e2e.test.ts
@@ -7,6 +7,11 @@ const {
  entersStateMock,
  createAudioPlayerMock,
  resolveAgentRouteMock,
+  agentCommandMock,
+  buildProviderRegistryMock,
+  createMediaAttachmentCacheMock,
+  normalizeMediaAttachmentsMock,
+  runCapabilityMock,
 } = vi.hoisted(() => {
  type EventHandler = (...args: unknown[]) => unknown;
  type MockConnection = {
@@ -62,6 +67,15 @@ const {
      state: { status: "idle" },
    })),
    resolveAgentRouteMock: vi.fn(() => ({ agentId: "agent-1", sessionKey: "discord:g1:c1" })),
+    agentCommandMock: vi.fn(async (_opts?: unknown, _runtime?: unknown) => ({ payloads: [] })),
+    buildProviderRegistryMock: vi.fn(() => ({})),
+    createMediaAttachmentCacheMock: vi.fn(() => ({
+      cleanup: vi.fn(async () => undefined),
+    })),
+    normalizeMediaAttachmentsMock: vi.fn(() => [{ kind: "audio", path: "/tmp/test.wav" }]),
+    runCapabilityMock: vi.fn(async () => ({
+      outputs: [{ kind: "audio.transcription", text: "hello from voice" }],
+    })),
  };
 });

@@ -85,6 +99,17 @@ vi.mock("../../routing/resolve-route.js", () => ({
  resolveAgentRoute: resolveAgentRouteMock,
 }));

+vi.mock("../../commands/agent.js", () => ({
+  agentCommandFromIngress: agentCommandMock,
+}));
+
+vi.mock("../../media-understanding/runner.js", () => ({
+  buildProviderRegistry: buildProviderRegistryMock,
+  createMediaAttachmentCache: createMediaAttachmentCacheMock,
+  normalizeMediaAttachments: normalizeMediaAttachmentsMock,
+  runCapability: runCapabilityMock,
+}));
+
 let managerModule: typeof import("./manager.js");

 function createClient() {
@@ -122,15 +147,27 @@ describe("DiscordVoiceManager", () => {
    entersStateMock.mockResolvedValue(undefined);
    createAudioPlayerMock.mockClear();
    resolveAgentRouteMock.mockClear();
+    agentCommandMock.mockReset();
+    agentCommandMock.mockResolvedValue({ payloads: [] });
+    buildProviderRegistryMock.mockReset();
+    buildProviderRegistryMock.mockReturnValue({});
+    createMediaAttachmentCacheMock.mockClear();
+    normalizeMediaAttachmentsMock.mockReset();
+    normalizeMediaAttachmentsMock.mockReturnValue([{ kind: "audio", path: "/tmp/test.wav" }]);
+    runCapabilityMock.mockReset();
+    runCapabilityMock.mockResolvedValue({
+      outputs: [{ kind: "audio.transcription", text: "hello from voice" }],
+    });
  });

  const createManager = (
    discordConfig: ConstructorParameters<
      typeof managerModule.DiscordVoiceManager
    >[0]["discordConfig"] = {},
+    clientOverride?: ReturnType<typeof createClient>,
  ) =>
    new managerModule.DiscordVoiceManager({
-      client: createClient() as never,
+      client: (clientOverride ?? createClient()) as never,
      cfg: {},
      discordConfig,
      accountId: "default",
@@ -248,4 +285,119 @@ describe("DiscordVoiceManager", () => {

    expect(joinVoiceChannelMock).toHaveBeenCalledTimes(2);
  });
+
+  it("passes senderIsOwner=true for allowlisted voice speakers", async () => {
+    const client = createClient();
+    client.fetchMember.mockResolvedValue({
+      nickname: "Owner Nick",
+      user: {
+        id: "u-owner",
+        username: "owner",
+        globalName: "Owner",
+        discriminator: "1234",
+      },
+    });
+    const manager = createManager({ allowFrom: ["discord:u-owner"] }, client);
+    await (
+      manager as unknown as {
+        processSegment: (params: {
+          entry: unknown;
+          wavPath: string;
+          userId: string;
+          durationSeconds: number;
+        }) => Promise<void>;
+      }
+    ).processSegment({
+      entry: {
+        guildId: "g1",
+        channelId: "c1",
+        route: { sessionKey: "discord:g1:c1", agentId: "agent-1" },
+      },
+      wavPath: "/tmp/test.wav",
+      userId: "u-owner",
+      durationSeconds: 1.2,
+    });
+
+    const commandArgs = agentCommandMock.mock.calls.at(-1)?.[0] as
+      | { senderIsOwner?: boolean }
+      | undefined;
+    expect(commandArgs?.senderIsOwner).toBe(true);
+  });
+
+  it("passes senderIsOwner=false for non-owner voice speakers", async () => {
+    const client = createClient();
+    client.fetchMember.mockResolvedValue({
+      nickname: "Guest Nick",
+      user: {
+        id: "u-guest",
+        username: "guest",
+        globalName: "Guest",
+        discriminator: "4321",
+      },
+    });
+    const manager = createManager({ allowFrom: ["discord:u-owner"] }, client);
+    await (
+      manager as unknown as {
+        processSegment: (params: {
+          entry: unknown;
+          wavPath: string;
+          userId: string;
+          durationSeconds: number;
+        }) => Promise<void>;
+      }
+    ).processSegment({
+      entry: {
+        guildId: "g1",
+        channelId: "c1",
+        route: { sessionKey: "discord:g1:c1", agentId: "agent-1" },
+      },
+      wavPath: "/tmp/test.wav",
+      userId: "u-guest",
+      durationSeconds: 1.2,
+    });
+
+    const commandArgs = agentCommandMock.mock.calls.at(-1)?.[0] as
+      | { senderIsOwner?: boolean }
+      | undefined;
+    expect(commandArgs?.senderIsOwner).toBe(false);
+  });
+
+  it("reuses speaker context cache for repeated segments from the same speaker", async () => {
+    const client = createClient();
+    client.fetchMember.mockResolvedValue({
+      nickname: "Cached Speaker",
+      user: {
+        id: "u-cache",
+        username: "cache",
+        globalName: "Cache",
+        discriminator: "1111",
+      },
+    });
+    const manager = createManager({ allowFrom: ["discord:u-cache"] }, client);
+    const runSegment = async () =>
+      await (
+        manager as unknown as {
+          processSegment: (params: {
+            entry: unknown;
+            wavPath: string;
+            userId: string;
+            durationSeconds: number;
+          }) => Promise<void>;
+        }
+      ).processSegment({
+        entry: {
+          guildId: "g1",
+          channelId: "c1",
+          route: { sessionKey: "discord:g1:c1", agentId: "agent-1" },
+        },
+        wavPath: "/tmp/test.wav",
+        userId: "u-cache",
+        durationSeconds: 1.2,
+      });
+
+    await runSegment();
+    await runSegment();
+
+    expect(client.fetchMember).toHaveBeenCalledTimes(1);
+  });
 });
--- a/src/discord/voice/manager.ts
+++ b/src/discord/voice/manager.ts
@@ -18,8 +18,9 @@ import {
 } from "@discordjs/voice";
 import { resolveAgentDir } from "../../agents/agent-scope.js";
 import type { MsgContext } from "../../auto-reply/templating.js";
-import { agentCommand } from "../../commands/agent.js";
+import { agentCommandFromIngress } from "../../commands/agent.js";
 import type { OpenClawConfig } from "../../config/config.js";
+import { isDangerousNameMatchingEnabled } from "../../config/dangerous-name-matching.js";
 import type { DiscordAccountConfig, TtsConfig } from "../../config/types.js";
 import { logVerbose, shouldLogVerbose } from "../../globals.js";
 import { formatErrorMessage } from "../../infra/errors.js";
@@ -35,6 +36,8 @@ import { resolveAgentRoute } from "../../routing/resolve-route.js";
 import type { RuntimeEnv } from "../../runtime.js";
 import { parseTtsDirectives } from "../../tts/tts-core.js";
 import { resolveTtsConfig, textToSpeech, type ResolvedTtsConfig } from "../../tts/tts.js";
+import { resolveDiscordOwnerAccess } from "../monitor/allow-list.js";
+import { formatDiscordUserTag } from "../monitor/format.js";

 const require = createRequire(import.meta.url);

@@ -48,6 +51,7 @@ const SPEAKING_READY_TIMEOUT_MS = 60_000;
 const DECRYPT_FAILURE_WINDOW_MS = 30_000;
 const DECRYPT_FAILURE_RECONNECT_THRESHOLD = 3;
 const DECRYPT_FAILURE_PATTERN = /DecryptionFailed\(/;
+const SPEAKER_CONTEXT_CACHE_TTL_MS = 60_000;

 const logger = createSubsystemLogger("discord/voice");

@@ -275,6 +279,16 @@ export class DiscordVoiceManager {
  private botUserId?: string;
  private readonly voiceEnabled: boolean;
  private autoJoinTask: Promise<void> | null = null;
+  private readonly ownerAllowFrom: string[];
+  private readonly allowDangerousNameMatching: boolean;
+  private readonly speakerContextCache = new Map<
+    string,
+    {
+      label: string;
+      senderIsOwner: boolean;
+      expiresAt: number;
+    }
+  >();

  constructor(
    private params: {
@@ -288,6 +302,9 @@ export class DiscordVoiceManager {
  ) {
    this.botUserId = params.botUserId;
    this.voiceEnabled = params.discordConfig.voice?.enabled !== false;
+    this.ownerAllowFrom =
+      params.discordConfig.allowFrom ?? params.discordConfig.dm?.allowFrom ?? [];
+    this.allowDangerousNameMatching = isDangerousNameMatchingEnabled(params.discordConfig);
  }

  setBotUserId(id?: string) {
@@ -625,15 +642,16 @@ export class DiscordVoiceManager {
      `transcription ok (${transcript.length} chars): guild ${entry.guildId} channel ${entry.channelId}`,
    );

-    const speakerLabel = await this.resolveSpeakerLabel(entry.guildId, userId);
-    const prompt = speakerLabel ? `${speakerLabel}: ${transcript}` : transcript;
+    const speaker = await this.resolveSpeakerContext(entry.guildId, userId);
+    const prompt = speaker.label ? `${speaker.label}: ${transcript}` : transcript;

-    const result = await agentCommand(
+    const result = await agentCommandFromIngress(
      {
        message: prompt,
        sessionKey: entry.route.sessionKey,
        agentId: entry.route.agentId,
        messageChannel: "discord",
+        senderIsOwner: speaker.senderIsOwner,
        deliver: false,
      },
      this.params.runtime,
@@ -757,16 +775,113 @@ export class DiscordVoiceManager {
    }
  }

-  private async resolveSpeakerLabel(guildId: string, userId: string): Promise<string | undefined> {
+  private resolveSpeakerIsOwner(params: { id: string; name?: string; tag?: string }): boolean {
+    return resolveDiscordOwnerAccess({
+      allowFrom: this.ownerAllowFrom,
+      sender: {
+        id: params.id,
+        name: params.name,
+        tag: params.tag,
+      },
+      allowNameMatching: this.allowDangerousNameMatching,
+    }).ownerAllowed;
+  }
+
+  private resolveSpeakerContextCacheKey(guildId: string, userId: string): string {
+    return `${guildId}:${userId}`;
+  }
+
+  private getCachedSpeakerContext(
+    guildId: string,
+    userId: string,
+  ):
+    | {
+        label: string;
+        senderIsOwner: boolean;
+      }
+    | undefined {
+    const key = this.resolveSpeakerContextCacheKey(guildId, userId);
+    const cached = this.speakerContextCache.get(key);
+    if (!cached) {
+      return undefined;
+    }
+    if (cached.expiresAt <= Date.now()) {
+      this.speakerContextCache.delete(key);
+      return undefined;
+    }
+    return {
+      label: cached.label,
+      senderIsOwner: cached.senderIsOwner,
+    };
+  }
+
+  private setCachedSpeakerContext(
+    guildId: string,
+    userId: string,
+    context: { label: string; senderIsOwner: boolean },
+  ): void {
+    const key = this.resolveSpeakerContextCacheKey(guildId, userId);
+    this.speakerContextCache.set(key, {
+      label: context.label,
+      senderIsOwner: context.senderIsOwner,
+      expiresAt: Date.now() + SPEAKER_CONTEXT_CACHE_TTL_MS,
+    });
+  }
+
+  private async resolveSpeakerContext(
+    guildId: string,
+    userId: string,
+  ): Promise<{
+    label: string;
+    senderIsOwner: boolean;
+  }> {
+    const cached = this.getCachedSpeakerContext(guildId, userId);
+    if (cached) {
+      return cached;
+    }
+    const identity = await this.resolveSpeakerIdentity(guildId, userId);
+    const context = {
+      label: identity.label,
+      senderIsOwner: this.resolveSpeakerIsOwner({
+        id: identity.id,
+        name: identity.name,
+        tag: identity.tag,
+      }),
+    };
+    this.setCachedSpeakerContext(guildId, userId, context);
+    return context;
+  }
+
+  private async resolveSpeakerIdentity(
+    guildId: string,
+    userId: string,
+  ): Promise<{
+    id: string;
+    label: string;
+    name?: string;
+    tag?: string;
+  }> {
    try {
      const member = await this.params.client.fetchMember(guildId, userId);
-      return member.nickname ?? member.user?.globalName ?? member.user?.username ?? userId;
+      const username = member.user?.username ?? undefined;
+      return {
+        id: userId,
+        label: member.nickname ?? member.user?.globalName ?? username ?? userId,
+        name: username,
+        tag: member.user ? formatDiscordUserTag(member.user) : undefined,
+      };
    } catch {
      try {
        const user = await this.params.client.fetchUser(userId);
-        return user.globalName ?? user.username ?? userId;
+        const username = user.username ?? undefined;
+        return {
+          id: userId,
+          label: user.globalName ?? username ?? userId,
+          name: username,
+          tag: formatDiscordUserTag(user),
+        };
      } catch {
-        return userId;
+        return { id: userId, label: userId };
      }
    }
  }