mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(authz): link plugin settings and say reply now on board-only denials (#20182)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents run with agent credentials, and plugin management routes are board-only > - A board-only denial tells the agent the denial is final and links the board page that holds the control, when the page is known (#20151) > - Plugin routes had no known page, so the denial said "link a page only if you already know its route" > - In a chat eval, an agent asked to finish configuring a plugin got this denial and kept searching the API for about two minutes, so it did not answer in time > - This pull request names the plugin manager, or that plugin's settings page, in the denial > - The benefit is that the agent can hand the step to the person with a working link right away ## Linked Issues or Issue Description Refs #20151 **What happened** An agent that called a board-only plugin route (for example `GET /api/plugins` or `POST /api/plugins/:pluginId/config`) got the generic remediation with no page. In the `plugins.configure-plugin` chat eval, the agent kept calling `search_api` for another plugin route after the 403 and did not reply within the 150-second window. **Expected behavior** The denial names the page where a board user does the step: the plugin manager (`/<prefix>/company/settings/instance/plugins`) or the plugin's settings page (`/<prefix>/company/settings/instance/plugins/<id or key>`). No link is given when the operator hides plugin management (`PAPERCLIP_HIDDEN_SETTINGS=instance.plugins`). **Steps to reproduce** 1. Install a plugin. 2. As an agent, call `POST /api/plugins/<pluginId>/config`. 3. See a 403 `board_access_required` remediation without a plugin page. ## What Changed - `server/src/routes/authz.ts`: board-only denials on `/api/plugins` routes link the plugin manager, or the plugin's settings page when the path names a plugin UUID or key. The plugin settings page accepts either. No link when plugin management is hidden. - `server/src/routes/authz.ts`: every agent board-only denial now says to stop working on the denied step and reply now. With the plugin link alone, the eval agent still searched the API for two more minutes. - `server/src/routes/authz.ts`: any manifest-format plugin key (including dot-free keys such as `my-plugin`) gets a direct link; plugin manager endpoints (`examples`, `install`, `tools`, `ui-contributions`) link the plugin list. - `ui/src/pages/PluginSettings.tsx`: custom settings slots match by plugin key as well as plugin id, so a page opened by key shows the plugin's own settings. - `server/src/__tests__/board-access-denial.test.ts`: cases for list, install, config by id, disable by key, a dot-free key, manager endpoints, and the hidden-settings case. `ui/src/pages/PluginSettings.test.tsx`: opening the page by key mounts that plugin's settings slot. - Merged `master`, keeping its project access-members denial cases. ## Verification - `cd server && npx vitest run src/__tests__/board-access-denial.test.ts src/__tests__/agent-permissions-routes.test.ts src/__tests__/environment-routes.test.ts src/__tests__/execution-workspace-runtime-lease-route.test.ts`: 241 passed after merging master. - `cd ui && npx vitest run src/pages/PluginSettings.test.tsx`: 9 passed; the new key-route case fails without the `PluginSettings` change. - `cd server && pnpm typecheck`: passes. ## Risks - Low risk. Only the remediation text of agent board-only denials on `/api/plugins` routes changes. Board users and authorization do not change. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
492316e576
commit
7c65a2f479
4 files changed
+73
-6
No files matched your search
@@ -39,6 +39,13 @@ describe("board-only denial", () => {
|
||||
["patch", `/api/agents/${AGENT_ID}/permissions`, `[Open the agent's permissions](/<prefix>/agents/${AGENT_ID}/permissions)`],
|
||||
["post", `/api/agents/${AGENT_ID}/keys`, `[Open the agent's API keys](/<prefix>/agents/${AGENT_ID}/api-keys)`],
|
||||
["post", `/api/agents/${AGENT_ID}/pause`, `[Open the agent's profile](/<prefix>/agents/${AGENT_ID})`],
|
||||
["get", "/api/plugins", "[Open plugin settings](/<prefix>/company/settings/instance/plugins)"],
|
||||
["post", "/api/plugins/install", "[Open plugin settings](/<prefix>/company/settings/instance/plugins)"],
|
||||
["post", `/api/plugins/${AGENT_ID}/config`, `[Open the plugin's settings](/<prefix>/company/settings/instance/plugins/${AGENT_ID})`],
|
||||
["post", "/api/plugins/acme.website-preview/disable", "[Open the plugin's settings](/<prefix>/company/settings/instance/plugins/acme.website-preview)"],
|
||||
["post", "/api/plugins/my-plugin/config", "[Open the plugin's settings](/<prefix>/company/settings/instance/plugins/my-plugin)"],
|
||||
["post", "/api/plugins/tools/execute", "[Open plugin settings](/<prefix>/company/settings/instance/plugins)"],
|
||||
["get", "/api/plugins/ui-contributions", "[Open plugin settings](/<prefix>/company/settings/instance/plugins)"],
|
||||
["post", `/api/projects/${AGENT_ID}/access-members`, `[Open the project's access settings](/<prefix>/projects/${AGENT_ID}/configuration)`],
|
||||
["delete", `/api/projects/${AGENT_ID}/access-members/m-1`, `[Open the project's access settings](/<prefix>/projects/${AGENT_ID}/configuration)`],
|
||||
] as const)("tells an agent %s %s is final and names the board page", async (method, path, link) => {
|
||||
@@ -47,6 +54,7 @@ describe("board-only denial", () => {
|
||||
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("reply now");
|
||||
expect(res.body.remediation).toContain(link);
|
||||
});
|
||||
|
||||
@@ -88,6 +96,19 @@ describe("board-only denial", () => {
|
||||
expect(res.body.remediation).not.toContain("/company/settings/connections");
|
||||
});
|
||||
|
||||
it("does not link plugin settings while the operator hides plugin management", async () => {
|
||||
const previous = process.env.PAPERCLIP_HIDDEN_SETTINGS;
|
||||
process.env.PAPERCLIP_HIDDEN_SETTINGS = "instance.plugins";
|
||||
try {
|
||||
const res = await request(createApp(agentActor)).post(`/api/plugins/${AGENT_ID}/config`).expect(403);
|
||||
expect(res.body.remediation).toContain("this denial is final");
|
||||
expect(res.body.remediation).not.toContain("/instance/plugins");
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.PAPERCLIP_HIDDEN_SETTINGS;
|
||||
else process.env.PAPERCLIP_HIDDEN_SETTINGS = previous;
|
||||
}
|
||||
});
|
||||
|
||||
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);
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { SecretBindingTargetType } from "@paperclipai/shared";
|
||||
import { forbidden, HttpError, unauthorized } from "../errors.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { responsibleUserAuthzShadowMode } from "../services/authorization.js";
|
||||
import { getHiddenSettings } from "../services/settings-visibility.js";
|
||||
|
||||
function throwOrShadowResponsibleUserCompanyAccessDeny(
|
||||
req: Request,
|
||||
@@ -65,8 +66,25 @@ const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray, req: Request)
|
||||
[/^\/companies\/[^/]+(?:\/(?:archive|branding|settings))?\/?$/, () => ["company settings", "/<prefix>/company/settings"]],
|
||||
];
|
||||
|
||||
// Plugin management pages. A plugin id or key segment opens that plugin's settings
|
||||
// (the page accepts either); a plugin-manager endpoint segment opens the manager.
|
||||
// An operator can hide plugin management, so link it only while it is visible.
|
||||
const PLUGIN_REFERENCE = /^[a-z0-9][a-z0-9._-]*$/i;
|
||||
const PLUGIN_MANAGER_SEGMENTS = new Set(["examples", "install", "tools", "ui-contributions"]);
|
||||
function pluginControlPage(path: string): [string, string] | null {
|
||||
const match = /^\/plugins(?:\/([^/]+))?(?:\/|$)/.exec(path);
|
||||
if (!match || getHiddenSettings().has("instance.plugins")) return null;
|
||||
const segment = match[1];
|
||||
const plugin = segment && PLUGIN_REFERENCE.test(segment) && !PLUGIN_MANAGER_SEGMENTS.has(segment) ? segment : null;
|
||||
return plugin
|
||||
? ["the plugin's settings", `/<prefix>/company/settings/instance/plugins/${plugin}`]
|
||||
: ["plugin settings", "/<prefix>/company/settings/instance/plugins"];
|
||||
}
|
||||
|
||||
function boardControlPage(req: Request) {
|
||||
const path = (req.originalUrl ?? "").split("?")[0]!.replace(/^\/api(?=\/)/, "");
|
||||
const pluginPage = pluginControlPage(path);
|
||||
if (pluginPage) return pluginPage;
|
||||
for (const [pattern, page] of BOARD_CONTROL_PAGES) {
|
||||
const match = pattern.exec(path);
|
||||
if (match) return page(match, req);
|
||||
@@ -86,7 +104,7 @@ export function boardAccessRequired(req: Request) {
|
||||
: "They can do it in the Paperclip web app or with their own board credentials. Link a page only if you already know its route; do not search for one.";
|
||||
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}`,
|
||||
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. Stop working on this step and reply now: tell the person that a board user must do it. ${where}`,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,8 @@ const mockPluginsApi = vi.hoisted(() => ({
|
||||
}));
|
||||
|
||||
const mockSetBreadcrumbs = vi.hoisted(() => vi.fn());
|
||||
const mockRoute = vi.hoisted(() => ({ pluginId: "plugin-1" }));
|
||||
const mockSlots = vi.hoisted(() => ({ slots: [] as Array<Record<string, unknown>> }));
|
||||
|
||||
vi.mock("@/api/plugins", () => ({
|
||||
pluginsApi: mockPluginsApi,
|
||||
@@ -40,12 +42,12 @@ vi.mock("@/context/CompanyContext", () => ({
|
||||
vi.mock("@/lib/router", () => ({
|
||||
Link: ({ to, children }: { to: string; children: React.ReactNode }) => <a href={to}>{children}</a>,
|
||||
Navigate: () => null,
|
||||
useParams: () => ({ companyPrefix: "PAP", pluginId: "plugin-1" }),
|
||||
useParams: () => ({ companyPrefix: "PAP", pluginId: mockRoute.pluginId }),
|
||||
}));
|
||||
|
||||
vi.mock("@/plugins/slots", () => ({
|
||||
PluginSlotMount: () => null,
|
||||
usePluginSlots: () => ({ slots: [] }),
|
||||
PluginSlotMount: ({ slot }: { slot: { id: string } }) => <div data-testid="settings-slot">{slot.id}</div>,
|
||||
usePluginSlots: () => ({ slots: mockSlots.slots }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/PageTabBar", () => ({
|
||||
@@ -168,6 +170,31 @@ describe("PluginSettings", () => {
|
||||
container.remove();
|
||||
document.body.innerHTML = "";
|
||||
vi.clearAllMocks();
|
||||
mockRoute.pluginId = "plugin-1";
|
||||
mockSlots.slots = [];
|
||||
});
|
||||
|
||||
it("shows a plugin's custom settings page when the route names the plugin by key", async () => {
|
||||
mockRoute.pluginId = "acme.website-preview";
|
||||
mockSlots.slots = [
|
||||
{ id: "settings", type: "settingsPage", pluginId: "plugin-1", pluginKey: "acme.website-preview" },
|
||||
{ id: "other-settings", type: "settingsPage", pluginId: "plugin-2", pluginKey: "acme.other" },
|
||||
];
|
||||
mockPluginsApi.get.mockResolvedValue(basePlugin({
|
||||
pluginKey: "acme.website-preview",
|
||||
status: "ready",
|
||||
manifestJson: { displayName: "Website Preview", version: "0.1.0", capabilities: [] },
|
||||
}));
|
||||
|
||||
const root = await renderSettings(container);
|
||||
|
||||
const mounted = [...container.querySelectorAll('[data-testid="settings-slot"]')].map((node) => node.textContent);
|
||||
expect(mounted).toEqual(["settings"]);
|
||||
expect(container.textContent).not.toContain("This plugin does not require any settings.");
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("routes environment-provider plugins to instance environments when they have no instance config", async () => {
|
||||
|
||||
@@ -118,8 +118,9 @@ export function PluginSettings() {
|
||||
enabled: !!selectedCompanyId,
|
||||
});
|
||||
|
||||
// Filter slots to only show settings pages for this specific plugin
|
||||
const pluginSlots = slots.filter((slot) => slot.pluginId === pluginId);
|
||||
// Filter slots to only show settings pages for this specific plugin. The route
|
||||
// segment can be the plugin's id or its key, so match either.
|
||||
const pluginSlots = slots.filter((slot) => slot.pluginId === pluginId || slot.pluginKey === pluginId);
|
||||
|
||||
// If the plugin has a custom settingsPage slot, prefer that over auto-generated form
|
||||
const hasCustomSettingsPage = pluginSlots.length > 0;
|
||||
|
||||
Reference in new issue
Block a user