From d9fd656a06b91adfcaab406f08235f2a2d445227 Mon Sep 17 00:00:00 2001 From: davefmurray <1911980+davefmurray@users.noreply.github.com> Date: Sat, 5 Sep 2026 04:58:39 -0400 Subject: [PATCH] fix: validate gateway group membership before mutations --- .changeset/quiet-groups-validate.md | 5 ++ src/gateway.js | 25 ++++++- test/gateway-groups.test.js | 107 ++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 .changeset/quiet-groups-validate.md create mode 100644 test/gateway-groups.test.js diff --git a/.changeset/quiet-groups-validate.md b/.changeset/quiet-groups-validate.md new file mode 100644 index 0000000..7087e51 --- /dev/null +++ b/.changeset/quiet-groups-validate.md @@ -0,0 +1,5 @@ +--- +"grok-bot-cli": patch +--- + +Validate group membership in gateway mode before sending mutations: deduplicate member references, enforce one to six bot members, reject nested groups, and reject bots as group targets. diff --git a/src/gateway.js b/src/gateway.js index bd0eae4..0321cb7 100644 --- a/src/gateway.js +++ b/src/gateway.js @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { ensureSandboxHeaders, headersFromEnsureSandbox, headersFromEnv, mergeGatewayHeaders, normalizeHeaderMap, requestHeaders } from "./headers.js"; import { hasGrokBotGatewaySession, loadGrokBotGatewaySession } from "./app-session.js"; -import { AVATAR_COLORS, AVATAR_SHAPES } from "./store.js"; +import { AVATAR_COLORS, AVATAR_SHAPES, MAX_GROUP_MEMBERS } from "./store.js"; export class GatewayError extends Error { constructor(message, { status, method } = {}) { @@ -228,9 +228,27 @@ export async function deleteAgent(session, ref) { return rec; } +function normalizeMemberIds(records, memberRefs) { + const memberIds = new Set(); + for (const ref of memberRefs) { + const rec = resolveFromList(records, ref); + if (rec.isGroup) { + throw new GatewayError(`Cannot add group "${rec.name}" as a member. Nested groups are not allowed.`); + } + memberIds.add(rec.id); + } + if (memberIds.size === 0) { + throw new GatewayError("A group needs at least one existing member agent."); + } + if (memberIds.size > MAX_GROUP_MEMBERS) { + throw new GatewayError(`A group can have at most ${MAX_GROUP_MEMBERS} members.`); + } + return [...memberIds]; +} + export async function createGroup(session, input) { const records = await listAgents(session); - const memberAgentIds = (input.memberIds || []).map((ref) => resolveFromList(records, ref).id); + const memberAgentIds = normalizeMemberIds(records, input.memberIds || []); const data = await gatewayCall(session, "createGroup", { name: input.name, description: input.description || "", @@ -248,7 +266,8 @@ export async function createGroup(session, input) { export async function setGroupMembers(session, groupRef, memberRefs) { const records = await listAgents(session); const group = resolveFromList(records, groupRef); - const memberAgentIds = memberRefs.map((ref) => resolveFromList(records, ref).id); + if (!group.isGroup) throw new GatewayError(`"${group.name}" is a bot, not a group.`); + const memberAgentIds = normalizeMemberIds(records, memberRefs); const data = await gatewayCall(session, "setGroupMembers", { id: group.id, memberAgentIds, diff --git a/test/gateway-groups.test.js b/test/gateway-groups.test.js new file mode 100644 index 0000000..3c9417a --- /dev/null +++ b/test/gateway-groups.test.js @@ -0,0 +1,107 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + addGroupMember, + createGroup, + removeGroupMember, + setGroupMembers, +} from "../src/gateway.js"; +import { MAX_GROUP_MEMBERS } from "../src/store.js"; + +const session = { gatewayUrl: "https://box.cursor.sh", gatewayToken: "test-token" }; +const bots = Array.from({ length: MAX_GROUP_MEMBERS + 1 }, (_, i) => ({ + id: `bot-${i + 1}`, + name: `Bot ${i + 1}`, + title: `Title ${i + 1}`, + isGroup: false, +})); +const group = { id: "group-1", name: "Launch", isGroup: true, memberIds: [bots[0].id] }; + +function mockGateway(t, currentGroup = group) { + const calls = []; + t.mock.method(globalThis, "fetch", async (url, options) => { + const method = new URL(url).pathname.split("/").pop(); + const body = JSON.parse(options.body); + calls.push({ method, body }); + const data = method === "listAgents" + ? { agents: [...bots, currentGroup] } + : { agent: { ...currentGroup, memberIds: body.memberAgentIds } }; + return new Response(JSON.stringify(data), { status: 200 }); + }); + return calls; +} + +for (const [name, run] of [ + ["create", (refs) => createGroup(session, { name: "New group", memberIds: refs })], + ["set", (refs) => setGroupMembers(session, group.name, refs)], +]) { + test(`${name} resolves and deduplicates member aliases before sending IDs`, async (t) => { + const calls = mockGateway(t); + const refs = bots.slice(0, MAX_GROUP_MEMBERS).flatMap((bot) => [ + bot.name, bot.id, bot.title.toUpperCase(), + ]); + const result = await run(refs); + const expected = bots.slice(0, MAX_GROUP_MEMBERS).map((bot) => bot.id); + assert.deepEqual(calls.at(-1).body.memberAgentIds, expected); + assert.deepEqual(result.memberIds, expected); + }); + + for (const [label, refs, error] of [ + ["empty membership", [], /at least one existing member/], + ["nested groups", [group.name], /Nested groups are not allowed/], + ["too many members", bots.map((bot) => bot.id), /at most 6 members/], + ["unknown members", ["Missing bot"], /No bot or group named/], + ]) { + test(`${name} rejects ${label} before any mutation`, async (t) => { + const calls = mockGateway(t); + await assert.rejects(run(refs), { name: "GatewayError", message: error }); + assert.ok(calls.length > 0); + assert.ok(calls.every((call) => call.method === "listAgents")); + }); + } +} + +for (const [name, run] of [ + ["set", () => setGroupMembers(session, bots[0].name, [bots[1].id])], + ["add", () => addGroupMember(session, bots[0].name, bots[1].id)], + ["remove", () => removeGroupMember(session, bots[0].name, bots[1].id)], +]) { + test(`${name} rejects a bot as the group target before any mutation`, async (t) => { + const calls = mockGateway(t); + await assert.rejects(run(), { name: "GatewayError", message: /is a bot, not a group/ }); + assert.ok(calls.every((call) => call.method === "listAgents")); + }); +} + +test("add rejects a nested group before any mutation", async (t) => { + const calls = mockGateway(t); + await assert.rejects(addGroupMember(session, group.id, group.id), /Nested groups/); + assert.ok(calls.every((call) => call.method === "listAgents")); +}); + +test("add rejects a seventh member before any mutation", async (t) => { + const calls = mockGateway(t, { + ...group, + memberIds: bots.slice(0, MAX_GROUP_MEMBERS).map((bot) => bot.id), + }); + await assert.rejects(addGroupMember(session, group.id, bots.at(-1).id), /at most 6 members/); + assert.ok(calls.every((call) => call.method === "listAgents")); +}); + +test("remove rejects removing the last member before any mutation", async (t) => { + const calls = mockGateway(t); + await assert.rejects(removeGroupMember(session, group.id, bots[0].id), /at least one/); + assert.ok(calls.every((call) => call.method === "listAgents")); +}); + +test("add and remove send valid updated membership", async (t) => { + const calls = mockGateway(t, { ...group, memberIds: [bots[0].id, bots[1].id] }); + const added = await addGroupMember(session, group.name, bots[2].name); + assert.deepEqual(added.memberIds, [bots[0].id, bots[1].id, bots[2].id]); + const removed = await removeGroupMember(session, group.name, bots[0].name); + assert.deepEqual(removed.memberIds, [bots[1].id]); + assert.deepEqual(calls.filter((call) => call.method === "setGroupMembers").map((call) => call.body), [ + { id: group.id, memberAgentIds: [bots[0].id, bots[1].id, bots[2].id] }, + { id: group.id, memberAgentIds: [bots[1].id] }, + ]); +});