diff --git a/src/bot/instance.test.ts b/src/bot/instance.test.ts index 1e0df26..e745258 100644 --- a/src/bot/instance.test.ts +++ b/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; 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 () => { - const ctx = makeGateCtx({ adminGroups: [6] }); + it("allows an enforced admin command when the live lookup returns a matching group", async () => { + 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.tsClient.getClientsInChannel).not.toHaveBeenCalled(); // no fallback needed + expect(ctx.executeCommand).not.toHaveBeenCalled(); + expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE); }); - it("denies an admin command when known groups do not match (no fallback, with reply)", async () => { - const ctx = makeGateCtx({ adminGroups: [6] }); + it("uses live groups, not stale event groups: a freshly-promoted sender is allowed", async () => { + 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 () => { - const ctx = makeGateCtx({ adminGroups: [6], clients: [{ id: 5, serverGroups: ["8"] }] }); + it("resolves out-of-channel senders server-wide: empty event groups but a matching live group → allowed", async () => { + const ctx = makeGateCtx({ adminGroups: [6], lookupGroups: ["6"] }); await handleTextMessage.call(ctx, makeMsg("!stop", [], "5")); - expect(ctx.executeCommand).not.toHaveBeenCalled(); - expect(ctx.tsClient.sendTextMessage).toHaveBeenCalledWith(COMMAND_DENIED_MESSAGE); - }); - - 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); + expect(ctx.tsClient.getClientServerGroups).toHaveBeenCalledTimes(1); + expect(ctx.executeCommand).toHaveBeenCalledTimes(1); }); }); diff --git a/src/bot/instance.ts b/src/bot/instance.ts index 263beab..af3867f 100755 --- a/src/bot/instance.ts +++ b/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 - * the async group lookup when the synchronous decision is "deny because the - * event carried no groups" — i.e. an admin command, enforcement on, and - * empty invokerGroups. Fails closed if groups remain undeterminable. + * live from this.config. Public commands and the enforcement-off case are + * allowed with NO query. For an ENFORCED admin command we resolve the + * sender's CURRENT server groups with a targeted server-wide lookup rather + * 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 { const adminGroups = this.config.adminGroups; - if (canRunCommand(commandName, msg.invokerGroups, adminGroups)) return true; - // Here: admin command, enforcement on, and the provided groups did not match. - // If the event actually carried groups, this is a genuine deny — no lookup. - if (msg.invokerGroups.length > 0) return false; - // Groups unknown (sender not in the view cache): one targeted lookup, then - // re-decide. canRunCommand([], …) is false ⇒ fail-closed when still unknown. + // Public command, or enforcement off → allow without any lookup. + // (canRunCommand with empty groups is true iff the command is public OR + // adminGroups is empty.) + if (canRunCommand(commandName, [], adminGroups)) return true; + // Enforced admin command: authoritative decision uses freshly-resolved, + // 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 - * channel client list (whose entries already carry parsed serverGroups). - * Returns [] when the client can't be found or the query fails (→ deny). + * Resolve the sender's current server groups by client id, server-wide. + * Returns [] on a bad id or query failure (→ fail-closed deny upstream). */ private async lookupInvokerGroups(invokerId: string): Promise { const clid = Number(invokerId); if (!Number.isFinite(clid) || clid <= 0) return []; try { - const clients = await this.tsClient.getClientsInChannel(); - const match = clients.find((c) => c.id === clid); - return match?.serverGroups ?? []; + return await this.tsClient.getClientServerGroups(clid); } catch { return []; } diff --git a/src/ts-protocol/client.ts b/src/ts-protocol/client.ts index 31b317e..d9697c8 100644 --- a/src/ts-protocol/client.ts +++ b/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 { + 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 {