mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
400e73d15c
commit
e83d9bb529
9 files changed
+127
-17
No files matched your search
@@ -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](/<prefix>/company/settings)"],
|
||||
["post", `/api/companies/${COMPANY_ID}/secrets`, "[Open company secrets](/<prefix>/company/settings/secrets)"],
|
||||
["post", `/api/secrets/${AGENT_ID}/rotate`, "[Open company secrets](/<prefix>/company/settings/secrets)"],
|
||||
["post", `/api/companies/${COMPANY_ID}/budget-incidents/i-1/resolve?x=1`, "[Open budgets](/<prefix>/activity/budgets)"],
|
||||
["patch", `/api/agents/${AGENT_ID}/permissions`, `[Open the agent's profile](/<prefix>/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](/<prefix>/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");
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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 }
|
||||
: {}),
|
||||
|
||||
@@ -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 <prefix> for the agent to fill from its own task identifier.
|
||||
const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray) => [string, string]]> = [
|
||||
[/^\/(?:companies\/[^/]+\/)?secrets?(?:[-/]|$)/, () => ["company secrets", "/<prefix>/company/settings/secrets"]],
|
||||
[/^\/companies\/[^/]+\/(?:budgets|budget-incidents|costs)(?:\/|$)/, () => ["budgets", "/<prefix>/activity/budgets"]],
|
||||
[/^\/companies\/[^/]+\/(?:members|invites|join-requests)(?:\/|$)/, () => ["company members", "/<prefix>/company/settings/members"]],
|
||||
[/^\/companies\/[^/]+\/(?:tools\/)?gateways(?:\/|$)|^\/tool-gateway\//, () => ["tool gateways", "/<prefix>/apps/gateways"]],
|
||||
[/^\/approvals\/([^/]+)/, match => ["the approval", `/<prefix>/approvals/${match[1]}`]],
|
||||
[/^\/agents\/([0-9a-f-]{36})(?:\/|$)/i, match => ["the agent's profile", `/<prefix>/agents/${match[1]}`]],
|
||||
[/^\/companies\/[^/]+(?:\/(?:archive|branding|settings))?\/?$/, () => ["company settings", "/<prefix>/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 <prefix>.`
|
||||
: "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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
Reference in new issue
Block a user