Skip to content

Commit e36c1bf

Browse files
kevin9327davidmckayv
authored andcommitted
Keep the computer fleet off a signed-in user's listing
The admin computers page asks for every Bot's machine. That list was behind a session only, so anyone signed in could read private coworker ids and whether those computers were running.
1 parent c3f4a04 commit e36c1bf

2 files changed

Lines changed: 101 additions & 3 deletions

File tree

server/src/computer/routes.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,16 @@ export function createComputerRoutes(
208208
* The computers, for the admin surface.
209209
*
210210
* Not per-Bot in the path the way the acting routes are: this asks the computer what it holds, and
211-
* it holds a list. `:botId` is still there because every route under this router has it and the
212-
* gateway wants somebody to attribute the call to.
211+
* it holds a list. `:botId` is still there because every route under this router has it. The
212+
* list itself is every computer, so a signed-in user is not enough; an administrator has to ask.
213213
*/
214214
routes.get("/:botId/computers", async (context) => {
215+
// The session guard and the question of whether this person may act as the Bot in the path are
216+
// both applied by the middleware above. Neither is the question here: the answer is the whole
217+
// fleet whatever `:botId` says, so it takes administering the deployment.
218+
const denied = requireAdmin(context);
219+
if (denied) return denied;
220+
215221
try {
216222
return context.json(await gateway.computers());
217223
} catch (error) {

server/tests/computer-routes.test.ts

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, test } from "bun:test";
22
import type { MiddlewareHandler } from "hono";
3-
import type { AppVariables } from "../src/auth/guards";
3+
import type { AppVariables, AuthenticatedActor } from "../src/auth/guards";
44
import type { ComputerGateway } from "../src/computer/gateway";
55
import type { PolicyStore } from "../src/computer/policy-store";
66
import { createComputerRoutes } from "../src/computer/routes";
@@ -40,3 +40,95 @@ describe("computer routes", () => {
4040
expect(requestedBotIds).toEqual(["bot-17"]);
4141
});
4242
});
43+
44+
/**
45+
* The fleet listing is the one route here that is not about the Bot in its path.
46+
*
47+
* `:botId` is ignored and the handler returns every computer, so a signed-in person asking about a
48+
* Bot they own learned every Bot id in the deployment and whether its computer was running,
49+
* private coworkers included. Being signed in is not the question; administering the deployment is.
50+
*/
51+
const member: AuthenticatedActor = {
52+
id: "user-1",
53+
email: "member@openbot.test",
54+
role: "user",
55+
};
56+
57+
const administrator: AuthenticatedActor = {
58+
id: "admin-1",
59+
email: "admin@openbot.test",
60+
role: "admin",
61+
};
62+
63+
function asActor(
64+
actor: AuthenticatedActor,
65+
): MiddlewareHandler<{ Variables: AppVariables }> {
66+
return async (context, next) => {
67+
context.set("actor", actor);
68+
await next();
69+
};
70+
}
71+
72+
function appFor(actor: AuthenticatedActor, computers: () => Promise<unknown>) {
73+
let listed = 0;
74+
const countingGateway = {
75+
async computers() {
76+
listed += 1;
77+
return computers();
78+
},
79+
} as ComputerGateway;
80+
81+
return {
82+
app: createComputerRoutes(
83+
countingGateway,
84+
{} as PolicyStore,
85+
asActor(actor),
86+
// Permissive. Whether this person may act as the Bot in the path is a different question with
87+
// its own suite, and `:botId` is not what this route answers about anyway.
88+
async () => true,
89+
),
90+
listed: () => listed,
91+
};
92+
}
93+
94+
describe("computer fleet listing", () => {
95+
test("refuses a signed-in user the fleet, and does not ask the gateway", async () => {
96+
const { app, listed } = appFor(member, async () => ({
97+
isolation: "per-bot",
98+
computers: [
99+
{ botId: "private-coworker", running: true, startedAt: null },
100+
],
101+
}));
102+
103+
const response = await app.request("http://openbot.test/any-bot/computers");
104+
105+
expect(response.status).toBe(403);
106+
await expect(response.json()).resolves.toEqual({
107+
error: "Administrator access required.",
108+
});
109+
// Refused before the gateway is asked: a check that runs after the fleet has been read is not a
110+
// check, it is a filter on the response.
111+
expect(listed()).toBe(0);
112+
});
113+
114+
test("lets an administrator see the fleet", async () => {
115+
const fleet = {
116+
isolation: "per-bot" as const,
117+
computers: [
118+
{
119+
botId: "private-coworker",
120+
running: true,
121+
startedAt: "2026-08-20T00:00:00.000Z",
122+
egress: null,
123+
},
124+
],
125+
};
126+
const { app, listed } = appFor(administrator, async () => fleet);
127+
128+
const response = await app.request("http://openbot.test/any-bot/computers");
129+
130+
expect(response.status).toBe(200);
131+
await expect(response.json()).resolves.toEqual(fleet);
132+
expect(listed()).toBe(1);
133+
});
134+
});

0 commit comments

Comments
 (0)