From e83d9bb529a9dd43087bfd91368f90513f0ef79a Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 8 Nov 2026 20:16:24 -0600 Subject: [PATCH] fix(authz): make board-only denials final and actionable for agents Agents that hit a board-only route got a bare 403 "Board access required" and kept searching the API for another route until the chat attempt ran out. The shared assertBoard denial now tells agent actors the action is board-only by design, that no other route can do it, and names the board page where the person acts when the route maps to one (company settings, secrets, budgets, members, gateways, approvals, agent profile). The HTTP status and error text are unchanged; agents also get code board_access_required. Chat guidance treats a board-only operation as final instead of asking the agent to discover an operator path. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/board-access-denial.test.ts | 74 +++++++++++++++++++ .../__tests__/browser-use-route-scope.test.ts | 3 +- server/src/__tests__/costs-service.test.ts | 4 +- server/src/middleware/error-handler.ts | 3 + server/src/routes/authz.ts | 41 +++++++++- server/src/routes/environments.ts | 6 +- server/src/routes/instance-settings.ts | 6 +- server/src/routes/issues.ts | 5 +- server/src/services/agent-conversations.ts | 2 +- 9 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 server/src/__tests__/board-access-denial.test.ts diff --git a/server/src/__tests__/board-access-denial.test.ts b/server/src/__tests__/board-access-denial.test.ts new file mode 100644 index 00000000000..98b20c19e37 --- /dev/null +++ b/server/src/__tests__/board-access-denial.test.ts @@ -0,0 +1,74 @@ +import express from "express"; +import request from "supertest"; +import { describe, expect, it } from "vitest"; + +import { errorHandler } from "../middleware/index.js"; +import { assertBoard, assertBoardOrgAccess, BOARD_ACCESS_REQUIRED_CODE } from "../routes/authz.js"; +import { AGENT_CHAT_DIRECTIVE } from "../services/agent-conversations.js"; + +const COMPANY_ID = "10000000-0000-4000-8000-000000000001"; +const AGENT_ID = "20000000-0000-4000-8000-000000000002"; + +const agentActor: Express.Request["actor"] = { type: "agent", source: "agent_key", agentId: "agent-1", companyId: COMPANY_ID }; +const boardActor: Express.Request["actor"] = { type: "board", source: "local_implicit", userId: "local-board" }; + +function createApp(actor: Express.Request["actor"], guard = assertBoard) { + const app = express(); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + const router = express.Router(); + router.all(/.*/, (req, res) => { + guard(req); + res.json({ ok: true }); + }); + app.use("/api", router); + app.use(errorHandler); + return app; +} + +describe("board-only denial", () => { + it.each([ + ["post", `/api/companies/${COMPANY_ID}/archive`, "[Open company settings](//company/settings)"], + ["post", `/api/companies/${COMPANY_ID}/secrets`, "[Open company secrets](//company/settings/secrets)"], + ["post", `/api/secrets/${AGENT_ID}/rotate`, "[Open company secrets](//company/settings/secrets)"], + ["post", `/api/companies/${COMPANY_ID}/budget-incidents/i-1/resolve?x=1`, "[Open budgets](//activity/budgets)"], + ["patch", `/api/agents/${AGENT_ID}/permissions`, `[Open the agent's profile](//agents/${AGENT_ID})`], + ] as const)("tells an agent %s %s is final and names the board page", async (method, path, link) => { + const res = await request(createApp(agentActor))[method](path).expect(403); + expect(res.body.error).toBe("Board access required"); + expect(res.body.code).toBe(BOARD_ACCESS_REQUIRED_CODE); + expect(res.body.remediation).toContain("this denial is final"); + expect(res.body.remediation).toContain("do not search for one"); + expect(res.body.remediation).toContain(link); + }); + + it("falls back to a general board instruction when no page is known", async () => { + const res = await request(createApp(agentActor)).get(`/api/companies/${COMPANY_ID}/managed-agent-profiles`).expect(403); + expect(res.body.code).toBe(BOARD_ACCESS_REQUIRED_CODE); + expect(res.body.remediation).toContain("this denial is final"); + expect(res.body.remediation).toContain("Paperclip web app or with their own board credentials"); + expect(res.body.remediation).not.toContain("](/"); + }); + + it("applies through the org-access guard", async () => { + const res = await request(createApp(agentActor, assertBoardOrgAccess)).delete(`/api/companies/${COMPANY_ID}`).expect(403); + expect(res.body.code).toBe(BOARD_ACCESS_REQUIRED_CODE); + expect(res.body.remediation).toContain("[Open company settings](//company/settings)"); + }); + + it("keeps the plain denial for actors that are not agents", async () => { + const res = await request(createApp({ type: "none", source: "none" })).post(`/api/companies/${COMPANY_ID}/archive`).expect(403); + expect(res.body).toEqual({ error: "Board access required" }); + await request(createApp(boardActor)).post(`/api/companies/${COMPANY_ID}/archive`).expect(200); + }); + + it("chat guidance treats a board-only operation as final", () => { + expect(AGENT_CHAT_DIRECTIVE).toContain("its API catalog authorization actor is board, or a call returns 403 \"Board access required\""); + expect(AGENT_CHAT_DIRECTIVE).toContain("that boundary is final"); + expect(AGENT_CHAT_DIRECTIVE).toContain("so stop searching for one"); + expect(AGENT_CHAT_DIRECTIVE).toContain("tell the person which control in the Paperclip web app to use, link its page when you know the route"); + expect(AGENT_CHAT_DIRECTIVE).not.toContain("use permitted discovery to identify the supported operator path"); + }); +}); diff --git a/server/src/__tests__/browser-use-route-scope.test.ts b/server/src/__tests__/browser-use-route-scope.test.ts index 113b4081849..df017d48fe7 100644 --- a/server/src/__tests__/browser-use-route-scope.test.ts +++ b/server/src/__tests__/browser-use-route-scope.test.ts @@ -64,7 +64,8 @@ describe("browser task route scope", () => { actorType = type; const response = await http(app)[method](path); expect(response.status).toBe(type === "board" ? 404 : 403); - expect(response.body).toEqual({ error: type === "board" ? "Task not found" : "Board access required" }); + expect(response.body).toMatchObject({ error: type === "board" ? "Task not found" : "Board access required" }); + expect(response.body.code).toBe(type === "agent" ? "board_access_required" : undefined); } } expect(select).not.toHaveBeenCalled(); diff --git a/server/src/__tests__/costs-service.test.ts b/server/src/__tests__/costs-service.test.ts index 62380d7f490..06a393eadf1 100644 --- a/server/src/__tests__/costs-service.test.ts +++ b/server/src/__tests__/costs-service.test.ts @@ -341,7 +341,7 @@ describe("cost routes", () => { .send({ budgetMonthlyCents: 2500 }); expect(res.status).toBe(403); - expect(res.body).toEqual({ error: "Board access required" }); + expect(res.body).toMatchObject({ error: "Board access required", code: "board_access_required" }); expect(mockAgentService.update).not.toHaveBeenCalled(); expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalled(); @@ -360,7 +360,7 @@ describe("cost routes", () => { .send({ budgetMonthlyCents: 2500 }); expect(res.status).toBe(403); - expect(res.body).toEqual({ error: "Board access required" }); + expect(res.body).toMatchObject({ error: "Board access required", code: "board_access_required" }); expect(mockAgentService.update).not.toHaveBeenCalled(); expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalled(); diff --git a/server/src/middleware/error-handler.ts b/server/src/middleware/error-handler.ts index 438a27acda6..d374e5f37d1 100644 --- a/server/src/middleware/error-handler.ts +++ b/server/src/middleware/error-handler.ts @@ -174,6 +174,8 @@ export function errorHandler( const redactedSkillPolicyDenial = isRedactedSkillPolicyDenial(details); const workspaceRepairPreconditionFailure = details?.code === "workspace_repair_precondition_failed"; + // Its code and remediation are already top-level; repeating them only lengthens agent context. + const boardAccessDenial = details?.code === "board_access_required"; const structuredConnectionError = new Set([ "user_authorization_required", "organization_authorization_required", @@ -255,6 +257,7 @@ export function errorHandler( : {}), ...(!redactedSkillPolicyDenial && !workspaceRepairPreconditionFailure && + !boardAccessDenial && responseDetailsValue ? { details: responseDetailsValue } : {}), diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index 2c3f6359cbc..0d65dc66536 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -29,9 +29,48 @@ export function assertAuthenticated(req: Request) { } } +export const BOARD_ACCESS_REQUIRED_CODE = "board_access_required"; + +// The board page that holds the control behind a board-only API route, when one +// is known. Paths keep for the agent to fill from its own task identifier. +const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray) => [string, string]]> = [ + [/^\/(?:companies\/[^/]+\/)?secrets?(?:[-/]|$)/, () => ["company secrets", "//company/settings/secrets"]], + [/^\/companies\/[^/]+\/(?:budgets|budget-incidents|costs)(?:\/|$)/, () => ["budgets", "//activity/budgets"]], + [/^\/companies\/[^/]+\/(?:members|invites|join-requests)(?:\/|$)/, () => ["company members", "//company/settings/members"]], + [/^\/companies\/[^/]+\/(?:tools\/)?gateways(?:\/|$)|^\/tool-gateway\//, () => ["tool gateways", "//apps/gateways"]], + [/^\/approvals\/([^/]+)/, match => ["the approval", `//approvals/${match[1]}`]], + [/^\/agents\/([0-9a-f-]{36})(?:\/|$)/i, match => ["the agent's profile", `//agents/${match[1]}`]], + [/^\/companies\/[^/]+(?:\/(?:archive|branding|settings))?\/?$/, () => ["company settings", "//company/settings"]], +]; + +function boardControlPage(req: Request) { + const path = (req.originalUrl ?? "").split("?")[0]!.replace(/^\/api(?=\/)/, ""); + for (const [pattern, page] of BOARD_CONTROL_PAGES) { + const match = pattern.exec(path); + if (match) return page(match); + } + return null; +} + +/** + * A board-only denial. Agents get a final, actionable remediation so they hand the + * action to the person instead of searching other routes for one that does not exist. + */ +export function boardAccessRequired(req: Request) { + if (req.actor.type !== "agent") return forbidden("Board access required"); + const page = boardControlPage(req); + const where = page + ? `They can do it in the Paperclip web app: link [Open ${page[0]}](${page[1]}), with this conversation's company prefix as .` + : "They can do it in the Paperclip web app or with their own board credentials; link the most specific page you know."; + return forbidden("Board access required", { + code: BOARD_ACCESS_REQUIRED_CODE, + remediation: `This action is board-only by design and this denial is final: no agent credential, tool or other API route can do it, so do not search for one. Tell the person that a board user must do it. ${where}`, + }); +} + export function assertBoard(req: Request) { if (req.actor.type !== "board") { - throw forbidden("Board access required"); + throw boardAccessRequired(req); } } diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 7acd2f1fddb..3260ae6c989 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -59,7 +59,7 @@ import { assertAgentsReadyForEnvironmentChange, } from "../services/plugin-environment-driver.js"; import { getConfiguredSecretProvider } from "../secrets/configured-provider.js"; -import { assertBoardOrgAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertBoardOrgAccess, getActorInfo } from "./authz.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; import { environmentService } from "../services/environments.js"; import { SANDBOX_PLATFORM_PROBE_METADATA_KEY } from "../services/environment-custom-image-runtime.js"; @@ -400,9 +400,7 @@ export function environmentRoutes( } function assertCustomImageCompanyAccess(req: Request, companyId: string) { - if (req.actor.type !== "board") { - throw forbidden("Board access required"); - } + assertBoard(req); if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; const allowedCompanies = req.actor.companyIds ?? []; if (!allowedCompanies.includes(companyId)) { diff --git a/server/src/routes/instance-settings.ts b/server/src/routes/instance-settings.ts index 0d7d28c588f..3372b295792 100644 --- a/server/src/routes/instance-settings.ts +++ b/server/src/routes/instance-settings.ts @@ -32,7 +32,7 @@ import { } from "../services/index.js"; import { environmentService } from "../services/environments.js"; import { assertEnvironmentSelectionForCompany } from "./environment-selection.js"; -import { assertBoardOrgAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertBoardOrgAccess, getActorInfo } from "./authz.js"; function sameJsonValue(a: unknown, b: unknown): boolean { if (a === b) return true; @@ -90,9 +90,7 @@ function publishActivitiesBestEffort(publications: ActivityPublication[], action } function assertCanManageInstanceSettings(req: Request) { - if (req.actor.type !== "board") { - throw forbidden("Board access required"); - } + assertBoard(req); if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) { return; } diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 29d8aeb579c..24b0a3cdf65 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -17016,10 +17016,7 @@ export function issueRoutes( }); router.post("/issues/:id/admin/force-release", async (req, res) => { - if (req.actor.type !== "board") { - res.status(403).json({ error: "Board access required" }); - return; - } + assertBoard(req); if (!req.actor.userId) { throw forbidden("Board user context required"); } diff --git a/server/src/services/agent-conversations.ts b/server/src/services/agent-conversations.ts index 0f6cbf19979..2f81391892c 100644 --- a/server/src/services/agent-conversations.ts +++ b/server/src/services/agent-conversations.ts @@ -93,7 +93,7 @@ export const AGENT_CHAT_DIRECTIVE = `You are in an ongoing conversation with the When changing defaults or policy for new or future work, update the resource that owns those defaults. Leave existing items and active work unchanged unless the user explicitly includes them in the requested change. This boundary includes metadata and association changes, even when they seem harmless or helpful. Do not broaden the request to align related existing items. Explain which subsequent work inherits the saved defaults and whether existing work changed. -If the current execution lacks the board or instance-operator identity required by an operation, use permitted discovery to identify the supported operator path, complete other permitted work, then explain the required handoff and end your response. A user's request or confirmation does not expand your credential's scope. Treat permission denials as boundaries: do not probe host processes, private control files, or unrelated endpoints for alternate access. Distinguish a requested action from a verified action. +If an operation needs board or instance-operator identity that this execution lacks (its API catalog authorization actor is board, or a call returns 403 "Board access required"), that boundary is final: no agent credential, tool or other route can do it, so stop searching for one. Complete other permitted work, then tell the person which control in the Paperclip web app to use, link its page when you know the route (a denial's remediation may name it), and end your response. A user's request or confirmation does not expand your credential's scope. Treat permission denials as boundaries: do not probe host processes, private control files, or unrelated endpoints for alternate access. Distinguish a requested action from a verified action. Research, clarify, and develop full plans here using the conversation's plan document. Revise the draft as the discussion develops. Planning alone does not create execution tasks. Put implementation and substantial execution into separate tasks.