Make the instance actor discoverable through WebFinger

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
 -------------

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]

packages/botkit/src/instance-impl.ts

@@ -369,6 +369,18 @@ export class InstanceImpl<TContextData>
     // 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<TContextData>
     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;
   }
 

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<string | undefined> {
+      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<void>({ 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<void>({
+      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<void>({ 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<void>({ 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<void>({ 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<void>({
+      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<void>({
       kv: new MemoryKvStore(),