fix(bot): resolve sender server groups live + server-wide for the admin-command gate

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/bot/instance.test.ts

@@ -117,14 +117,18 @@ describe("BotInstance.runExclusive — serialization", () => {
  *  `this.isCommandAllowed(...)` resolve against this same object. */
 function makeGateCtx(opts: {
   adminGroups?: number[];
-  clients?: Array<{ id: number; serverGroups: string[] }>;
+  lookupGroups?: string[];
+  lookupThrows?: boolean;
 }) {
   const ctx: any = {
     config: { commandPrefix: "!", commandAliases: {}, adminGroups: opts.adminGroups ?? [] },
     logger: { info: vi.fn(), error: vi.fn() },
     tsClient: {
       sendTextMessage: vi.fn(async () => {}),
-      getClientsInChannel: vi.fn(async () => opts.clients ?? []),
+      getClientServerGroups: vi.fn(async () => {
+        if (opts.lookupThrows) throw new Error("query failed");
+        return opts.lookupGroups ?? [];
+      }),
     },
     executeCommand: vi.fn(async () => null),
     isCommandAllowed: (BotInstance.prototype as any).isCommandAllowed,
@@ -143,52 +147,66 @@ const handleTextMessage = (BotInstance.prototype as any).handleTextMessage as (
 ) => Promise<void>;
 
 describe("BotInstance.handleTextMessage — command permission gate", () => {
-  it("runs a public command even with enforcement on", async () => {
+  it("runs a public command with no group lookup, even under enforcement", async () => {
     const ctx = makeGateCtx({ adminGroups: [6] });
-    await handleTextMessage.call(ctx, makeMsg("!play 晴天"));
+    await handleTextMessage.call(ctx, makeMsg("!play 晴天", ["6"]));
     expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
+    expect(ctx.tsClient.getClientServerGroups).not.toHaveBeenCalled();
     expect(ctx.tsClient.sendTextMessage).not.toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
   });
 
-  it("runs an admin command when enforcement is off (empty adminGroups)", async () => {
+  it("runs an admin command with no lookup when enforcement is off", async () => {
     const ctx = makeGateCtx({ adminGroups: [] });
     await handleTextMessage.call(ctx, makeMsg("!stop"));
     expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
+    expect(ctx.tsClient.getClientServerGroups).not.toHaveBeenCalled();
   });
 
-  it("runs an admin command when the event carried a matching group", async () => {
+  it("allows an enforced admin command when the live lookup returns a matching group", async () => {
-    const ctx = makeGateCtx({ adminGroups: [6] });
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["6"] });
+    await handleTextMessage.call(ctx, makeMsg("!stop"));
+    expect(ctx.tsClient.getClientServerGroups).toHaveBeenCalledTimes(1);
+    expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
+  });
+
+  it("denies an enforced admin command when the live lookup has no matching group", async () => {
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["8"] });
+    await handleTextMessage.call(ctx, makeMsg("!stop"));
+    expect(ctx.executeCommand).not.toHaveBeenCalled();
+    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
+  });
+
+  it("fails closed when the live lookup returns no groups", async () => {
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: [] });
+    await handleTextMessage.call(ctx, makeMsg("!stop"));
+    expect(ctx.executeCommand).not.toHaveBeenCalled();
+    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
+  });
+
+  it("fails closed when the live lookup throws", async () => {
+    const ctx = makeGateCtx({ adminGroups: [6], lookupThrows: true });
+    await handleTextMessage.call(ctx, makeMsg("!stop"));
+    expect(ctx.executeCommand).not.toHaveBeenCalled();
+    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
+  });
+
+  it("ignores stale event groups: a demoted sender (cached match) is denied by the live lookup", async () => {
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["8"] });
     await handleTextMessage.call(ctx, makeMsg("!stop", ["6"]));
-    expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
+    expect(ctx.executeCommand).not.toHaveBeenCalled();
-    expect(ctx.tsClient.getClientsInChannel).not.toHaveBeenCalled(); // no fallback needed
+    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
   });
 
-  it("denies an admin command when known groups do not match (no fallback, with reply)", async () => {
+  it("uses live groups, not stale event groups: a freshly-promoted sender is allowed", async () => {
-    const ctx = makeGateCtx({ adminGroups: [6] });
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["6"] });
     await handleTextMessage.call(ctx, makeMsg("!stop", ["8"]));
-    expect(ctx.executeCommand).not.toHaveBeenCalled();
-    expect(ctx.tsClient.getClientsInChannel).not.toHaveBeenCalled();
-    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
-  });
-
-  it("falls back to a group lookup when the event carried no groups, and allows on match", async () => {
-    const ctx = makeGateCtx({ adminGroups: [6], clients: [{ id: 5, serverGroups: ["6"] }] });
-    await handleTextMessage.call(ctx, makeMsg("!stop", [], "5"));
-    expect(ctx.tsClient.getClientsInChannel).toHaveBeenCalledTimes(1);
     expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
   });
 
-  it("fails closed when the fallback finds the client but no matching group", async () => {
+  it("resolves out-of-channel senders server-wide: empty event groups but a matching live group → allowed", async () => {
-    const ctx = makeGateCtx({ adminGroups: [6], clients: [{ id: 5, serverGroups: ["8"] }] });
+    const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["6"] });
     await handleTextMessage.call(ctx, makeMsg("!stop", [], "5"));
-    expect(ctx.executeCommand).not.toHaveBeenCalled();
+    expect(ctx.tsClient.getClientServerGroups).toHaveBeenCalledTimes(1);
-    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
+    expect(ctx.executeCommand).toHaveBeenCalledTimes(1);
-  });
-
-  it("fails closed when the fallback cannot find the client at all", async () => {
-    const ctx = makeGateCtx({ adminGroups: [6], clients: [] });
-    await handleTextMessage.call(ctx, makeMsg("!stop", [], "5"));
-    expect(ctx.executeCommand).not.toHaveBeenCalled();
-    expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE);
   });
 });

src/bot/instance.ts

@@ -362,35 +362,34 @@ export class BotInstance extends EventEmitter {
 
   /**
    * Decide whether a chat command may run for this sender. Reads adminGroups
-   * live from this.config (the router mutates the same object). Only performs
+   * live from this.config. Public commands and the enforcement-off case are
-   * the async group lookup when the synchronous decision is "deny because the
+   * allowed with NO query. For an ENFORCED admin command we resolve the
-   * event carried no groups" — i.e. an admin command, enforcement on, and
+   * sender's CURRENT server groups with a targeted server-wide lookup rather
-   * empty invokerGroups. Fails closed if groups remain undeterminable.
+   * than trusting the text event's cached groups — those are empty for
+   * out-of-channel senders and stale after a live promotion/demotion. Fails
+   * closed when the groups can't be determined.
    */
   private async isCommandAllowed(commandName: string, msg: TS3TextMessage): Promise<boolean> {
     const adminGroups = this.config.adminGroups;
-    if (canRunCommand(commandName, msg.invokerGroups, adminGroups)) return true;
+    // Public command, or enforcement off → allow without any lookup.
-    // Here: admin command, enforcement on, and the provided groups did not match.
+    // (canRunCommand with empty groups is true iff the command is public OR
-    // If the event actually carried groups, this is a genuine deny — no lookup.
+    // adminGroups is empty.)
-    if (msg.invokerGroups.length > 0) return false;
+    if (canRunCommand(commandName, [], adminGroups)) return true;
-    // Groups unknown (sender not in the view cache): one targeted lookup, then
+    // Enforced admin command: authoritative decision uses freshly-resolved,
-    // re-decide. canRunCommand([], …) is false ⇒ fail-closed when still unknown.
+    // server-wide groups. Fail closed if they can't be determined.
     const groups = await this.lookupInvokerGroups(msg.invokerId);
     return canRunCommand(commandName, groups, adminGroups);
   }
 
   /**
-   * Best-effort lookup of a sender's server groups by client id, via the
+   * Resolve the sender's current server groups by client id, server-wide.
-   * channel client list (whose entries already carry parsed serverGroups).
+   * Returns [] on a bad id or query failure (→ fail-closed deny upstream).
-   * Returns [] when the client can't be found or the query fails (→ deny).
    */
   private async lookupInvokerGroups(invokerId: string): Promise<string[]> {
     const clid = Number(invokerId);
     if (!Number.isFinite(clid) || clid <= 0) return [];
     try {
-      const clients = await this.tsClient.getClientsInChannel();
+      return await this.tsClient.getClientServerGroups(clid);
-      const match = clients.find((c) => c.id === clid);
-      return match?.serverGroups ?? [];
     } catch {
       return [];
     }

src/ts-protocol/client.ts

@@ -8,6 +8,7 @@ import {
   listChannels,
   listClients,
   clientMove,
+  getClientInfo,
   fileTransferDeleteFile,
   type Identity,
   type TextMessage,
@@ -333,6 +334,26 @@ export class TS3Client extends EventEmitter {
     }
   }
 
+  /**
+   * Resolve a client's CURRENT server groups by client id, server-wide (works
+   * regardless of channel/view) via a targeted `clientinfo` query. The raw
+   * `client_servergroups` field is a comma-separated list (same field
+   * `listClients` parses). Returns [] if the client can't be resolved or the
+   * query fails, so callers fail closed.
+   */
+  async getClientServerGroups(clid: number): Promise<string[]> {
+    if (!this.client) return [];
+    try {
+      const info = await getClientInfo(this.client, clid);
+      // `client_servergroups`: comma-separated server-group ids (verified in
+      // @honeybbq/teamspeak-client dist/index.mjs; listClients parses the same).
+      const raw = info.client_servergroups ?? "";
+      return raw ? raw.split(",") : [];
+    } catch {
+      return [];
+    }
+  }
+
   // --- Raw command & file transfer pass-through ---
 
   async execCommand(cmd: string): Promise<void> {