A known bot can ask for more permissions (GRYT-2006)

src/db/interfaces.ts

@@ -346,6 +346,10 @@ export interface BotRecord {
   updated_at: Date;
   decided_at: Date | null;
   decided_by_server_user_id: string | null;
+  /** What an approved bot has asked for since, waiting on an admin (GRYT-2006). */
+  more_permissions: Permission[];
+  /** Asked for later and turned down, so the next knock doesn't ask again. */
+  declined_permissions: Permission[];
 }
 
 /** `request` records a knock and admits nothing; `disabled` refuses at the door,

src/db/sqlite/bots.test.ts

@@ -14,6 +14,7 @@ import {
   deleteBotRegistration,
   getBotById,
   listBots,
+  recordBotAsksMore,
   recordBotKnock,
   updateBotGrant,
 } from "./bots";
@@ -226,3 +227,46 @@ describe("what an approved bot can actually do", () => {
     assert.deepEqual([...standing.permissions], []);
   });
 });
+
+describe("an approved bot that asks for more (GRYT-2006)", () => {
+  const BOT_D = "BOT_eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee";
+  const PENDING = "BOT_ffffffffffffffffffffffffffffffffffffffff";
+
+  async function approved() {
+    await recordBotKnock({ botId: BOT_D, nickname: "Gryt", requestedPermissions: ["read_messages", "send_messages"] });
+    await decideBot(BOT_D, "approved", "owner", ["read_messages", "send_messages"]);
+  }
+
+  it("is held for an admin, and grants nothing on its own", async () => {
+    await approved();
+    assert.equal(await recordBotAsksMore(BOT_D, ["read_messages", "set_activity", "add_reactions"]), true);
+    const bot = await getBotById(BOT_D);
+    assert.deepEqual(bot?.more_permissions.sort(), ["add_reactions", "set_activity"]);
+    assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages"]);
+    assert.equal(bot?.status, "approved");
+    assert.equal(await recordBotAsksMore(BOT_D, ["set_activity"]), false, "asking again is not news");
+  });
+
+  it("gets only what was ticked from the new ask, and keeps what it had", async () => {
+    await decideBot(BOT_D, "approved", "owner", ["read_messages", "send_messages", "set_activity", "kick_members"]);
+    const bot = await getBotById(BOT_D);
+    assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages", "set_activity"]);
+    assert.deepEqual(bot?.more_permissions, []);
+    assert.deepEqual(bot?.declined_permissions, ["add_reactions"]);
+    assert.equal(await recordBotAsksMore(BOT_D, ["add_reactions"]), false, "a no is not asked again");
+  });
+
+  it("stays in, with what it had, when the new ask is declined", async () => {
+    await recordBotAsksMore(BOT_D, ["view_members"]);
+    await decideBot(BOT_D, "denied", "owner");
+    const bot = await getBotById(BOT_D);
+    assert.equal(bot?.status, "approved");
+    assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages", "set_activity"]);
+    assert.ok(bot?.declined_permissions.includes("view_members"));
+  });
+
+  it("is not asked for a bot still waiting at the door", async () => {
+    await recordBotKnock({ botId: PENDING, nickname: "Waiting", requestedPermissions: ["read_messages"] });
+    assert.equal(await recordBotAsksMore(PENDING, ["kick_members"]), false);
+  });
+});

src/db/sqlite/bots.ts

@@ -55,6 +55,8 @@ function rowToBot(r: Record<string, unknown>): BotRecord {
     updated_at: fromIso(r.updated_at as string),
     decided_at: fromIsoNullable(r.decided_at as string | null),
     decided_by_server_user_id: (r.decided_by_server_user_id as string) ?? null,
+    more_permissions: parsePermissions(r.more_permissions),
+    declined_permissions: parsePermissions(r.declined_permissions),
   };
 }
 
@@ -115,6 +117,24 @@ export async function recordBotKnock(input: {
   return { bot, created: true };
 }
 
+/** An approved bot declaring permissions it never asked for. Nothing is granted here: it
+    is held for an admin, and whatever was turned down before is not asked again. */
+export async function recordBotAsksMore(botId: string, declared: Permission[]): Promise<boolean> {
+  const existing = await getBotById(botId);
+  if (!existing || existing.status !== "approved") return false;
+  const known = new Set([...existing.requested_permissions, ...existing.declined_permissions, ...existing.more_permissions]);
+  const fresh = normalizePermissions(declared).filter((p) => !known.has(p));
+  if (fresh.length === 0) return false;
+
+  const db = getSqliteDb();
+  db.prepare(`UPDATE bots SET more_permissions = ?, updated_at = ? WHERE bot_id = ?`).run(
+    JSON.stringify(normalizePermissions([...existing.more_permissions, ...fresh])),
+    toIso(new Date()),
+    botId,
+  );
+  return true;
+}
+
 /** The unattended path: an operator decides up front and hands over a token.
     Approved from the start, since the decision has already been made. */
 export async function createBotRegistration(input: {
@@ -186,14 +206,37 @@ export async function decideBot(
   const existing = await getBotById(botId);
   if (!existing) return null;
 
+  const db = getSqliteDb();
+  const now = toIso(new Date());
+
+  /* Answering an approved bot's later ask. Declining it must not turn the bot away:
+     it keeps what it had, and only the new ones it isn't given are remembered as no. */
+  if (existing.status === "approved" && existing.more_permissions.length > 0) {
+    const more = existing.more_permissions;
+    const allowed = new Set([...existing.requested_permissions, ...more]);
+    const wanted = decision === "approved" ? normalizePermissions(grantedPermissions).filter((p) => allowed.has(p)) : [];
+    const granted = decision === "approved" ? wanted : existing.granted_permissions;
+    const given = new Set(granted);
+    db.prepare(
+      `UPDATE bots SET requested_permissions = ?, granted_permissions = ?, more_permissions = '[]', declined_permissions = ?, decided_at = ?, decided_by_server_user_id = ?, updated_at = ? WHERE bot_id = ?`,
+    ).run(
+      JSON.stringify(normalizePermissions([...existing.requested_permissions, ...more.filter((p) => given.has(p))])),
+      JSON.stringify(normalizePermissions(granted)),
+      JSON.stringify(normalizePermissions([...existing.declined_permissions, ...more.filter((p) => !given.has(p))])),
+      now,
+      decidedByServerUserId,
+      now,
+      botId,
+    );
+    return getBotById(botId);
+  }
+
   const asked = new Set(existing.requested_permissions);
   const granted =
     decision === "approved"
       ? normalizePermissions(grantedPermissions).filter((p) => asked.has(p))
       : [];
 
-  const db = getSqliteDb();
-  const now = toIso(new Date());
   db.prepare(
     `UPDATE bots SET status = ?, granted_permissions = ?, rank = ?, decided_at = ?, decided_by_server_user_id = ?, updated_at = ? WHERE bot_id = ?`,
   ).run(

src/db/sqlite/connection.ts

@@ -754,6 +754,13 @@ function hasColumn(d: DatabaseSync, table: string, column: string): boolean {
 
 function runMigrations(d: DatabaseSync): void {
   ensureSearchIndex(d);
+  // A known bot asking for more, and what was turned down (GRYT-2006).
+  if (!hasColumn(d, "bots", "more_permissions")) {
+    d.exec("ALTER TABLE bots ADD COLUMN more_permissions TEXT NOT NULL DEFAULT '[]'");
+  }
+  if (!hasColumn(d, "bots", "declined_permissions")) {
+    d.exec("ALTER TABLE bots ADD COLUMN declined_permissions TEXT NOT NULL DEFAULT '[]'");
+  }
   // Conversations muted on that phone (GRYT-1689). Only dev servers ever had the table without it.
   if (!hasColumn(d, "push_devices", "muted")) {
     d.exec("ALTER TABLE push_devices ADD COLUMN muted TEXT NOT NULL DEFAULT '[]'");

src/socket/handlers/admin.ts

@@ -170,6 +170,8 @@ async function botsView() {
       description: b.description,
       requested: b.requested_permissions,
       granted: b.granted_permissions,
+      // Asked for since it was approved, and not yet answered (GRYT-2006).
+      more: b.more_permissions,
       rank: b.rank,
       status: b.status,
       // Whether it is still waiting for a bot to turn up and claim it.

src/socket/handlers/join.ts

@@ -37,6 +37,7 @@ import {
   clearJoinRequest,
   getBotById,
   claimBotRegistration,
+  recordBotAsksMore,
   recordBotKnock,
   insertServerAudit,
 } from "../../db";
@@ -115,7 +116,19 @@ async function admitBot(
   const existing = await getBotById(botId);
 
   if (existing) {
-    if (existing.status === "approved") return { ok: true, bot: existing };
+    if (existing.status === "approved") {
+      // Let in on what it already holds. Anything new it declares waits for an admin (GRYT-2006).
+      if (await recordBotAsksMore(botId, normalizePermissions(declaration?.permissions ?? []))) {
+        insertServerAudit({
+          actorServerUserId: null,
+          action: "bot_asked_more",
+          target: botId,
+          meta: { nickname: existing.nickname, asked: (await getBotById(botId))?.more_permissions ?? [] },
+        }).catch((e: unknown) => consola.warn("audit log write failed", e));
+        broadcastServerUiUpdate();
+      }
+      return { ok: true, bot: existing };
+    }
     if (existing.status === "denied") {
       // Told the same thing as a pending bot: confirming somebody looked and
       // said no invites an argument with the message.