Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.

## Unreleased

### A Bot's own decline is only recorded against a Bot the caller may reach

A Bot reports that it declined a request through the person's session, and the audit row says
`reportedBy: the Bot itself`. The route wrote that row for any agent id in the path, without asking
whether the caller could reach that Bot, so any signed-in person could put a decline, in any words,
against any coworker, one they cannot see included, and an administrator reading the trail would take
it for something the Bot said. The route now asks the store first, as every other route on a Bot
does, and answers not found for a Bot the caller cannot reach, writing nothing.

### The engine socket the supervisor is given can be pointed somewhere else

Compose mounted `/var/run/docker.sock` into the supervisor as a fixed path. That is correct for
Expand Down
15 changes: 15 additions & 0 deletions server/src/agents/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,21 @@ export function createAgentRoutes(
return context.json({ error: "A reason is required." }, 400);
}

/*
* The same question every other route here asks first: is this a Bot the caller may reach?
*
* The row says "reportedBy: the Bot itself", and the Bot reports through the person's session,
* so the trail's only way of knowing the report came from a Bot is that the person could have
* been talking to that Bot. Without this check, any signed-in person could write a decline
* against any id at all, a coworker they cannot see included, and an administrator reading the
* trail would take it for something the Bot said. Not found rather than forbidden, as the store
* answers everywhere else, so the check does not confirm which ids exist.
*/
const agent = await store.get(context.var.actor, agentId);
if (!agent) {
return context.json({ error: "Agent not found." }, 404);
}

if (auditStore) {
const actor = context.var.actor;
await recordAuditEvent(auditStore, {
Expand Down
38 changes: 38 additions & 0 deletions server/tests/bot-lifecycle-audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ function app(overrides: Record<string, unknown> = {}) {
};

const store = {
get: async (_actor: unknown, id: string) =>
id === "bot-1" ? { id: "bot-1", name: "Sales" } : null,
create: async () => ({ id: "bot-1", name: "Sales" }),
update: async () => ({ id: "bot-1", name: "Sales" }),
duplicate: async () => ({ id: "bot-2", name: "Sales copy" }),
Expand Down Expand Up @@ -209,6 +211,42 @@ describe("what a Bot is, on the trail", () => {
expect(rows).toHaveLength(0);
});

test("a decline is recorded against a Bot the caller may reach", async () => {
// The Bot reports through the person's session, so the row is only worth what that session
// could reach: a decline on a Bot the caller may talk to is the Bot's own word.
const { rows, hono } = app();

const response = await hono.request("http://t/api/agents/bot-1/declined", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ reason: "It asked me to delete the ledger." }),
});

expect(response.status).toBe(200);
expect(rows[0]?.eventType).toBe("bot.declined");
expect(rows[0]?.targetId).toBe("bot-1");
expect(rows[0]?.payload.reportedBy).toBe("the Bot itself");
});

test("a decline against a Bot the caller cannot reach is not found, and writes nothing", async () => {
// Every other route here asks the store first. Without the same question, a signed-in person
// could write "the Bot itself declined" against any id at all and an administrator would read
// it as something the Bot said.
const { rows, hono } = app();

const response = await hono.request(
"http://t/api/agents/somebody-elses-bot/declined",
{
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ reason: "Forged." }),
},
);

expect(response.status).toBe(404);
expect(rows).toHaveLength(0);
});

test("a trail that is down does not fail the change", async () => {
// The Bot is already updated and the caller has been told so.
const failing: AuditStore = {
Expand Down