diff --git a/CHANGES.md b/CHANGES.md index 1389e8f..9a956ee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,22 @@ Version 0.5.3 To be released. +### @fedify/botkit + + - Fixed multi-bot instances so their instance actor is discoverable through + WebFinger. Servers that dereference a signature's key owner through + WebFinger rather than by URI, such as GoToSocial, rejected every request + the instance actor signed, so follows from those servers never completed. + [[#45], [#46] by Les Orchard\] + - Reserved the instance actor's name against bot usernames as well as bot + identifiers. `Instance.createBot()` now throws a `TypeError` for a + username that matches the instance actor, which would otherwise have lost + its own WebFinger mapping. Single-bot instances have no instance actor, + so `createBot()` is unaffected. [[#45], [#46] by Les Orchard\] + +[#45]: https://github.com/fedify-dev/botkit/issues/45 +[#46]: https://github.com/fedify-dev/botkit/pull/46 + Version 0.5.2 ------------- diff --git a/changes.d/botkit/instance-actor-webfinger.md b/changes.d/botkit/instance-actor-webfinger.md new file mode 100644 index 0000000..1ec590c --- /dev/null +++ b/changes.d/botkit/instance-actor-webfinger.md @@ -0,0 +1,15 @@ +--- +links: + '#45': https://github.com/fedify-dev/botkit/issues/45 + '#46': https://github.com/fedify-dev/botkit/pull/46 +--- + - Fixed multi-bot instances so their instance actor is discoverable through + WebFinger. Servers that dereference a signature's key owner through + WebFinger rather than by URI, such as GoToSocial, rejected every request + the instance actor signed, so follows from those servers never completed. + [[#45], [#46] by Les Orchard] + - Reserved the instance actor's name against bot usernames as well as bot + identifiers. `Instance.createBot()` now throws a `TypeError` for a + username that matches the instance actor, which would otherwise have lost + its own WebFinger mapping. Single-bot instances have no instance actor, + so `createBot()` is unaffected. [[#45], [#46] by Les Orchard] diff --git a/packages/botkit/src/instance-impl.ts b/packages/botkit/src/instance-impl.ts index a1612e7..fa060c8 100644 --- a/packages/botkit/src/instance-impl.ts +++ b/packages/botkit/src/instance-impl.ts @@ -369,6 +369,18 @@ export class InstanceImpl // acct: resources and mentions vary in casing), so two usernames // differing only in case would be indistinguishable: const username = bot.username.toLowerCase(); + // A static bot holding the instance actor's name would leave the actor + // with no WebFinger record, which is the fault this reservation exists to + // prevent. Registration is synchronous, so the collision is rejected + // outright here rather than resolved by ordering in mapHandle(): + if ( + !this.compatMode && + username === this.instanceActorIdentifier.toLowerCase() + ) { + throw new TypeError( + `The username is reserved for the instance actor: ${bot.username}`, + ); + } for (const existing of this.#bots.values()) { if (existing.username.toLowerCase() === username) { throw new TypeError( @@ -535,6 +547,25 @@ export class InstanceImpl if (bot instanceof GroupBotImpl && bot.group.mapUsername == null) { return username; } + // The instance actor is served by the actor dispatcher but is not a + // registered bot, so it needs a mapping of its own. Without one it has + // no WebFinger record, and implementations that resolve a signature's + // key owner through WebFinger rather than by URI (GoToSocial) reject + // every request the instance actor signs. + // + // It resolves last so that a mapping something else already owns keeps + // working: addBot() reserves the name against static bots, but a group's + // mapUsername() can only be evaluated per request, and one that claims + // the name resolved to its own bot before this method knew about the + // instance actor at all. Deferring keeps that mapping rather than + // silently redirecting the handle away from a bot that is still + // dereferenceable under its own identifier: + if ( + !this.compatMode && + normalized === this.instanceActorIdentifier.toLowerCase() + ) { + return this.instanceActorIdentifier; + } return null; } diff --git a/packages/botkit/src/instance-multi.test.ts b/packages/botkit/src/instance-multi.test.ts index 43f2e55..16dcd7f 100644 --- a/packages/botkit/src/instance-multi.test.ts +++ b/packages/botkit/src/instance-multi.test.ts @@ -143,6 +143,84 @@ describe("instance actor", () => { assert.ok(actor.publicKey != null); }); + test("is discoverable through WebFinger", async () => { + // The instance actor signs the requests a multi-bot instance makes on + // its own behalf. Implementations that dereference a signature's key + // owner through WebFinger rather than by URI (GoToSocial) reject every + // one of those requests unless the actor resolves here: + async function webFingerSelf( + instance: InstanceWithVoidContextData, + username: string, + ): Promise { + const response = await instance.fetch( + new Request( + `https://example.com/.well-known/webfinger?resource=${ + encodeURIComponent(`acct:${username}@example.com`) + }`, + ), + ); + assert.deepStrictEqual(response.status, 200, `WebFinger ${username}`); + // Response.json() is typed as any, so the payload is narrowed rather + // than asserted into shape: + const jrd: unknown = await response.json(); + const links = typeof jrd === "object" && jrd != null && "links" in jrd + ? jrd.links + : undefined; + if (!Array.isArray(links)) { + assert.fail(`WebFinger ${username} returned no links array`); + } + for (const link of links) { + if ( + typeof link === "object" && link != null && + "rel" in link && link.rel === "self" && + "href" in link && typeof link.href === "string" + ) { + return link.href; + } + } + return undefined; + } + + const instance = createInstance({ kv: new MemoryKvStore() }); + instance.createBot("alpha", { username: "alphabot" }); + assert.deepStrictEqual( + await webFingerSelf(instance, DEFAULT_INSTANCE_ACTOR_IDENTIFIER), + `https://example.com/ap/actor/${DEFAULT_INSTANCE_ACTOR_IDENTIFIER}`, + ); + + // A renamed instance actor resolves under its new identifier, and the + // default one goes back to being an ordinary username: + const renamed = createInstance({ + kv: new MemoryKvStore(), + instanceActorIdentifier: "fetcher", + }); + renamed.createBot("alpha", { username: "alphabot" }); + assert.deepStrictEqual( + await webFingerSelf(renamed, "fetcher"), + "https://example.com/ap/actor/fetcher", + ); + + // A group's mapUsername() can only be evaluated per request, so it + // cannot be rejected at registration the way a static bot's username is. + // The instance actor therefore resolves last, leaving an explicit + // mapping that already worked on 0.5.2 pointing where it always did -- + // the alternative silently redirects the handle away from a bot that is + // still dereferenceable under its own identifier: + const dynamic = createInstance({ kv: new MemoryKvStore() }); + dynamic.createBot( + (_ctx, identifier) => + identifier === "lang_en" ? { username: "en" } : null, + { + mapUsername: (_ctx, username) => + username === DEFAULT_INSTANCE_ACTOR_IDENTIFIER ? "lang_en" : null, + }, + ); + assert.deepStrictEqual( + await webFingerSelf(dynamic, DEFAULT_INSTANCE_ACTOR_IDENTIFIER), + "https://example.com/ap/actor/lang_en", + ); + }); + test("signs the shared inbox on multi-bot instances", () => { const instance = createInstance({ kv: new MemoryKvStore() }); instance.createBot("alpha", { username: "alphabot" }); @@ -183,6 +261,43 @@ describe("instance actor", () => { assert.deepStrictEqual(actor.type, "Application"); }); + test("cannot have its name taken as a bot's username", () => { + // Only the identifier was reserved before. Since the instance actor now + // resolves ahead of the bots in mapHandle(), a bot holding its name would + // silently lose its own WebFinger mapping, so the username is reserved + // too -- case-insensitively, as WebFinger lookups vary in casing: + const instance = createInstance({ kv: new MemoryKvStore() }); + assert.throws( + () => + instance.createBot("sneaky", { + username: DEFAULT_INSTANCE_ACTOR_IDENTIFIER, + }), + TypeError, + ); + assert.throws( + () => + instance.createBot("sneaky", { + username: DEFAULT_INSTANCE_ACTOR_IDENTIFIER.toUpperCase(), + }), + TypeError, + ); + + // The reservation follows instanceActorIdentifier rather than the + // default name: a renamed actor reserves its own, and frees the default. + const renamed = createInstance({ + kv: new MemoryKvStore(), + instanceActorIdentifier: "fetcher", + }); + assert.throws( + () => renamed.createBot("sneaky", { username: "Fetcher" }), + TypeError, + ); + const bot = renamed.createBot("underscores", { + username: DEFAULT_INSTANCE_ACTOR_IDENTIFIER, + }); + assert.deepStrictEqual(bot.identifier, "underscores"); + }); + test("can be renamed through instanceActorIdentifier", async () => { const instance = createInstance({ kv: new MemoryKvStore(),