Ask whether the person may act as the Bot they named

server/src/agents/profile-policy.ts

@@ -23,3 +23,17 @@ export function canManageAgent(
 }
 
 export const canRunAgent = canAccessAgent;
+
+/**
+ * Whether this person may act as this Bot.
+ *
+ * Injected rather than imported, so a surface that acts as a Bot depends on the question and not on
+ * the agents table. It also keeps the answer in one place: the store's read path already filters on
+ * {@link canAccessAgent}, so asking it is the same rule the roster and the runtime already apply,
+ * rather than a second copy that can drift from them.
+ */
+export type BotAccessCheck = (
+  /** The whole actor, not just the id: an administrator reaches every Bot, and a role tells us. */
+  actor: AgentActor,
+  botId: string,
+) => Promise<boolean>;

server/src/app.ts

@@ -1,5 +1,6 @@
 import type { Hono as HonoApp, MiddlewareHandler } from "hono";
 import { Hono } from "hono";
+import type { BotAccessCheck } from "./agents/profile-policy";
 import type { AgentProfileStore } from "./agents/profile-store";
 import { createAgentRoutes } from "./agents/routes";
 import { type AuditReader, type AuditStore, auditQueryFromUrl } from "./audit";
@@ -295,6 +296,21 @@ export function createApp(
     app.route("/", copilotHandler);
   }
 
+  /**
+   * May this person act as this Bot?
+   *
+   * The store's own read path already applies `canAccessAgent`, so asking it for the Bot is the same
+   * question the roster and the runtime ask, rather than a second copy of the rule.
+   *
+   * A deployment with no profile store has no agents table and therefore no private Bot to protect:
+   * its Bots come from the tenant package and are public to everybody who can sign in. Answering yes
+   * there keeps that deployment working without weakening one that has owners.
+   */
+  const canUseBot: BotAccessCheck = agentProfileStore
+    ? async (actor, botId) =>
+        (await agentProfileStore.get(actor, botId)) !== null
+    : async () => true;
+
   // The Bot computer. Acting on a page needs the gateway and the policy it enforces, so all
   // three arrive together or the routes are not mounted: a computer whose actions were ungoverned is
   // not a reduced feature, it is the one shape of this feature that must not exist.
@@ -306,6 +322,7 @@ export function createApp(
         computerGateway,
         computerPolicy,
         requireUser,
+        canUseBot,
       ),
     );
   }
