From a94659cc65e334e4a83989cae66370f9afa05e1d Mon Sep 17 00:00:00 2001 From: Sivert <316918682+grytchat@users.noreply.github.com> Date: Tue, 3 Nov 2026 13:03:51 +0100 Subject: [PATCH] A known bot can ask for more permissions (GRYT-2006) A bot's list was fixed at its first knock, so a bot that needed more had to come back with a new key: a second bot under the same name, with none of the first one's history. Now an approved bot that declares permissions it never asked for still gets in with what it holds, and the new ones are stored as more_permissions for an admin. Nothing is granted until one answers. Approve grants what was ticked from the new ones on top of what it had; whatever isn't ticked goes into declined_permissions and isn't asked again. Declining leaves the bot approved with what it had. The reason the list was fixed still holds: a taken-over image can ask, but it can't give itself anything. The bot list carries "more" so the client can show the question. Co-Authored-By: Claude Opus 5.5 --- src/db/interfaces.ts | 4 +++ src/db/sqlite/bots.test.ts | 44 +++++++++++++++++++++++++++++++++ src/db/sqlite/bots.ts | 47 ++++++++++++++++++++++++++++++++++-- src/db/sqlite/connection.ts | 7 ++++++ src/socket/handlers/admin.ts | 2 ++ src/socket/handlers/join.ts | 15 +++++++++++- 6 files changed, 116 insertions(+), 3 deletions(-) diff --git a/src/db/interfaces.ts b/src/db/interfaces.ts index 95db1e9d..ef85d8e2 100644 --- a/src/db/interfaces.ts +++ b/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, diff --git a/src/db/sqlite/bots.test.ts b/src/db/sqlite/bots.test.ts index b83c17dc..dbde0cb1 100644 --- a/src/db/sqlite/bots.test.ts +++ b/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); + }); +}); diff --git a/src/db/sqlite/bots.ts b/src/db/sqlite/bots.ts index 2fd831af..c74a7454 100644 --- a/src/db/sqlite/bots.ts +++ b/src/db/sqlite/bots.ts @@ -55,6 +55,8 @@ function rowToBot(r: Record): 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 { + 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( diff --git a/src/db/sqlite/connection.ts b/src/db/sqlite/connection.ts index 057b2b9a..8079247c 100644 --- a/src/db/sqlite/connection.ts +++ b/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 '[]'"); diff --git a/src/socket/handlers/admin.ts b/src/socket/handlers/admin.ts index c2c9bad1..55306750 100644 --- a/src/socket/handlers/admin.ts +++ b/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. diff --git a/src/socket/handlers/join.ts b/src/socket/handlers/join.ts index b1c14c4b..ded9b2a5 100644 --- a/src/socket/handlers/join.ts +++ b/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.