From 995a04a25dca7f9090d44ff0d5077b10409cb0ad Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Sat, 5 Sep 2026 20:58:09 +0900 Subject: [PATCH] Record a Bot's decline only against a Bot the caller may reach The declined route wrote a bot.declined row, marked "reportedBy: the Bot itself", for whatever agent id was in the path. Every other route on a Bot asks the store first and answers not found for one the caller cannot reach; this one did not, so any signed-in person could put a decline in any words against any coworker, including one they cannot see, and the trail would present it as the Bot's own word. Ask the store the same question before recording, and answer not found for a Bot the caller cannot reach, writing nothing. --- CHANGELOG.md | 9 ++++++ server/src/agents/routes.ts | 15 ++++++++++ server/tests/bot-lifecycle-audit.test.ts | 38 ++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aae4275bf..939dacd6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/server/src/agents/routes.ts b/server/src/agents/routes.ts index aec1a557b..dc398f7a5 100644 --- a/server/src/agents/routes.ts +++ b/server/src/agents/routes.ts @@ -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, { diff --git a/server/tests/bot-lifecycle-audit.test.ts b/server/tests/bot-lifecycle-audit.test.ts index 00c18128c..2ace6ea3f 100644 --- a/server/tests/bot-lifecycle-audit.test.ts +++ b/server/tests/bot-lifecycle-audit.test.ts @@ -26,6 +26,8 @@ function app(overrides: Record = {}) { }; 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" }), @@ -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 = {