@@ -336,12 +353,15 @@ export function createApp(
   if (componentStore) {
     app.route(
       "/api/components",
-      createComponentRoutes(componentStore, requireUser, auditStore),
+      createComponentRoutes(componentStore, requireUser, auditStore, canUseBot),
     );
   }
 
   if (pluginStore) {
-    app.route("/api/plugins", createPluginRoutes(pluginStore, requireUser));
+    app.route(
+      "/api/plugins",
+      createPluginRoutes(pluginStore, requireUser, canUseBot),
+    );
   }
 
   if (sandboxedStore) {

server/src/components/routes.ts

@@ -1,5 +1,6 @@
 import type { MiddlewareHandler } from "hono";
 import { Hono } from "hono";
+import type { BotAccessCheck } from "../agents/profile-policy";
 import type { AuditStore } from "../audit";
 import { recordAuditEvent } from "../audit";
 import type { AppVariables } from "../auth/guards";
@@ -31,7 +32,13 @@ const DEV_ACTOR_EMAIL = "dev@openbot.local";
 export function createComponentRoutes(
   store: ComponentStore,
   requireUser: MiddlewareHandler<{ Variables: AppVariables }>,
-  auditStore?: AuditStore,
+  auditStore: AuditStore | undefined,
+  /**
+   * Whether the caller may act as the Bot they named. What a Bot may draw, and the data a drawing
+   * reads, are facts about that Bot; an administrator granting one is a separate question and stays
+   * behind `requireAdmin`.
+   */
+  canUseBot: BotAccessCheck,
 ) {
   const routes = new Hono<{ Variables: AppVariables }>();
 
@@ -116,11 +123,13 @@ export function createComponentRoutes(
    * Deliberately says nothing about the components this Bot does NOT hold. A list of everything it
    * is missing would be a list the surface could accidentally register.
    */
-  routes.get("/for-agent/:agentId", requireUser, async (context) =>
-    context.json({
-      components: await store.listForAgent(context.req.param("agentId")),
-    }),
-  );
+  routes.get("/for-agent/:agentId", requireUser, async (context) => {
+    const agentId = context.req.param("agentId");
+    if (!(await canUseBot(context.var.actor, agentId))) {
+      return context.json({ error: "There is no such Bot." }, 404);
+    }
+    return context.json({ components: await store.listForAgent(agentId) });
+  });
 
   /**
    * May this Bot use this component, right now?
@@ -140,6 +149,11 @@ export function createComponentRoutes(
     if (!agentId) {
       return context.json({ error: "The Bot is required." }, 400);
     }
+    // Asked before the grant is, because the grant belongs to the Bot and says nothing about who is
+    // asking on its behalf.
+    if (!(await canUseBot(context.var.actor, agentId))) {
+      return context.json({ error: "There is no such Bot." }, 404);
+    }
     const functions = Array.isArray(body?.functions)
       ? body.functions.filter(
           (entry): entry is string => typeof entry === "string",
@@ -213,6 +227,11 @@ export function createComponentRoutes(
         400,
       );
     }
+    // Before the grant, and before anything runs. This is the route that executes, so borrowing a
+    // Bot here borrows whatever its components were granted.
+    if (!(await canUseBot(context.var.actor, agentId))) {
+      return context.json({ error: "There is no such Bot." }, 404);
+    }
 
     const refuse = async (reason: string) => {
       await audit(context, "component.function_refused", name, {
@@ -325,7 +344,6 @@ export function createComponentRoutes(
     if (!agentId) {
       return context.json({ error: "The Bot is required." }, 400);
     }
-
     try {
       await store.grant(name, agentId);
     } catch (error) {

server/src/computer/routes.ts

@@ -1,5 +1,6 @@
 import type { Context, MiddlewareHandler } from "hono";
 import { Hono } from "hono";
+import type { BotAccessCheck } from "../agents/profile-policy";
 import type { AppVariables } from "../auth/guards";
 import { requireAdmin } from "../auth/guards";
 import {
@@ -34,14 +35,37 @@ export function createComputerRoutes(
   gateway: ComputerGateway,
   policyStore: PolicyStore,
   requireUser: MiddlewareHandler<{ Variables: AppVariables }>,
+  /**
+   * Whether the caller may act as the Bot in the path. Required rather than optional, so a new
+   * deployment cannot be wired up without an answer to it.
+   */
+  canUseBot: BotAccessCheck,
 ) {
   const routes = new Hono<{ Variables: AppVariables }>();
 
-  routes.get("/:botId/status", requireUser, async (context) =>
+  /**
+   * Every route under a Bot id, in one place.
+   *
+   * The Bot travels in the path, so each route would otherwise have to remember to ask, and the one
+   * that forgot would be the whole surface. Reads are gated as well as actions: a screenshot of
+   * somebody's Bot is whatever page it is signed into.
+   *
+   * The answer is the same for a Bot that does not exist and one belonging to somebody else, so this
+   * cannot be used to find out which Bots a deployment has.
+   */
+  routes.use("/:botId/*", requireUser, async (context, next) => {
+    const botId = context.req.param("botId");
+    if (botId && !(await canUseBot(context.var.actor, botId))) {
+      return context.json({ error: "There is no such Bot." }, 404);
+    }
+    await next();
+  });
+
+  routes.get("/:botId/status", async (context) =>
     context.json(await client.status(context.req.param("botId"))),
   );
 
-  routes.get("/:botId/screenshot", requireUser, async (context) => {
+  routes.get("/:botId/screenshot", async (context) => {
     try {
       return context.json(
         await client.forBot(context.req.param("botId")).screenshot(),
@@ -51,15 +75,15 @@ export function createComputerRoutes(
     }
   });
 
-  routes.get("/:botId/read", requireUser, async (context) => {
+  routes.get("/:botId/read", async (context) => {
     try {
       return context.json(await gateway.read(context.req.param("botId")));
     } catch (error) {
       return context.json({ error: describe(error) }, statusFor(error));
     }
   });
 
-  routes.post("/:botId/navigate", requireUser, async (context) => {
+  routes.post("/:botId/navigate", async (context) => {
     const body = (await context.req.json().catch(() => null)) as {
       url?: unknown;
     } | null;
@@ -95,7 +119,7 @@ export function createComputerRoutes(
     }
   });
 
-  routes.post("/:botId/snapshot", requireUser, async (context) => {
+  routes.post("/:botId/snapshot", async (context) => {
     try {
       return context.json(await gateway.snapshot(context.req.param("botId")));
     } catch (error) {
@@ -109,15 +133,15 @@ export function createComputerRoutes(
    * Each one hands the gateway the computer id, the Bot, the actor and the input, and does no checking
    * of its own beyond the shape of the request. Where a decision gets made is a single place.
    */
-  routes.post("/:botId/click", requireUser, (context) =>
+  routes.post("/:botId/click", (context) =>
     act(context, (botId, actor, body, signal) => {
       const ref = asRef(body);
       if (!ref) return badRef;
       return gateway.click(botId, botId, actor, ref, signal);
     }),
   );
 
-  routes.post("/:botId/type", requireUser, (context) =>
+  routes.post("/:botId/type", (context) =>
     act(context, (botId, actor, body, signal) => {
       const ref = asRef(body);
       if (!ref) return badRef;
@@ -138,7 +162,7 @@ export function createComputerRoutes(
     }),
   );
 
-  routes.post("/:botId/key", requireUser, (context) =>
+  routes.post("/:botId/key", (context) =>
     act(context, (botId, actor, body, signal) => {
       if (typeof body?.key !== "string" || !body.key) {
         return { error: "A key name is required, such as Enter or Tab." };
@@ -157,7 +181,7 @@ export function createComputerRoutes(
     }),
   );
 
-  routes.post("/:botId/scroll", requireUser, (context) =>
+  routes.post("/:botId/scroll", (context) =>
     act(context, (botId, actor, body) =>
       gateway.scroll(botId, botId, actor, {
         ...(typeof body?.deltaY === "number" ? { deltaY: body.deltaY } : {}),
@@ -169,15 +193,15 @@ export function createComputerRoutes(
    * Who has the wheel. Polled by the surface next to the screen, so the person sees the Bot ask for
    * help without reloading anything.
    */
-  routes.get("/:botId/control", requireUser, async (context) => {
+  routes.get("/:botId/control", async (context) => {
     try {
       return context.json(await gateway.control(context.req.param("botId")));
     } catch (error) {
       return context.json({ error: describe(error) }, statusFor(error));
     }
   });
 
-  routes.post("/:botId/control/request", requireUser, (context) =>
+  routes.post("/:botId/control/request", (context) =>
     act(context, (botId, actor, body) =>
       gateway.requestHelp(
         botId,
@@ -197,7 +221,7 @@ export function createComputerRoutes(
    * it holds a list. `:botId` is still there because every route under this router has it and the
    * gateway wants somebody to attribute the call to.
    */
-  routes.get("/:botId/computers", requireUser, async (context) => {
+  routes.get("/:botId/computers", async (context) => {
     try {
       return context.json(await gateway.computers());
     } catch (error) {
@@ -206,25 +230,25 @@ export function createComputerRoutes(
   });
 
   /** Stop the browser, keep the logins. */
-  routes.post("/:botId/computers/stop", requireUser, (context) =>
+  routes.post("/:botId/computers/stop", (context) =>
     act(context, (botId, actor) => gateway.stopComputer(botId, botId, actor)),
   );
 
   /** Delete the profile. Every login goes with it, which is the point and also the danger. */
-  routes.post("/:botId/computers/reset", requireUser, (context) =>
+  routes.post("/:botId/computers/reset", (context) =>
     act(context, (botId, actor) => gateway.resetComputer(botId, botId, actor)),
   );
 
-  routes.post("/:botId/control/take", requireUser, (context) =>
+  routes.post("/:botId/control/take", (context) =>
     act(context, (botId, actor) => gateway.takeControl(botId, botId, actor)),
   );
 
-  routes.post("/:botId/control/release", requireUser, (context) =>
+  routes.post("/:botId/control/release", (context) =>
     act(context, (botId, actor) => gateway.releaseControl(botId, botId, actor)),
   );
 
   /** The Bot asking for a value it must not be told. */
-  routes.post("/:botId/control/secret", requireUser, (context) =>
+  routes.post("/:botId/control/secret", (context) =>
     act(context, (botId, actor, body) => {
       if (typeof body?.ref !== "string" || !body.ref) {
         return {
@@ -253,7 +277,7 @@ export function createComputerRoutes(
    * route rather than a `kind` on the input route below, so that grepping for where a secret can enter
    * this server returns exactly one place.
    */
-  routes.post("/:botId/human/secret", requireUser, (context) =>
+  routes.post("/:botId/human/secret", (context) =>
     act(context, (botId, actor, body) => {
       if (typeof body?.text !== "string" || !body.text) {
         return { error: "A value is required." };
@@ -270,7 +294,7 @@ export function createComputerRoutes(
    * unrecorded, because the reason a takeover exists is to let them enter the thing nothing else
    * should keep.
    */
-  routes.post("/:botId/human/:kind", requireUser, async (context) => {
+  routes.post("/:botId/human/:kind", async (context) => {
     const kind = context.req.param("kind");
     if (
       kind !== "click" &&
@@ -297,7 +321,7 @@ export function createComputerRoutes(
   });
 
   /** The Bot's files. Through the gateway, like every other acting call. */
-  routes.post("/:botId/files/list", requireUser, (context) =>
+  routes.post("/:botId/files/list", (context) =>
     act(context, (botId, actor, body) =>
       gateway.listFiles(botId, botId, actor, {
         ...(typeof body?.path === "string" && body.path.trim()
@@ -307,7 +331,7 @@ export function createComputerRoutes(
     ),
   );
 
-  routes.post("/:botId/files/read", requireUser, (context) =>
+  routes.post("/:botId/files/read", (context) =>
     act(context, (botId, actor, body) => {
       if (typeof body?.path !== "string" || !body.path.trim()) {
         return { error: "A file path is required." };
@@ -316,7 +340,7 @@ export function createComputerRoutes(
     }),
   );
 
-  routes.post("/:botId/files/write", requireUser, (context) =>
+  routes.post("/:botId/files/write", (context) =>
     act(context, (botId, actor, body) => {
       if (typeof body?.path !== "string" || !body.path.trim()) {
         return { error: "A file path is required." };

server/src/index.ts

@@ -12,9 +12,9 @@ import {
   startChannelActivityListener,
 } from "./channels/events";
 import { createChannelStore } from "./channels/routes";
+import { websocket as channelSocket } from "./channels/socket";
 import { createStallGuard } from "./channels/stall-guard";
 import { createThreadIdentity } from "./channels/thread-identity";
-import { websocket as channelSocket } from "./channels/socket";
 import { createSandboxedStore } from "./components/sandboxed";
 import { createComponentStore } from "./components/store";
 import { createComputerClient } from "./computer/client";
@@ -425,10 +425,19 @@ serve<SocketData>({
       }
       // The session guard, applied by hand because middleware does not run on an upgrade. An
       // unauthenticated socket here would be the whole point of the proxy defeated.
-      const actor = await identifyUser(request).catch(() => null);
+      const actor = await resolveRequestActor(request).catch(() => null);
       if (!actor) {
         return new Response("Sign in first.", { status: 401 });
       }
+      // And which Bot, which the guard above does not answer. This socket carries that Bot's screen,
+      // so signing in is not enough: without this, anybody signed in watches anybody's Bot work.
+      if (
+        !(await agentProfileStore
+          .get({ id: actor.id, role: actor.role }, streamBotId)
+          .catch(() => null))
+      ) {
+        return new Response("There is no such Bot.", { status: 404 });
+      }
       // Located per Bot when there is a supervisor, and the one shared computer when there is not.
       let upstream: string;
       try {