From 7a329fb8bb9f74cdd75f3f7789c43fab0b2e7581 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:56:48 -0500 Subject: [PATCH] Harden API route authorization boundaries (#4122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The REST API is the control-plane boundary for companies, agents, plugins, adapters, costs, invites, and issue mutations. > - Several routes still relied on broad board or company access checks without consistently enforcing the narrower actor, company, and active-checkout boundaries those operations require. > - That can allow agents or non-admin users to mutate sensitive resources outside the intended governance path. > - This pull request hardens the route authorization layer and adds regression coverage for the audited API surfaces. > - The benefit is tighter multi-company isolation, safer plugin and adapter administration, and stronger enforcement of active issue ownership. ## What Changed - Added route-level authorization checks for budgets, plugin administration/scoped routes, adapter management, company import/export, direct agent creation, invite test resolution, and issue mutation/write surfaces. - Enforced active checkout ownership for agent-authenticated issue mutations, while preserving explicit management overrides for permitted managers. - Restricted sensitive adapter and plugin management operations to instance-admin or properly scoped actors. - Tightened company portability and invite probing routes so agents cannot cross company boundaries. - Updated access constants and the Company Access UI copy for the new active-checkout management grant. - Added focused regression tests covering cross-company denial, agent self-mutation denial, admin-only operations, and active checkout ownership. - Rebased the branch onto `public-gh/master` and fixed validation fallout from the rebase: heartbeat-context route ordering and a company import/export e2e fixture that now opts out of direct-hire approval before using direct agent creation. - Updated onboarding and signoff e2e setup to create seed agents through `/agent-hires` plus board approval, so they remain compatible with the approval-gated new-agent default. - Addressed Greptile feedback by removing a duplicate company export API alias, avoiding N+1 reporting-chain lookups in active-checkout override checks, allowing agent mutations on unassigned `in_progress` issues, and blocking NAT64 invite-probe targets. ## Verification - `pnpm exec vitest run server/src/__tests__/issues-goal-context-routes.test.ts cli/src/__tests__/company-import-export-e2e.test.ts` - `pnpm exec vitest run server/src/__tests__/plugin-routes-authz.test.ts server/src/__tests__/adapter-routes-authz.test.ts server/src/__tests__/agent-permissions-routes.test.ts server/src/__tests__/company-portability-routes.test.ts server/src/__tests__/costs-service.test.ts server/src/__tests__/invite-test-resolution-route.test.ts server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/agent-adapter-validation-routes.test.ts` - `pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts` - `pnpm exec vitest run server/src/__tests__/invite-test-resolution-route.test.ts` - `pnpm -r typecheck` - `pnpm --filter server typecheck` - `pnpm --filter ui typecheck` - `pnpm build` - `pnpm test:e2e -- tests/e2e/onboarding.spec.ts tests/e2e/signoff-policy.spec.ts` - `pnpm test:e2e -- tests/e2e/signoff-policy.spec.ts` - `pnpm test:run` was also run. It failed under default full-suite parallelism with two order-dependent failures in `plugin-routes-authz.test.ts` and `routines-e2e.test.ts`; both files passed when rerun directly together with `pnpm exec vitest run server/src/__tests__/plugin-routes-authz.test.ts server/src/__tests__/routines-e2e.test.ts`. ## Risks - Medium risk: this changes authorization behavior across multiple sensitive API surfaces, so callers that depended on broad board/company access may now receive `403` or `409` until they use the correct governance path. - Direct agent creation now respects the company-level board-approval requirement; integrations that need pending hires should use `/api/companies/:companyId/agent-hires`. - Active in-progress issue mutations now require checkout ownership or an explicit management override, which may reveal workflow assumptions in older automation. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5 coding agent, tool-using workflow with local shell, Git, GitHub CLI, and repository tests. ## 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 - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../company-import-export-e2e.test.ts | 5 + packages/shared/src/constants.ts | 1 + .../__tests__/adapter-routes-authz.test.ts | 255 ++++++++++++ .../agent-adapter-validation-routes.test.ts | 14 +- .../agent-permissions-routes.test.ts | 109 ++++- .../company-portability-routes.test.ts | 110 ++++- server/src/__tests__/costs-service.test.ts | 100 ++++- .../invite-test-resolution-route.test.ts | 190 +++++++++ ...ue-agent-mutation-ownership-routes.test.ts | 375 ++++++++++++++++++ .../src/__tests__/plugin-routes-authz.test.ts | 281 ++++++++++++- server/src/routes/access.ts | 266 +++++++++++-- server/src/routes/adapters.ts | 25 +- server/src/routes/agents.ts | 25 +- server/src/routes/companies.ts | 2 +- server/src/routes/costs.ts | 8 +- server/src/routes/issues.ts | 159 ++++++-- server/src/routes/plugins.ts | 80 +++- tests/e2e/signoff-policy.spec.ts | 13 +- ui/src/api/companies.ts | 7 +- ui/src/components/OnboardingWizard.tsx | 13 +- ui/src/pages/CompanyAccess.tsx | 1 + ui/src/pages/CompanyExport.tsx | 2 +- 22 files changed, 1903 insertions(+), 138 deletions(-) create mode 100644 server/src/__tests__/adapter-routes-authz.test.ts create mode 100644 server/src/__tests__/invite-test-resolution-route.test.ts create mode 100644 server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts diff --git a/cli/src/__tests__/company-import-export-e2e.test.ts b/cli/src/__tests__/company-import-export-e2e.test.ts index c543249ea6..6d7ac1d467 100644 --- a/cli/src/__tests__/company-import-export-e2e.test.ts +++ b/cli/src/__tests__/company-import-export-e2e.test.ts @@ -287,6 +287,11 @@ describeEmbeddedPostgres("paperclipai company import/export e2e", () => { headers: { "content-type": "application/json" }, body: JSON.stringify({ name: `CLI Export Source ${Date.now()}` }), }); + await api(apiBase, `/api/companies/${sourceCompany.id}`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ requireBoardApprovalForNewAgents: false }), + }); const sourceAgent = await api<{ id: string; name: string }>( apiBase, diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 810bfd93e4..0e7b5927b7 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -434,6 +434,7 @@ export const PERMISSION_KEYS = [ "users:manage_permissions", "tasks:assign", "tasks:assign_scope", + "tasks:manage_active_checkouts", "joins:approve", ] as const; export type PermissionKey = (typeof PERMISSION_KEYS)[number]; diff --git a/server/src/__tests__/adapter-routes-authz.test.ts b/server/src/__tests__/adapter-routes-authz.test.ts new file mode 100644 index 0000000000..e2b976ccac --- /dev/null +++ b/server/src/__tests__/adapter-routes-authz.test.ts @@ -0,0 +1,255 @@ +import express from "express"; +import request from "supertest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ServerAdapterModule } from "../adapters/index.js"; + +const mocks = vi.hoisted(() => { + const externalRecords = new Map(); + + return { + externalRecords, + execFile: vi.fn((_file: string, _args: string[], optionsOrCallback: unknown, maybeCallback?: unknown) => { + const callback = typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback; + if (typeof callback === "function") { + callback(null, "", ""); + } + return { + kill: vi.fn(), + on: vi.fn(), + }; + }), + listAdapterPlugins: vi.fn(), + addAdapterPlugin: vi.fn((record: any) => { + externalRecords.set(record.type, record); + }), + removeAdapterPlugin: vi.fn((type: string) => { + externalRecords.delete(type); + }), + getAdapterPluginByType: vi.fn((type: string) => externalRecords.get(type)), + getAdapterPluginsDir: vi.fn(), + getDisabledAdapterTypes: vi.fn(), + setAdapterDisabled: vi.fn(), + loadExternalAdapterPackage: vi.fn(), + buildExternalAdapters: vi.fn(async () => []), + reloadExternalAdapter: vi.fn(), + getUiParserSource: vi.fn(), + getOrExtractUiParserSource: vi.fn(), + }; +}); + +vi.mock("node:child_process", () => ({ + execFile: mocks.execFile, +})); + +vi.mock("../services/adapter-plugin-store.js", () => ({ + listAdapterPlugins: mocks.listAdapterPlugins, + addAdapterPlugin: mocks.addAdapterPlugin, + removeAdapterPlugin: mocks.removeAdapterPlugin, + getAdapterPluginByType: mocks.getAdapterPluginByType, + getAdapterPluginsDir: mocks.getAdapterPluginsDir, + getDisabledAdapterTypes: mocks.getDisabledAdapterTypes, + setAdapterDisabled: mocks.setAdapterDisabled, +})); + +vi.mock("../adapters/plugin-loader.js", () => ({ + buildExternalAdapters: mocks.buildExternalAdapters, + loadExternalAdapterPackage: mocks.loadExternalAdapterPackage, + getUiParserSource: mocks.getUiParserSource, + getOrExtractUiParserSource: mocks.getOrExtractUiParserSource, + reloadExternalAdapter: mocks.reloadExternalAdapter, +})); + +const EXTERNAL_ADAPTER_TYPE = "external_admin_test"; +const EXTERNAL_PACKAGE_NAME = "paperclip-external-adapter"; +let adapterRoutes: typeof import("../routes/adapters.js").adapterRoutes; +let errorHandler: typeof import("../middleware/index.js").errorHandler; +let registerServerAdapter: typeof import("../adapters/registry.js").registerServerAdapter; +let unregisterServerAdapter: typeof import("../adapters/registry.js").unregisterServerAdapter; +let setOverridePaused: typeof import("../adapters/registry.js").setOverridePaused; + +function createAdapter(type = EXTERNAL_ADAPTER_TYPE): ServerAdapterModule { + return { + type, + models: [], + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: type, + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + }; +} + +function installedRecord(type = EXTERNAL_ADAPTER_TYPE) { + return { + packageName: EXTERNAL_PACKAGE_NAME, + type, + installedAt: new Date(0).toISOString(), + }; +} + +function createApp(actor: Express.Request["actor"]) { + if (!adapterRoutes || !errorHandler) { + throw new Error("adapter route test dependencies were not loaded"); + } + + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + app.use("/api", adapterRoutes()); + app.use(errorHandler); + return app; +} + +function boardMember(membershipRole: "admin" | "operator" | "viewer"): Express.Request["actor"] { + return { + type: "board", + userId: `${membershipRole}-user`, + userName: null, + userEmail: null, + source: "session", + isInstanceAdmin: false, + companyIds: ["company-1"], + memberships: [ + { + companyId: "company-1", + membershipRole, + status: "active", + }, + ], + }; +} + +const instanceAdmin: Express.Request["actor"] = { + type: "board", + userId: "instance-admin", + userName: null, + userEmail: null, + source: "session", + isInstanceAdmin: true, + companyIds: [], + memberships: [], +}; + +function sendMutatingRequest(app: express.Express, name: string) { + switch (name) { + case "install": + return request(app) + .post("/api/adapters/install") + .send({ packageName: EXTERNAL_PACKAGE_NAME }); + case "disable": + return request(app) + .patch(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}`) + .send({ disabled: true }); + case "override": + return request(app) + .patch("/api/adapters/claude_local/override") + .send({ paused: true }); + case "delete": + return request(app).delete(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}`); + case "reload": + return request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reload`); + case "reinstall": + return request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reinstall`); + default: + throw new Error(`Unknown mutating adapter route: ${name}`); + } +} + +function seedInstalledExternalAdapter() { + mocks.externalRecords.set(EXTERNAL_ADAPTER_TYPE, installedRecord()); + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + registerServerAdapter(createAdapter()); +} + +describe("adapter management route authorization", () => { + beforeAll(async () => { + const [routes, middleware, registry] = await Promise.all([ + import("../routes/adapters.js"), + import("../middleware/index.js"), + import("../adapters/registry.js"), + ]); + adapterRoutes = routes.adapterRoutes; + errorHandler = middleware.errorHandler; + registerServerAdapter = registry.registerServerAdapter; + unregisterServerAdapter = registry.unregisterServerAdapter; + setOverridePaused = registry.setOverridePaused; + }, 20_000); + + beforeEach(() => { + vi.clearAllMocks(); + mocks.externalRecords.clear(); + + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + setOverridePaused("claude_local", false); + mocks.listAdapterPlugins.mockImplementation(() => [...mocks.externalRecords.values()]); + mocks.getAdapterPluginsDir.mockReturnValue("/tmp/paperclip-adapter-route-authz-test"); + mocks.getDisabledAdapterTypes.mockReturnValue([]); + mocks.setAdapterDisabled.mockReturnValue(true); + mocks.buildExternalAdapters.mockResolvedValue([]); + mocks.loadExternalAdapterPackage.mockResolvedValue(createAdapter()); + mocks.reloadExternalAdapter.mockImplementation(async (type: string) => createAdapter(type)); + }); + + afterEach(() => { + unregisterServerAdapter(EXTERNAL_ADAPTER_TYPE); + setOverridePaused("claude_local", false); + }); + + it.each([ + "install", + "disable", + "override", + "delete", + "reload", + "reinstall", + ])("rejects %s for a non-instance-admin board user with company membership", async (routeName) => { + seedInstalledExternalAdapter(); + const app = createApp(boardMember("admin")); + + const res = await sendMutatingRequest(app, routeName); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + }); + + it.each([ + ["install", 201], + ["disable", 200], + ["override", 200], + ["delete", 200], + ["reload", 200], + ["reinstall", 200], + ] as const)("allows instance admins to reach %s", async (routeName, expectedStatus) => { + if (routeName !== "install") { + seedInstalledExternalAdapter(); + } + const app = createApp(instanceAdmin); + + const res = await sendMutatingRequest(app, routeName); + + expect(res.status, JSON.stringify(res.body)).toBe(expectedStatus); + }); + + it.each(["viewer", "operator"] as const)( + "does not let a company %s trigger adapter npm install or reload", + async (membershipRole) => { + seedInstalledExternalAdapter(); + const app = createApp(boardMember(membershipRole)); + + const install = await request(app) + .post("/api/adapters/install") + .send({ packageName: EXTERNAL_PACKAGE_NAME }); + const reload = await request(app).post(`/api/adapters/${EXTERNAL_ADAPTER_TYPE}/reload`); + + expect(install.status, JSON.stringify(install.body)).toBe(403); + expect(reload.status, JSON.stringify(reload.body)).toBe(403); + expect(mocks.execFile).not.toHaveBeenCalled(); + expect(mocks.loadExternalAdapterPackage).not.toHaveBeenCalled(); + expect(mocks.reloadExternalAdapter).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 1d54ff0ff2..6fabde6d42 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -131,7 +131,19 @@ async function createApp() { }; next(); }); - app.use("/api", agentRoutes({} as any)); + const db = { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(async () => [ + { + id: "company-1", + requireBoardApprovalForNewAgents: false, + }, + ]), + })), + })), + }; + app.use("/api", agentRoutes(db as any)); app.use(errorHandler); return app; } diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 978e7c2e24..f1f903073b 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -119,23 +119,25 @@ function registerModuleMocks() { })); } -function createDbStub() { +function createDbStub(options: { requireBoardApprovalForNewAgents?: boolean } = {}) { return { select: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ - then: vi.fn().mockResolvedValue([{ - id: companyId, - name: "Paperclip", - requireBoardApprovalForNewAgents: false, - }]), + then: vi.fn((resolve) => + Promise.resolve(resolve([{ + id: companyId, + name: "Paperclip", + requireBoardApprovalForNewAgents: options.requireBoardApprovalForNewAgents ?? false, + }])), + ), }), }), }), }; } -async function createApp(actor: Record) { +async function createApp(actor: Record, dbOptions: { requireBoardApprovalForNewAgents?: boolean } = {}) { const [{ errorHandler }, { agentRoutes }] = await Promise.all([ vi.importActual("../middleware/index.js"), vi.importActual("../routes/agents.js"), @@ -146,7 +148,7 @@ async function createApp(actor: Record) { (req as any).actor = actor; next(); }); - app.use("/api", agentRoutes(createDbStub() as any)); + app.use("/api", agentRoutes(createDbStub(dbOptions) as any)); app.use(errorHandler); return app; } @@ -398,6 +400,97 @@ describe("agent permission routes", () => { expect(mockLogActivity).not.toHaveBeenCalled(); }); + it("blocks direct agent creation for authenticated company members without agent create permission", async () => { + mockAccessService.canUser.mockResolvedValue(false); + + const app = await createApp({ + type: "board", + userId: "member-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Backdoor", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("agents:create"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("allows direct agent creation for authenticated board users with agent create permission when approval is not required", async () => { + mockAccessService.canUser.mockResolvedValue(true); + + const app = await createApp({ + type: "board", + userId: "agent-admin-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(201); + expect(mockAgentService.create).toHaveBeenCalledWith( + companyId, + expect.objectContaining({ + status: "idle", + }), + ); + expect(mockAccessService.setPrincipalPermission).toHaveBeenCalledWith( + companyId, + "agent", + agentId, + "tasks:assign", + true, + "agent-admin-user", + ); + }); + + it("rejects direct agent creation when new agents require board approval", async () => { + const app = await createApp( + { + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }, + { requireBoardApprovalForNewAgents: true }, + ); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(409); + expect(res.body.error).toContain("/agent-hires"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + expect(mockApprovalService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("grants tasks:assign by default when board creates a new agent", async () => { const app = await createApp({ type: "board", diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index 3faf65e8ea..c833d76673 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -77,6 +77,32 @@ async function createApp(actor: Record) { return app; } +const companyId = "11111111-1111-4111-8111-111111111111"; + +const exportRequest = { + include: { company: true, agents: true, projects: true }, +}; + +function createExportResult() { + return { + rootPath: "paperclip", + manifest: { + agents: [], + skills: [], + projects: [], + issues: [], + envInputs: [], + includes: { company: true, agents: true, projects: true, issues: false, skills: false }, + company: null, + schemaVersion: 1, + generatedAt: "2026-01-01T00:00:00.000Z", + source: null, + }, + files: {}, + warnings: [], + }; +} + describe("company portability routes", () => { beforeEach(() => { vi.resetModules(); @@ -90,30 +116,53 @@ describe("company portability routes", () => { it("rejects non-CEO agents from CEO-safe export preview routes", async () => { mockAgentService.getById.mockResolvedValue({ id: "agent-1", - companyId: "11111111-1111-4111-8111-111111111111", + companyId, role: "engineer", }); const app = await createApp({ type: "agent", agentId: "agent-1", - companyId: "11111111-1111-4111-8111-111111111111", + companyId, source: "agent_key", runId: "run-1", }); const res = await request(app) - .post("/api/companies/11111111-1111-4111-8111-111111111111/exports/preview") - .send({ include: { company: true, agents: true, projects: true } }); + .post(`/api/companies/${companyId}/exports/preview`) + .send(exportRequest); expect(res.status).toBe(403); expect(res.body.error).toContain("Only CEO agents"); expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); }); + it("rejects non-CEO agents from legacy and CEO-safe export bundle routes", async () => { + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId, + role: "engineer", + }); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId, + source: "agent_key", + runId: "run-1", + }); + + for (const path of [`/api/companies/${companyId}/export`, `/api/companies/${companyId}/exports`]) { + const res = await request(app).post(path).send(exportRequest); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Only CEO agents"); + } + expect(mockCompanyPortabilityService.exportBundle).not.toHaveBeenCalled(); + }); + it("allows CEO agents to use company-scoped export preview routes", async () => { mockAgentService.getById.mockResolvedValue({ id: "agent-1", - companyId: "11111111-1111-4111-8111-111111111111", + companyId, role: "ceo", }); mockCompanyPortabilityService.previewExport.mockResolvedValue({ @@ -128,19 +177,64 @@ describe("company portability routes", () => { const app = await createApp({ type: "agent", agentId: "agent-1", - companyId: "11111111-1111-4111-8111-111111111111", + companyId, source: "agent_key", runId: "run-1", }); const res = await request(app) - .post("/api/companies/11111111-1111-4111-8111-111111111111/exports/preview") - .send({ include: { company: true, agents: true, projects: true } }); + .post(`/api/companies/${companyId}/exports/preview`) + .send(exportRequest); expect(res.status).toBe(200); expect(res.body.rootPath).toBe("paperclip"); }); + it("allows CEO agents to export through legacy and CEO-safe bundle routes", async () => { + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId, + role: "ceo", + }); + mockCompanyPortabilityService.exportBundle.mockResolvedValue(createExportResult()); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId, + source: "agent_key", + runId: "run-1", + }); + + for (const path of [`/api/companies/${companyId}/export`, `/api/companies/${companyId}/exports`]) { + const res = await request(app).post(path).send(exportRequest); + + expect(res.status).toBe(200); + expect(res.body.rootPath).toBe("paperclip"); + } + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledTimes(2); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith(1, companyId, exportRequest); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith(2, companyId, exportRequest); + }); + + it("allows board users to export through legacy and CEO-safe bundle routes", async () => { + mockCompanyPortabilityService.exportBundle.mockResolvedValue(createExportResult()); + const app = await createApp({ + type: "board", + userId: "user-1", + companyIds: [companyId], + source: "session", + isInstanceAdmin: false, + }); + + for (const path of [`/api/companies/${companyId}/export`, `/api/companies/${companyId}/exports`]) { + const res = await request(app).post(path).send(exportRequest); + + expect(res.status).toBe(200); + expect(res.body.rootPath).toBe("paperclip"); + } + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledTimes(2); + }); + it("rejects replace collision strategy on CEO-safe import routes", async () => { mockAgentService.getById.mockResolvedValue({ id: "agent-1", diff --git a/server/src/__tests__/costs-service.test.ts b/server/src/__tests__/costs-service.test.ts index 83e4e87b70..1b0677c605 100644 --- a/server/src/__tests__/costs-service.test.ts +++ b/server/src/__tests__/costs-service.test.ts @@ -147,6 +147,13 @@ beforeEach(() => { budgetMonthlyCents: 100, spentMonthlyCents: 0, }); + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + name: "Budget Agent", + budgetMonthlyCents: 100, + spentMonthlyCents: 0, + }); mockAgentService.update.mockResolvedValue({ id: "agent-1", companyId: "company-1", @@ -216,13 +223,6 @@ describe("cost routes", () => { }); it("rejects agent budget updates for board users outside the agent company", async () => { - mockAgentService.getById.mockResolvedValue({ - id: "agent-1", - companyId: "company-1", - name: "Budget Agent", - budgetMonthlyCents: 100, - spentMonthlyCents: 0, - }); const app = await createAppWithActor({ type: "board", userId: "board-user", @@ -238,6 +238,92 @@ describe("cost routes", () => { expect(res.status).toBe(403); expect(mockAgentService.update).not.toHaveBeenCalled(); }); + + it("rejects agent budget updates from the target agent without changing the budget policy", async () => { + const app = await createAppWithActor({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/agents/agent-1/budgets") + .send({ budgetMonthlyCents: 2500 }); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "Board access required" }); + expect(mockAgentService.update).not.toHaveBeenCalled(); + expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("rejects agent budget updates from another same-company agent without changing the budget policy", async () => { + const app = await createAppWithActor({ + type: "agent", + agentId: "agent-2", + companyId: "company-1", + runId: "run-2", + }); + + const res = await request(app) + .patch("/api/agents/agent-1/budgets") + .send({ budgetMonthlyCents: 2500 }); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "Board access required" }); + expect(mockAgentService.update).not.toHaveBeenCalled(); + expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("allows authorized board users to update an agent budget and budget policy", async () => { + mockAgentService.update.mockResolvedValueOnce({ + id: "agent-1", + companyId: "company-1", + name: "Budget Agent", + budgetMonthlyCents: 2500, + spentMonthlyCents: 0, + }); + const app = await createAppWithActor({ + type: "board", + userId: "board-user", + source: "session", + isInstanceAdmin: false, + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + }); + + const res = await request(app) + .patch("/api/agents/agent-1/budgets") + .send({ budgetMonthlyCents: 2500 }); + + expect(res.status).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith("agent-1", { budgetMonthlyCents: 2500 }); + expect(mockBudgetService.upsertPolicy).toHaveBeenCalledWith( + "company-1", + { + scopeType: "agent", + scopeId: "agent-1", + amount: 2500, + windowKind: "calendar_month_utc", + }, + "board-user", + ); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + companyId: "company-1", + actorType: "user", + actorId: "board-user", + agentId: null, + action: "agent.budget_updated", + entityType: "agent", + entityId: "agent-1", + details: { budgetMonthlyCents: 2500 }, + }), + ); + }); }); const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); diff --git a/server/src/__tests__/invite-test-resolution-route.test.ts b/server/src/__tests__/invite-test-resolution-route.test.ts new file mode 100644 index 0000000000..63857948be --- /dev/null +++ b/server/src/__tests__/invite-test-resolution-route.test.ts @@ -0,0 +1,190 @@ +import express from "express"; +import request from "supertest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + accessRoutes, + setInviteResolutionNetworkForTest, +} from "../routes/access.js"; +import { errorHandler } from "../middleware/index.js"; + +function createSelectChain(rows: unknown[]) { + const query = { + then(resolve: (value: unknown[]) => unknown) { + return Promise.resolve(rows).then(resolve); + }, + where() { + return query; + }, + }; + return { + from() { + return query; + }, + }; +} + +function createDbStub(inviteRows: unknown[]) { + return { + select() { + return createSelectChain(inviteRows); + }, + }; +} + +function createInvite(overrides: Record = {}) { + return { + id: "invite-1", + companyId: "company-1", + inviteType: "company_join", + allowedJoinTypes: "agent", + tokenHash: "hash", + defaultsPayload: null, + expiresAt: new Date("2027-03-07T00:10:00.000Z"), + invitedByUserId: null, + revokedAt: null, + acceptedAt: null, + createdAt: new Date("2026-03-07T00:00:00.000Z"), + updatedAt: new Date("2026-03-07T00:00:00.000Z"), + ...overrides, + }; +} + +function createApp(db: Record) { + const app = express(); + app.use((req, _res, next) => { + (req as any).actor = { type: "anon" }; + next(); + }); + app.use( + "/api", + accessRoutes(db as any, { + deploymentMode: "local_trusted", + deploymentExposure: "private", + bindHost: "127.0.0.1", + allowedHostnames: [], + }), + ); + app.use(errorHandler); + return app; +} + +describe("GET /invites/:token/test-resolution", () => { + const lookup = vi.fn(); + const requestHead = vi.fn(); + + beforeEach(() => { + lookup.mockReset(); + requestHead.mockReset(); + setInviteResolutionNetworkForTest({ lookup, requestHead }); + }); + + afterEach(() => { + setInviteResolutionNetworkForTest(null); + }); + + it.each([ + ["localhost", "http://localhost:3100/api/health", "127.0.0.1"], + ["IPv4 loopback", "http://127.0.0.1:3100/api/health", "127.0.0.1"], + ["IPv6 loopback", "http://[::1]:3100/api/health", "::1"], + ["IPv4-mapped IPv6 loopback hex", "http://[::ffff:7f00:1]/api/health", "::ffff:7f00:1"], + ["IPv4-mapped IPv6 RFC1918 hex", "http://[::ffff:c0a8:101]/api/health", "::ffff:c0a8:101"], + ["RFC1918 10/8", "http://10.0.0.5/api/health", "10.0.0.5"], + ["RFC1918 172.16/12", "http://172.16.10.5/api/health", "172.16.10.5"], + ["RFC1918 192.168/16", "http://192.168.1.10/api/health", "192.168.1.10"], + ["link-local metadata", "http://169.254.169.254/latest/meta-data", "169.254.169.254"], + ["multicast", "http://224.0.0.1/probe", "224.0.0.1"], + ["NAT64 well-known prefix", "https://gateway.example.test/health", "64:ff9b::0a00:0001"], + ["NAT64 local-use prefix", "https://gateway.example.test/health", "64:ff9b:1::0a00:0001"], + ])("rejects %s targets before probing", async (_label, url, address) => { + lookup.mockResolvedValue([{ address, family: address.includes(":") ? 6 : 4 }]); + const app = createApp(createDbStub([createInvite()])); + + const res = await request(app) + .get("/api/invites/pcp_invite_test/test-resolution") + .query({ url }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe( + "url resolves to a private, local, multicast, or reserved address", + ); + expect(requestHead).not.toHaveBeenCalled(); + }); + + it("rejects hostnames that resolve to private addresses", async () => { + lookup.mockResolvedValue([{ address: "10.1.2.3", family: 4 }]); + const app = createApp(createDbStub([createInvite()])); + + const res = await request(app) + .get("/api/invites/pcp_invite_test/test-resolution") + .query({ url: "https://gateway.example.test/health" }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe( + "url resolves to a private, local, multicast, or reserved address", + ); + expect(lookup).toHaveBeenCalledWith("gateway.example.test"); + expect(requestHead).not.toHaveBeenCalled(); + }); + + it("rejects hostnames when any resolved address is private", async () => { + lookup.mockResolvedValue([ + { address: "93.184.216.34", family: 4 }, + { address: "127.0.0.1", family: 4 }, + ]); + const app = createApp(createDbStub([createInvite()])); + + const res = await request(app) + .get("/api/invites/pcp_invite_test/test-resolution") + .query({ url: "https://mixed.example.test/health" }); + + expect(res.status).toBe(400); + expect(requestHead).not.toHaveBeenCalled(); + }); + + it("allows public HTTPS targets through the resolved and pinned probe path", async () => { + lookup.mockResolvedValue([{ address: "93.184.216.34", family: 4 }]); + requestHead.mockResolvedValue({ httpStatus: 204 }); + const app = createApp(createDbStub([createInvite()])); + + const res = await request(app) + .get("/api/invites/pcp_invite_test/test-resolution") + .query({ url: "https://gateway.example.test/health", timeoutMs: "2500" }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + inviteId: "invite-1", + requestedUrl: "https://gateway.example.test/health", + timeoutMs: 2500, + status: "reachable", + method: "HEAD", + httpStatus: 204, + }); + expect(requestHead).toHaveBeenCalledWith( + expect.objectContaining({ + resolvedAddress: "93.184.216.34", + resolvedAddresses: ["93.184.216.34"], + hostHeader: "gateway.example.test", + tlsServername: "gateway.example.test", + }), + 2500, + ); + }); + + it.each([ + ["missing invite", []], + ["revoked invite", [createInvite({ revokedAt: new Date("2026-03-07T00:05:00.000Z") })]], + ["expired invite", [createInvite({ expiresAt: new Date("2020-03-07T00:10:00.000Z") })]], + ])("returns not found for %s tokens before DNS lookup", async (_label, inviteRows) => { + const app = createApp(createDbStub(inviteRows)); + + const res = await request(app) + .get("/api/invites/pcp_invite_test/test-resolution") + .query({ url: "https://gateway.example.test/health" }); + + expect(res.status).toBe(404); + expect(res.body.error).toBe("Invite not found"); + expect(lookup).not.toHaveBeenCalled(); + expect(requestHead).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts new file mode 100644 index 0000000000..d3655240d9 --- /dev/null +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -0,0 +1,375 @@ +import { Readable } from "node:stream"; +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { errorHandler } from "../middleware/index.js"; +import { issueRoutes } from "../routes/issues.js"; + +const issueId = "11111111-1111-4111-8111-111111111111"; +const companyId = "22222222-2222-4222-8222-222222222222"; +const ownerAgentId = "33333333-3333-4333-8333-333333333333"; +const peerAgentId = "44444444-4444-4444-8444-444444444444"; +const ownerRunId = "55555555-5555-4555-8555-555555555555"; + +const mockIssueService = vi.hoisted(() => ({ + addComment: vi.fn(), + assertCheckoutOwner: vi.fn(), + getAttachmentById: vi.fn(), + getByIdentifier: vi.fn(), + getById: vi.fn(), + getRelationSummaries: vi.fn(), + getWakeableParentAfterChildCompletion: vi.fn(), + listAttachments: vi.fn(), + listWakeableBlockedDependents: vi.fn(), + remove: vi.fn(), + removeAttachment: vi.fn(), + update: vi.fn(), + findMentionedAgents: vi.fn(), +})); + +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + hasPermission: vi.fn(), +})); + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), + list: vi.fn(), + resolveByReference: vi.fn(), +})); + +const mockDocumentService = vi.hoisted(() => ({ + upsertIssueDocument: vi.fn(), +})); + +const mockWorkProductService = vi.hoisted(() => ({ + getById: vi.fn(), + update: vi.fn(), +})); + +const mockStorageService = vi.hoisted(() => ({ + provider: "local_disk", + putFile: vi.fn(), + getObject: vi.fn(), + headObject: vi.fn(), + deleteObject: vi.fn(), +})); + +vi.mock("@paperclipai/shared/telemetry", () => ({ + trackAgentTaskCompleted: vi.fn(), + trackErrorHandlerCrash: vi.fn(), +})); + +vi.mock("../telemetry.js", () => ({ + getTelemetryClient: vi.fn(() => ({ track: vi.fn() })), +})); + +vi.mock("../services/index.js", () => ({ + accessService: () => mockAccessService, + agentService: () => mockAgentService, + documentService: () => mockDocumentService, + executionWorkspaceService: () => ({}), + feedbackService: () => ({ + listIssueVotesForUser: vi.fn(async () => []), + saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })), + }), + goalService: () => ({}), + heartbeatService: () => ({ + wakeup: vi.fn(async () => undefined), + reportRunActivity: vi.fn(async () => undefined), + getRun: vi.fn(async () => null), + getActiveRunForAgent: vi.fn(async () => null), + cancelRun: vi.fn(async () => null), + }), + instanceSettingsService: () => ({ + get: vi.fn(async () => ({ + id: "instance-settings-1", + general: { + censorUsernameInLogs: false, + feedbackDataSharingPreference: "prompt", + }, + })), + listCompanyIds: vi.fn(async () => [companyId]), + }), + issueApprovalService: () => ({}), + issueService: () => mockIssueService, + logActivity: vi.fn(async () => undefined), + projectService: () => ({}), + routineService: () => ({ + syncRunStatusForIssue: vi.fn(async () => undefined), + }), + workProductService: () => mockWorkProductService, +})); + +function makeIssue(overrides: Record = {}) { + return { + id: issueId, + companyId, + status: "in_progress", + priority: "high", + projectId: null, + goalId: null, + parentId: null, + assigneeAgentId: ownerAgentId, + assigneeUserId: null, + createdByUserId: "board-user", + identifier: "PAP-1649", + title: "Owned active issue", + executionPolicy: null, + executionState: null, + hiddenAt: null, + ...overrides, + }; +} + +function makeAgent(id: string, overrides: Record = {}) { + return { + id, + companyId, + role: "engineer", + reportsTo: null, + permissions: { canCreateAgents: false }, + ...overrides, + }; +} + +function createApp(actor: Record) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = actor; + next(); + }); + app.use("/api", issueRoutes({} as any, mockStorageService as any)); + app.use(errorHandler); + return app; +} + +function peerActor(overrides: Record = {}) { + return { + type: "agent", + agentId: peerAgentId, + companyId, + source: "agent_key", + runId: "66666666-6666-4666-8666-666666666666", + ...overrides, + }; +} + +function ownerActor() { + return { + type: "agent", + agentId: ownerAgentId, + companyId, + source: "agent_key", + runId: ownerRunId, + }; +} + +function boardActor() { + return { + type: "board", + userId: "board-user", + companyIds: [companyId], + source: "local_implicit", + isInstanceAdmin: false, + }; +} + +describe("agent issue mutation checkout ownership", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAccessService.canUser.mockResolvedValue(true); + mockAccessService.hasPermission.mockResolvedValue(false); + mockAgentService.getById.mockImplementation(async (id: string) => { + if (id === ownerAgentId) return makeAgent(ownerAgentId); + if (id === peerAgentId) return makeAgent(peerAgentId); + return null; + }); + mockAgentService.list.mockResolvedValue([ + makeAgent(ownerAgentId), + makeAgent(peerAgentId), + ]); + mockAgentService.resolveByReference.mockResolvedValue({ ambiguous: false, agent: null }); + mockIssueService.getById.mockResolvedValue(makeIssue()); + mockIssueService.getByIdentifier.mockResolvedValue(null); + mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null }); + mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] }); + mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]); + mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null); + mockIssueService.findMentionedAgents.mockResolvedValue([]); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue(), + ...patch, + })); + mockIssueService.addComment.mockResolvedValue({ + id: "77777777-7777-4777-8777-777777777777", + issueId, + companyId, + body: "comment", + }); + mockIssueService.listAttachments.mockResolvedValue([]); + mockIssueService.remove.mockResolvedValue(makeIssue({ status: "cancelled" })); + mockIssueService.getAttachmentById.mockResolvedValue({ + id: "attachment-1", + issueId, + companyId, + objectKey: "issues/attachment-1/report.txt", + contentType: "text/plain", + byteSize: 6, + originalFilename: "report.txt", + }); + mockIssueService.removeAttachment.mockResolvedValue({ + id: "attachment-1", + issueId, + companyId, + objectKey: "issues/attachment-1/report.txt", + }); + mockDocumentService.upsertIssueDocument.mockResolvedValue({ + created: false, + document: { + id: "document-1", + key: "plan", + title: "Plan", + format: "markdown", + latestRevisionNumber: 2, + }, + }); + mockWorkProductService.getById.mockResolvedValue({ + id: "product-1", + issueId, + companyId, + type: "artifact", + }); + mockWorkProductService.update.mockResolvedValue({ + id: "product-1", + issueId, + companyId, + type: "artifact", + title: "Updated", + }); + mockStorageService.putFile.mockResolvedValue({ + provider: "local_disk", + objectKey: "issues/upload.txt", + contentType: "text/plain", + byteSize: 6, + sha256: "sha256", + originalFilename: "upload.txt", + }); + mockStorageService.getObject.mockResolvedValue({ + stream: Readable.from(Buffer.from("report")), + contentLength: 6, + }); + mockStorageService.deleteObject.mockResolvedValue(undefined); + }); + + it.each([ + ["patch", (app: express.Express) => request(app).patch(`/api/issues/${issueId}`).send({ title: "Blocked" })], + ["delete", (app: express.Express) => request(app).delete(`/api/issues/${issueId}`)], + ["comment", (app: express.Express) => request(app).post(`/api/issues/${issueId}/comments`).send({ body: "blocked" })], + [ + "document upsert", + (app: express.Express) => + request(app).put(`/api/issues/${issueId}/documents/plan`).send({ format: "markdown", body: "# blocked" }), + ], + ["work product update", (app: express.Express) => request(app).patch("/api/work-products/product-1").send({ title: "Blocked" })], + [ + "attachment upload", + (app: express.Express) => + request(app) + .post(`/api/companies/${companyId}/issues/${issueId}/attachments`) + .attach("file", Buffer.from("report"), { filename: "report.txt", contentType: "text/plain" }), + ], + ["attachment delete", (app: express.Express) => request(app).delete("/api/attachments/attachment-1")], + ])("rejects peer agent %s on another agent's active checkout", async (_name, sendRequest) => { + const res = await sendRequest(createApp(peerActor())); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.error).toBe("Issue is checked out by another agent"); + expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled(); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockIssueService.addComment).not.toHaveBeenCalled(); + expect(mockDocumentService.upsertIssueDocument).not.toHaveBeenCalled(); + expect(mockWorkProductService.update).not.toHaveBeenCalled(); + expect(mockStorageService.putFile).not.toHaveBeenCalled(); + expect(mockStorageService.deleteObject).not.toHaveBeenCalled(); + }); + + it("allows the checked-out owner with the matching run id to patch and update documents", async () => { + const app = createApp(ownerActor()); + + await request(app).patch(`/api/issues/${issueId}`).send({ title: "Updated" }).expect(200); + await request(app) + .put(`/api/issues/${issueId}/documents/plan`) + .send({ format: "markdown", body: "# updated" }) + .expect(200); + + expect(mockIssueService.assertCheckoutOwner).toHaveBeenCalledWith(issueId, ownerAgentId, ownerRunId); + expect(mockIssueService.update).toHaveBeenCalled(); + expect(mockDocumentService.upsertIssueDocument).toHaveBeenCalledWith( + expect.objectContaining({ + issueId, + key: "plan", + createdByAgentId: ownerAgentId, + createdByRunId: ownerRunId, + }), + ); + }); + + it("preserves board mutations on active checkouts", async () => { + const app = createApp(boardActor()); + + await request(app).patch(`/api/issues/${issueId}`).send({ title: "Board update" }).expect(200); + await request(app) + .put(`/api/issues/${issueId}/documents/plan`) + .send({ format: "markdown", body: "# board" }) + .expect(200); + + expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled(); + expect(mockIssueService.update).toHaveBeenCalled(); + expect(mockDocumentService.upsertIssueDocument).toHaveBeenCalled(); + }); + + it("allows agents with the active-checkout management grant to mutate active checkouts", async () => { + mockAccessService.hasPermission.mockImplementation(async ( + _companyId: string, + _principalType: string, + principalId: string, + permissionKey: string, + ) => principalId === peerAgentId && permissionKey === "tasks:manage_active_checkouts"); + + const res = await request(createApp(peerActor())).patch(`/api/issues/${issueId}`).send({ title: "Managed update" }); + + expect(res.status).toBe(200); + expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled(); + expect(mockIssueService.update).toHaveBeenCalled(); + }); + + it("allows same-company agent mutations when the issue is not in progress", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ status: "todo", assigneeAgentId: ownerAgentId })); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue({ status: "todo", assigneeAgentId: ownerAgentId }), + ...patch, + })); + + const res = await request(createApp(peerActor())).patch(`/api/issues/${issueId}`).send({ title: "Todo update" }); + + expect(res.status).toBe(200); + expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled(); + expect(mockIssueService.update).toHaveBeenCalled(); + }); + + it("allows same-company agent mutations on unassigned in-progress issues", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ assigneeAgentId: null })); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue({ assigneeAgentId: null }), + ...patch, + })); + + const res = await request(createApp(peerActor())).patch(`/api/issues/${issueId}`).send({ title: "Claimable update" }); + + expect(res.status).toBe(200); + expect(mockIssueService.assertCheckoutOwner).not.toHaveBeenCalled(); + expect(mockIssueService.update).toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 95a9281cae..0989a71a3b 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -35,7 +35,12 @@ vi.mock("../services/live-events.js", () => ({ async function createApp( actor: Record, loaderOverrides: Record = {}, - bridgeDeps?: Record, + routeOverrides: { + db?: unknown; + jobDeps?: unknown; + toolDeps?: unknown; + bridgeDeps?: unknown; + } = {}, ) { const [{ pluginRoutes }, { errorHandler }] = await Promise.all([ import("../routes/plugins.js"), @@ -53,12 +58,58 @@ async function createApp( req.actor = actor as typeof req.actor; next(); }); - app.use("/api", pluginRoutes({} as never, loader as never, undefined, undefined, undefined, bridgeDeps as never)); + app.use("/api", pluginRoutes( + (routeOverrides.db ?? {}) as never, + loader as never, + routeOverrides.jobDeps as never, + undefined, + routeOverrides.toolDeps as never, + routeOverrides.bridgeDeps as never, + )); app.use(errorHandler); return { app, loader }; } +function createSelectQueueDb(rows: Array>>) { + return { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: vi.fn(() => Promise.resolve(rows.shift() ?? [])), + })), + })), + })), + }; +} + +const companyA = "22222222-2222-4222-8222-222222222222"; +const companyB = "33333333-3333-4333-8333-333333333333"; +const agentA = "44444444-4444-4444-8444-444444444444"; +const runA = "55555555-5555-4555-8555-555555555555"; +const projectA = "66666666-6666-4666-8666-666666666666"; +const pluginId = "11111111-1111-4111-8111-111111111111"; + +function boardActor(overrides: Record = {}) { + return { + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: false, + companyIds: [companyA], + ...overrides, + }; +} + +function readyPlugin() { + mockRegistry.getById.mockResolvedValue({ + id: pluginId, + pluginKey: "paperclip.example", + version: "1.0.0", + status: "ready", + }); +} + describe("plugin install and upgrade authz", () => { beforeEach(() => { vi.resetAllMocks(); @@ -244,7 +295,7 @@ describe("scoped plugin API routes", () => { companyIds: ["company-1"], }, {}, - { workerManager }, + { bridgeDeps: { workerManager } }, ); const res = await request(app) @@ -265,3 +316,227 @@ describe("scoped plugin API routes", () => { ); }, 20_000); }); + +describe("plugin tool and bridge authz", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("rejects tool execution when the board user cannot access runContext.companyId", async () => { + const executeTool = vi.fn(); + const getTool = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool, + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: {}, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyB, + projectId: projectA, + }, + }); + + expect(res.status).toBe(403); + expect(getTool).not.toHaveBeenCalled(); + expect(executeTool).not.toHaveBeenCalled(); + }); + + it.each([ + [ + "agentId", + [ + [{ companyId: companyB }], + ], + ], + [ + "runId company", + [ + [{ companyId: companyA }], + [{ companyId: companyB, agentId: agentA }], + ], + ], + [ + "runId agent", + [ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: "77777777-7777-4777-8777-777777777777" }], + ], + ], + [ + "projectId", + [ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: agentA }], + [{ companyId: companyB }], + ], + ], + ])("rejects tool execution when runContext.%s is outside the company scope", async (_case, rows) => { + const executeTool = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + db: createSelectQueueDb(rows), + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool: vi.fn(() => ({ name: "paperclip.example:search" })), + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: {}, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + }); + + expect(res.status).toBe(403); + expect(executeTool).not.toHaveBeenCalled(); + }); + + it("allows tool execution when agent, run, and project all belong to runContext.companyId", async () => { + const executeTool = vi.fn().mockResolvedValue({ content: "ok" }); + const { app } = await createApp(boardActor(), {}, { + db: createSelectQueueDb([ + [{ companyId: companyA }], + [{ companyId: companyA, agentId: agentA }], + [{ companyId: companyA }], + ]), + toolDeps: { + toolDispatcher: { + listToolsForAgent: vi.fn(), + getTool: vi.fn(() => ({ name: "paperclip.example:search" })), + executeTool, + }, + }, + }); + + const res = await request(app) + .post("/api/plugins/tools/execute") + .send({ + tool: "paperclip.example:search", + parameters: { q: "test" }, + runContext: { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + }); + + expect(res.status).toBe(200); + expect(executeTool).toHaveBeenCalledWith( + "paperclip.example:search", + { q: "test" }, + { + agentId: agentA, + runId: runA, + companyId: companyA, + projectId: projectA, + }, + ); + }); + + it.each([ + ["legacy data", "post", `/api/plugins/${pluginId}/bridge/data`, { key: "health" }], + ["legacy action", "post", `/api/plugins/${pluginId}/bridge/action`, { key: "sync" }], + ["url data", "post", `/api/plugins/${pluginId}/data/health`, {}], + ["url action", "post", `/api/plugins/${pluginId}/actions/sync`, {}], + ] as const)("rejects %s bridge calls without companyId for non-admin users", async (_name, _method, path, body) => { + readyPlugin(); + const call = vi.fn(); + const { app } = await createApp(boardActor(), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(path) + .send(body); + + expect(res.status).toBe(403); + expect(call).not.toHaveBeenCalled(); + }); + + it("allows omitted-company bridge calls for instance admins as global plugin actions", async () => { + readyPlugin(); + const call = vi.fn().mockResolvedValue({ ok: true }); + const { app } = await createApp(boardActor({ + userId: "admin-1", + isInstanceAdmin: true, + companyIds: [], + }), {}, { + bridgeDeps: { + workerManager: { call }, + }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/actions/sync`) + .send({}); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ data: { ok: true } }); + expect(call).toHaveBeenCalledWith(pluginId, "performAction", { + key: "sync", + params: {}, + renderEnvironment: null, + }); + }); + + it("rejects manual job triggers for non-admin board users", async () => { + const scheduler = { triggerJob: vi.fn() }; + const jobStore = { getJobByIdForPlugin: vi.fn() }; + const { app } = await createApp(boardActor(), {}, { + jobDeps: { scheduler, jobStore }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/jobs/job-1/trigger`) + .send({}); + + expect(res.status).toBe(403); + expect(scheduler.triggerJob).not.toHaveBeenCalled(); + expect(jobStore.getJobByIdForPlugin).not.toHaveBeenCalled(); + }); + + it("allows manual job triggers for instance admins", async () => { + readyPlugin(); + const scheduler = { triggerJob: vi.fn().mockResolvedValue({ runId: "run-1", jobId: "job-1" }) }; + const jobStore = { getJobByIdForPlugin: vi.fn().mockResolvedValue({ id: "job-1" }) }; + const { app } = await createApp(boardActor({ + userId: "admin-1", + isInstanceAdmin: true, + companyIds: [], + }), {}, { + jobDeps: { scheduler, jobStore }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/jobs/job-1/trigger`) + .send({}); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ runId: "run-1", jobId: "job-1" }); + expect(scheduler.triggerJob).toHaveBeenCalledWith("job-1", "manual"); + }); +}); diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index aeebd61510..c4f414d3d3 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -4,7 +4,12 @@ import { randomBytes, timingSafeEqual } from "node:crypto"; +import { lookup as dnsLookup } from "node:dns/promises"; import fs from "node:fs"; +import type { IncomingMessage, RequestOptions as HttpRequestOptions } from "node:http"; +import { request as httpRequest } from "node:http"; +import { request as httpsRequest } from "node:https"; +import { isIP } from "node:net"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { Router } from "express"; @@ -84,6 +89,7 @@ const INVITE_TOKEN_ALPHABET = "abcdefghijklmnopqrstuvwxyz0123456789"; const INVITE_TOKEN_SUFFIX_LENGTH = 8; const INVITE_TOKEN_MAX_RETRIES = 5; const COMPANY_INVITE_TTL_MS = 72 * 60 * 60 * 1000; +const INVITE_RESOLUTION_DNS_TIMEOUT_MS = 3_000; type MemberGrantPayload = { permissionKey: PermissionKey; @@ -2101,44 +2107,259 @@ type InviteResolutionProbe = { message: string; }; +type InviteResolutionLookupResult = { + address: string; + family?: number; +}; + +type ResolvedInviteResolutionTarget = { + url: URL; + resolvedAddress: string; + resolvedAddresses: string[]; + hostHeader: string; + tlsServername?: string; +}; + +type InviteResolutionHeadResponse = { + httpStatus: number | null; +}; + +type InviteResolutionNetwork = { + lookup(hostname: string): Promise; + requestHead( + target: ResolvedInviteResolutionTarget, + timeoutMs: number + ): Promise; +}; + +function parseIpv4Address(address: string) { + const parts = address.split("."); + if (parts.length !== 4) return null; + const parsed = parts.map((part) => { + if (!/^\d+$/.test(part)) return NaN; + return Number(part); + }); + if (parsed.some((part) => !Number.isInteger(part) || part < 0 || part > 255)) { + return null; + } + return parsed as [number, number, number, number]; +} + +function isPrivateOrReservedIpv4(address: string) { + const octets = parseIpv4Address(address); + if (!octets) return true; + const [a, b, c] = octets; + if (a === 0) return true; + if (a === 10) return true; + if (a === 100 && b >= 64 && b <= 127) return true; + if (a === 127) return true; + if (a === 169 && b === 254) return true; + if (a === 172 && b >= 16 && b <= 31) return true; + if (a === 192 && b === 0 && c === 0) return true; + if (a === 192 && b === 168) return true; + if (a === 192 && b === 0 && c === 2) return true; + if (a === 192 && b === 88 && c === 99) return true; + if (a === 198 && (b === 18 || b === 19)) return true; + if (a === 198 && b === 51 && c === 100) return true; + if (a === 203 && b === 0 && c === 113) return true; + if (a >= 224) return true; + return false; +} + +function parseMappedIpv4Hex(address: string) { + const match = address.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); + if (!match) return null; + const hi = Number.parseInt(match[1]!, 16); + const lo = Number.parseInt(match[2]!, 16); + if (!Number.isInteger(hi) || !Number.isInteger(lo)) return null; + return `${hi >> 8}.${hi & 0xff}.${lo >> 8}.${lo & 0xff}`; +} + +function isPrivateOrReservedIpv6(address: string) { + const lower = address.toLowerCase(); + if (lower.startsWith("::ffff:")) { + const mappedIpv4 = lower.match(/^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/); + if (mappedIpv4?.[1]) return isPrivateOrReservedIpv4(mappedIpv4[1]); + const mappedIpv4Hex = parseMappedIpv4Hex(lower); + if (mappedIpv4Hex) return isPrivateOrReservedIpv4(mappedIpv4Hex); + return true; + } + if (lower === "::" || lower === "::1") return true; + if (lower.startsWith("fc") || lower.startsWith("fd")) return true; + if (/^fe[89ab]/.test(lower)) return true; + if (lower.startsWith("ff")) return true; + if (lower === "100::" || lower.startsWith("100:")) return true; + if (lower.startsWith("2001:db8:") || lower === "2001:db8::") return true; + if (lower.startsWith("2001:2:") || lower === "2001:2::") return true; + if (lower.startsWith("2002:")) return true; + if (lower.startsWith("64:ff9b:")) return true; + return false; +} + +function isPublicIpAddress(address: string) { + const ipVersion = isIP(address); + if (ipVersion === 4) return !isPrivateOrReservedIpv4(address); + if (ipVersion === 6) return !isPrivateOrReservedIpv6(address); + return false; +} + +function hostnameForResolution(url: URL) { + return url.hostname.replace(/^\[|\]$/g, ""); +} + +async function defaultInviteResolutionLookup( + hostname: string +): Promise { + return dnsLookup(hostname, { all: true, verbatim: true }); +} + +async function defaultInviteResolutionHeadRequest( + target: ResolvedInviteResolutionTarget, + timeoutMs: number +): Promise { + return new Promise((resolve, reject) => { + const url = target.url; + const request = url.protocol === "https:" ? httpsRequest : httpRequest; + const options: HttpRequestOptions & { servername?: string } = { + protocol: url.protocol, + hostname: target.resolvedAddress, + port: url.port || undefined, + method: "HEAD", + path: `${url.pathname}${url.search}`, + headers: { + Host: target.hostHeader + } + }; + if (target.tlsServername) { + options.servername = target.tlsServername; + } + + let settled = false; + const req = request(options, (response: IncomingMessage) => { + settled = true; + response.resume(); + resolve({ httpStatus: response.statusCode ?? null }); + }); + req.setTimeout(timeoutMs, () => { + if (settled) return; + const error = new Error("Invite resolution probe timed out"); + error.name = "AbortError"; + req.destroy(error); + }); + req.on("error", (error) => { + if (settled) return; + settled = true; + reject(error); + }); + req.end(); + }); +} + +const defaultInviteResolutionNetwork: InviteResolutionNetwork = { + lookup: defaultInviteResolutionLookup, + requestHead: defaultInviteResolutionHeadRequest +}; + +let inviteResolutionNetwork = defaultInviteResolutionNetwork; + +export function setInviteResolutionNetworkForTest( + network: Partial | null +) { + inviteResolutionNetwork = network + ? { ...defaultInviteResolutionNetwork, ...network } + : defaultInviteResolutionNetwork; +} + +async function lookupInviteResolutionHostname(hostname: string) { + let timeout: ReturnType | null = null; + try { + return await Promise.race([ + inviteResolutionNetwork.lookup(hostname), + new Promise((_, reject) => { + timeout = setTimeout( + () => + reject( + badRequest( + `url hostname DNS lookup timed out after ${INVITE_RESOLUTION_DNS_TIMEOUT_MS}ms` + ) + ), + INVITE_RESOLUTION_DNS_TIMEOUT_MS + ); + }) + ]); + } catch (error) { + if (error instanceof Error && "status" in error) throw error; + throw badRequest("url hostname could not be resolved"); + } finally { + if (timeout) clearTimeout(timeout); + } +} + +async function resolveInviteResolutionTarget( + url: URL +): Promise { + const hostname = hostnameForResolution(url); + const results = await lookupInviteResolutionHostname(hostname); + if (results.length === 0) { + throw badRequest("url hostname did not resolve to any addresses"); + } + + const resolvedAddresses = results.map((result) => result.address); + const unsafeAddress = resolvedAddresses.find((address) => !isPublicIpAddress(address)); + if (unsafeAddress) { + throw badRequest( + "url resolves to a private, local, multicast, or reserved address" + ); + } + + return { + url, + resolvedAddress: resolvedAddresses[0]!, + resolvedAddresses, + hostHeader: url.host, + tlsServername: url.protocol === "https:" && isIP(hostname) === 0 + ? hostname + : undefined + }; +} + async function probeInviteResolutionTarget( - url: URL, + target: ResolvedInviteResolutionTarget, timeoutMs: number ): Promise { const startedAt = Date.now(); - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), timeoutMs); try { - const response = await fetch(url, { - method: "HEAD", - redirect: "manual", - signal: controller.signal - }); + const response = await inviteResolutionNetwork.requestHead(target, timeoutMs); const durationMs = Date.now() - startedAt; if ( - response.ok || - response.status === 401 || - response.status === 403 || - response.status === 404 || - response.status === 405 || - response.status === 422 || - response.status === 500 || - response.status === 501 + response.httpStatus !== null && + ( + (response.httpStatus >= 200 && response.httpStatus < 300) || + response.httpStatus === 401 || + response.httpStatus === 403 || + response.httpStatus === 404 || + response.httpStatus === 405 || + response.httpStatus === 422 || + response.httpStatus === 500 || + response.httpStatus === 501 + ) ) { return { status: "reachable", method: "HEAD", durationMs, - httpStatus: response.status, - message: `Webhook endpoint responded to HEAD with HTTP ${response.status}.` + httpStatus: response.httpStatus, + message: `Webhook endpoint responded to HEAD with HTTP ${response.httpStatus}.` }; } return { status: "unreachable", method: "HEAD", durationMs, - httpStatus: response.status, - message: `Webhook endpoint probe returned HTTP ${response.status}.` + httpStatus: response.httpStatus, + message: response.httpStatus === null + ? "Webhook endpoint probe did not return an HTTP status." + : `Webhook endpoint probe returned HTTP ${response.httpStatus}.` }; } catch (error) { const durationMs = Date.now() - startedAt; @@ -2161,8 +2382,6 @@ async function probeInviteResolutionTarget( ? error.message : "Webhook endpoint probe failed." }; - } finally { - clearTimeout(timeout); } } @@ -2927,7 +3146,8 @@ export function accessRoutes( const timeoutMs = Number.isFinite(parsedTimeoutMs) ? Math.max(1000, Math.min(15000, Math.floor(parsedTimeoutMs))) : 5000; - const probe = await probeInviteResolutionTarget(target, timeoutMs); + const resolvedTarget = await resolveInviteResolutionTarget(target); + const probe = await probeInviteResolutionTarget(resolvedTarget, timeoutMs); res.json({ inviteId: invite.id, testResolutionPath: `/api/invites/${token}/test-resolution`, diff --git a/server/src/routes/adapters.ts b/server/src/routes/adapters.ts index a01f3132ba..40e5800ef9 100644 --- a/server/src/routes/adapters.ts +++ b/server/src/routes/adapters.ts @@ -6,7 +6,9 @@ * - Installing external adapters from npm packages or local paths * - Unregistering external adapters * - * All routes require board-level authentication (assertBoard middleware). + * Read-only routes require board org access. Mutating adapter management + * routes require instance-admin access because they can install, reload, or + * toggle server-side adapter code for the whole Paperclip instance. * * @module server/routes/adapters */ @@ -41,7 +43,7 @@ import type { AdapterPluginRecord } from "../services/adapter-plugin-store.js"; import type { ServerAdapterModule, AdapterConfigSchema } from "../adapters/types.js"; import { loadExternalAdapterPackage, getUiParserSource, getOrExtractUiParserSource, reloadExternalAdapter } from "../adapters/plugin-loader.js"; import { logger } from "../middleware/logger.js"; -import { assertBoardOrgAccess } from "./authz.js"; +import { assertBoardOrgAccess, assertInstanceAdmin } from "./authz.js"; import { BUILTIN_ADAPTER_TYPES } from "../adapters/builtin-adapter-types.js"; const execFileAsync = promisify(execFile); @@ -192,6 +194,9 @@ export function adapterRoutes() { * its model count, and load status. */ router.get("/adapters", async (_req, res) => { + // Adapter inventory is needed by ordinary board members when creating or + // editing company agents. Mutating adapter management routes below remain + // instance-admin only because they affect the whole server runtime. assertBoardOrgAccess(_req); const registeredAdapters = listServerAdapters(); @@ -218,7 +223,7 @@ export function adapterRoutes() { * - version?: string — target version for npm packages */ router.post("/adapters/install", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const { packageName, isLocalPath = false, version } = req.body as AdapterInstallRequest; @@ -350,7 +355,7 @@ export function adapterRoutes() { * Request body: { "disabled": boolean } */ router.patch("/adapters/:type", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; const { disabled } = req.body as { disabled?: boolean }; @@ -385,7 +390,7 @@ export function adapterRoutes() { * keep the adapter they started with. */ router.patch("/adapters/:type/override", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; const { paused } = req.body as { paused?: boolean }; @@ -413,7 +418,7 @@ export function adapterRoutes() { * Unregister an external adapter. Built-in adapters cannot be removed. */ router.delete("/adapters/:type", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const adapterType = req.params.type; @@ -488,7 +493,7 @@ export function adapterRoutes() { * Cannot be used on built-in adapter types. */ router.post("/adapters/:type/reload", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const type = req.params.type; @@ -540,7 +545,7 @@ export function adapterRoutes() { // This is a convenience shortcut for remove + install with the same // package name, but without the risk of losing the store record. router.post("/adapters/:type/reinstall", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); const type = req.params.type; @@ -613,6 +618,8 @@ export function adapterRoutes() { const CONFIG_SCHEMA_TTL_MS = 30_000; router.get("/adapters/:type/config-schema", async (req, res) => { + // Config schemas are read-only form metadata used when org members create + // or edit agents; they do not install or execute new adapter code. assertBoardOrgAccess(req); const { type } = req.params; @@ -651,6 +658,8 @@ export function adapterRoutes() { // The adapter package must export a "./ui-parser" entry in package.json // pointing to a self-contained ESM module with zero runtime dependencies. router.get("/adapters/:type/ui-parser.js", (req, res) => { + // UI parsers are read-only assets for displaying existing run output. + // Runtime-changing adapter management routes above require instance admin. assertBoardOrgAccess(req); const { type } = req.params; const source = getOrExtractUiParserSource(type); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 9d0eddc6a0..cad3130f60 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1552,10 +1552,21 @@ export function agentRoutes(db: Db) { router.post("/companies/:companyId/agents", validate(createAgentSchema), async (req, res) => { const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); + await assertCanCreateAgentsForCompany(req, companyId); - if (req.actor.type === "agent") { - assertBoard(req); + const company = await db + .select() + .from(companies) + .where(eq(companies.id, companyId)) + .then((rows) => rows[0] ?? null); + if (!company) { + res.status(404).json({ error: "Company not found" }); + return; + } + if (company.requireBoardApprovalForNewAgents) { + throw conflict( + "Direct agent creation requires board approval. Use POST /api/companies/:companyId/agent-hires to create a pending hire approval.", + ); } const { @@ -1563,6 +1574,14 @@ export function agentRoutes(db: Db) { ...createInput } = req.body; createInput.adapterType = assertKnownAdapterType(createInput.adapterType); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectAgentAdapterWorkspaceCommandPaths(createInput.adapterConfig), + ); + assertNoAgentInstructionsConfigMutation( + req, + (createInput.adapterConfig ?? {}) as Record, + ); const requestedAdapterConfig = applyCreateDefaultsByAdapterType( createInput.adapterType, ((createInput.adapterConfig ?? {}) as Record), diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index 22be5f863c..6516e5efb5 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -164,7 +164,7 @@ export function companyRoutes(db: Db, storage?: StorageService) { router.post("/:companyId/export", validate(companyPortabilityExportSchema), async (req, res) => { const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); + await assertCanManagePortability(req, companyId, "exports"); const result = await portability.exportBundle(companyId, req.body); res.json(result); }); diff --git a/server/src/routes/costs.ts b/server/src/routes/costs.ts index 46c6896fe0..df4452a977 100644 --- a/server/src/routes/costs.ts +++ b/server/src/routes/costs.ts @@ -290,13 +290,7 @@ export function costRoutes(db: Db) { } assertCompanyAccess(req, agent.companyId); - - if (req.actor.type === "agent") { - if (req.actor.agentId !== agentId) { - res.status(403).json({ error: "Agent can only change its own budget" }); - return; - } - } + assertBoard(req); const updated = await agents.update(agentId, { budgetMonthlyCents: req.body.budgetMonthlyCents }); if (!updated) { diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 03a35a47b9..15ffd41dd4 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -407,7 +407,39 @@ export function issueRoutes( return null; } - async function assertAgentRunCheckoutOwnership( + async function hasActiveCheckoutManagementOverride( + actorAgentId: string, + companyId: string, + assigneeAgentId: string, + ) { + const allowedByGrant = await access.hasPermission( + companyId, + "agent", + actorAgentId, + "tasks:manage_active_checkouts", + ); + if (allowedByGrant) return true; + + const companyAgents = await agentsSvc.list(companyId); + const agentsById = new Map(companyAgents.map((agent) => [agent.id, agent])); + const actorAgent = agentsById.get(actorAgentId); + if (!actorAgent) return false; + if (canCreateAgentsLegacy(actorAgent)) return true; + + // Reporting-chain managers may intervene in an agent's active checkout + // without taking the task over. Peers must own the checkout/run first. + let cursor: string | null = assigneeAgentId; + for (let depth = 0; cursor && depth < 50; depth += 1) { + const assignee = agentsById.get(cursor); + if (!assignee) return false; + if (assignee.reportsTo === actorAgentId) return true; + cursor = assignee.reportsTo; + } + + return false; + } + + async function assertAgentIssueMutationAllowed( req: Request, res: Response, issue: { id: string; companyId: string; status: string; assigneeAgentId: string | null }, @@ -418,9 +450,23 @@ export function issueRoutes( res.status(403).json({ error: "Agent authentication required" }); return false; } - if (issue.status !== "in_progress" || issue.assigneeAgentId !== actorAgentId) { + if (issue.status !== "in_progress" || issue.assigneeAgentId === null) { return true; } + if (issue.assigneeAgentId !== actorAgentId) { + if (await hasActiveCheckoutManagementOverride(actorAgentId, issue.companyId, issue.assigneeAgentId)) { + return true; + } + res.status(409).json({ + error: "Issue is checked out by another agent", + details: { + issueId: issue.id, + assigneeAgentId: issue.assigneeAgentId, + actorAgentId, + }, + }); + return false; + } const runId = requireAgentRunId(req, res); if (!runId) return false; const ownership = await svc.assertCheckoutOwner(issue.id, actorAgentId, runId); @@ -725,43 +771,6 @@ export function issueRoutes( res.json(removed); }); - router.get("/issues/:id", async (req, res) => { - const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); - const [{ project, goal }, ancestors, mentionedProjectIds, documentPayload, relations] = await Promise.all([ - resolveIssueProjectAndGoal(issue), - svc.getAncestors(issue.id), - svc.findMentionedProjectIds(issue.id, { includeCommentBodies: false }), - documentsSvc.getIssueDocumentPayload(issue), - svc.getRelationSummaries(issue.id), - ]); - const mentionedProjects = mentionedProjectIds.length > 0 - ? await projectsSvc.listByIds(issue.companyId, mentionedProjectIds) - : []; - const currentExecutionWorkspace = issue.executionWorkspaceId - ? await executionWorkspacesSvc.getById(issue.executionWorkspaceId) - : null; - const workProducts = await workProductsSvc.listForIssue(issue.id); - res.json({ - ...issue, - goalId: goal?.id ?? issue.goalId, - ancestors, - blockedBy: relations.blockedBy, - blocks: relations.blocks, - ...documentPayload, - project: project ?? null, - goal: goal ?? null, - mentionedProjects, - currentExecutionWorkspace, - workProducts, - }); - }); - router.get("/issues/:id/heartbeat-context", async (req, res) => { const id = req.params.id as string; const issue = await svc.getById(id); @@ -854,6 +863,43 @@ export function issueRoutes( }); }); + router.get("/issues/:id", async (req, res) => { + const id = req.params.id as string; + const issue = await svc.getById(id); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + assertCompanyAccess(req, issue.companyId); + const [{ project, goal }, ancestors, mentionedProjectIds, documentPayload, relations] = await Promise.all([ + resolveIssueProjectAndGoal(issue), + svc.getAncestors(issue.id), + svc.findMentionedProjectIds(issue.id, { includeCommentBodies: false }), + documentsSvc.getIssueDocumentPayload(issue), + svc.getRelationSummaries(issue.id), + ]); + const mentionedProjects = mentionedProjectIds.length > 0 + ? await projectsSvc.listByIds(issue.companyId, mentionedProjectIds) + : []; + const currentExecutionWorkspace = issue.executionWorkspaceId + ? await executionWorkspacesSvc.getById(issue.executionWorkspaceId) + : null; + const workProducts = await workProductsSvc.listForIssue(issue.id); + res.json({ + ...issue, + goalId: goal?.id ?? issue.goalId, + ancestors, + blockedBy: relations.blockedBy, + blocks: relations.blocks, + ...documentPayload, + project: project ?? null, + goal: goal ?? null, + mentionedProjects, + currentExecutionWorkspace, + workProducts, + }); + }); + router.get("/issues/:id/work-products", async (req, res) => { const id = req.params.id as string; const issue = await svc.getById(id); @@ -909,6 +955,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { res.status(400).json({ error: "Invalid document key", details: keyParsed.error.issues }); @@ -980,6 +1027,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { res.status(400).json({ error: "Invalid document key", details: keyParsed.error.issues }); @@ -1068,6 +1116,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const product = await workProductsSvc.createForIssue(issue.id, issue.companyId, { ...req.body, projectId: req.body.projectId ?? issue.projectId ?? null, @@ -1099,6 +1148,12 @@ export function issueRoutes( return; } assertCompanyAccess(req, existing.companyId); + const issue = await svc.getById(existing.issueId); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const product = await workProductsSvc.update(id, req.body); if (!product) { res.status(404).json({ error: "Work product not found" }); @@ -1127,6 +1182,12 @@ export function issueRoutes( return; } assertCompanyAccess(req, existing.companyId); + const issue = await svc.getById(existing.issueId); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const removed = await workProductsSvc.remove(id); if (!removed) { res.status(404).json({ error: "Work product not found" }); @@ -1294,6 +1355,8 @@ export function issueRoutes( res.status(404).json({ error: "Issue not found" }); return; } + assertCompanyAccess(req, issue.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertCanManageIssueApprovalLinks(req, res, issue.companyId))) return; const actor = getActorInfo(req); @@ -1326,6 +1389,8 @@ export function issueRoutes( res.status(404).json({ error: "Issue not found" }); return; } + assertCompanyAccess(req, issue.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertCanManageIssueApprovalLinks(req, res, issue.companyId))) return; await issueApprovalsSvc.unlink(id, approvalId); @@ -1457,7 +1522,7 @@ export function issueRoutes( } assertCompanyAccess(req, existing.companyId); assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body)); - if (!(await assertAgentRunCheckoutOwnership(req, res, existing))) return; + if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const actor = getActorInfo(req); const isClosed = isClosedIssueStatus(existing.status); @@ -2053,6 +2118,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, existing.companyId); + if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const attachments = await svc.listAttachments(id); const issue = await svc.remove(id); @@ -2166,7 +2232,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, existing.companyId); - if (!(await assertAgentRunCheckoutOwnership(req, res, existing))) return; + if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const actorRunId = requireAgentRunId(req, res); if (req.actor.type === "agent" && !actorRunId) return; @@ -2255,7 +2321,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); - if (!(await assertAgentRunCheckoutOwnership(req, res, issue))) return; + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const comment = await svc.getComment(commentId); if (!comment || comment.issueId !== id) { @@ -2400,7 +2466,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, issue.companyId); - if (!(await assertAgentRunCheckoutOwnership(req, res, issue))) return; + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue); if (closedExecutionWorkspace) { respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); @@ -2730,6 +2796,7 @@ export function issueRoutes( res.status(422).json({ error: "Issue does not belong to company" }); return; } + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; try { await runSingleFileUpload(req, res); @@ -2840,6 +2907,12 @@ export function issueRoutes( return; } assertCompanyAccess(req, attachment.companyId); + const issue = await svc.getById(attachment.issueId); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; try { await storage.deleteObject(attachment.companyId, attachment.objectKey); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 19a6739161..43a6e7dd25 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -26,7 +26,14 @@ import { Router } from "express"; import type { Request, Response } from "express"; import { and, desc, eq, gte } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { companies, pluginLogs, pluginWebhookDeliveries } from "@paperclipai/db"; +import { + agents, + companies, + heartbeatRuns, + pluginLogs, + pluginWebhookDeliveries, + projects, +} from "@paperclipai/db"; import type { PluginApiRouteDeclaration, PluginStatus, @@ -59,7 +66,7 @@ import { getActorInfo, } from "./authz.js"; import { validateInstanceConfig } from "../services/plugin-config-validator.js"; -import { forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; +import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; /** UI slot declaration extracted from plugin manifest */ type PluginUiSlotDeclaration = NonNullable["slots"]>[number]; @@ -548,6 +555,52 @@ export function pluginRoutes( }))); } + function assertPluginBridgeScope(req: Request, companyId: unknown): string | undefined { + if (companyId === undefined || companyId === null) { + assertInstanceAdmin(req); + return undefined; + } + if (typeof companyId !== "string" || companyId.trim().length === 0) { + throw badRequest('"companyId" must be a non-empty string when provided'); + } + assertCompanyAccess(req, companyId); + return companyId; + } + + async function validateToolRunContextScope(runContext: ToolRunContext): Promise { + const [agent] = await db + .select({ companyId: agents.companyId }) + .from(agents) + .where(eq(agents.id, runContext.agentId)) + .limit(1); + if (!agent || agent.companyId !== runContext.companyId) { + return '"runContext.agentId" does not belong to "runContext.companyId"'; + } + + const [run] = await db + .select({ companyId: heartbeatRuns.companyId, agentId: heartbeatRuns.agentId }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runContext.runId)) + .limit(1); + if (!run || run.companyId !== runContext.companyId) { + return '"runContext.runId" does not belong to "runContext.companyId"'; + } + if (run.agentId !== runContext.agentId) { + return '"runContext.runId" does not belong to "runContext.agentId"'; + } + + const [project] = await db + .select({ companyId: projects.companyId }) + .from(projects) + .where(eq(projects.id, runContext.projectId)) + .limit(1); + if (!project || project.companyId !== runContext.companyId) { + return '"runContext.projectId" does not belong to "runContext.companyId"'; + } + + return null; + } + /** * GET /api/plugins * @@ -742,6 +795,11 @@ export function pluginRoutes( } assertCompanyAccess(req, runContext.companyId); + const scopeError = await validateToolRunContextScope(runContext); + if (scopeError) { + res.status(403).json({ error: scopeError }); + return; + } // Verify the tool exists const registeredTool = toolDeps.toolDispatcher.getTool(tool); @@ -1020,9 +1078,7 @@ export function pluginRoutes( return; } - if (body.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1103,9 +1159,7 @@ export function pluginRoutes( return; } - if (body.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1186,9 +1240,7 @@ export function pluginRoutes( renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } | undefined; - if (body?.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body?.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -1265,9 +1317,7 @@ export function pluginRoutes( renderEnvironment?: PluginLauncherRenderContextSnapshot | null; } | undefined; - if (body?.companyId) { - assertCompanyAccess(req, body.companyId); - } + assertPluginBridgeScope(req, body?.companyId); try { const result = await bridgeDeps.workerManager.call( @@ -2140,7 +2190,7 @@ export function pluginRoutes( * - 400 if job not found, not active, already running, or worker unavailable */ router.post("/plugins/:pluginId/jobs/:jobId/trigger", async (req, res) => { - assertBoardOrgAccess(req); + assertInstanceAdmin(req); if (!jobDeps) { res.status(501).json({ error: "Job scheduling is not enabled" }); return; diff --git a/tests/e2e/signoff-policy.spec.ts b/tests/e2e/signoff-policy.spec.ts index a2cde2c385..eab54a3676 100644 --- a/tests/e2e/signoff-policy.spec.ts +++ b/tests/e2e/signoff-policy.spec.ts @@ -163,9 +163,9 @@ async function setupCompany(boardRequest: APIRequestContext): Promise { - const agentRes = await boardRequest.post(`${BASE_URL}/api/companies/${companyId}/agents`, { + const agentRes = await boardRequest.post(`${BASE_URL}/api/companies/${companyId}/agent-hires`, { data: { name, role, @@ -178,7 +178,14 @@ async function setupCompany(boardRequest: APIRequestContext): Promise - api.post(`/companies/${companyId}/export`, data), + api.post(`/companies/${companyId}/exports`, data), exportPreview: ( companyId: string, data: CompanyPortabilityExportRequest, ) => api.post(`/companies/${companyId}/exports/preview`, data), - exportPackage: ( - companyId: string, - data: CompanyPortabilityExportRequest, - ) => - api.post(`/companies/${companyId}/exports`, data), importPreview: (data: CompanyPortabilityPreviewRequest) => api.post("/companies/import/preview", data), importBundle: (data: CompanyPortabilityImportRequest) => diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 413c98e21d..9ec608f337 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -7,6 +7,7 @@ import { useCompany } from "../context/CompanyContext"; import { companiesApi } from "../api/companies"; import { goalsApi } from "../api/goals"; import { agentsApi } from "../api/agents"; +import { approvalsApi } from "../api/approvals"; import { issuesApi } from "../api/issues"; import { projectsApi } from "../api/projects"; import { queryKeys } from "../lib/queryKeys"; @@ -458,13 +459,23 @@ export function OnboardingWizard() { if (!result) return; } - const agent = await agentsApi.create(createdCompanyId, { + const hire = await agentsApi.hire(createdCompanyId, { name: agentName.trim(), role: "ceo", adapterType, adapterConfig: buildAdapterConfig(), runtimeConfig: buildNewAgentRuntimeConfig() }); + if (hire.approval) { + await approvalsApi.approve( + hire.approval.id, + "Approved during onboarding first-agent setup." + ); + queryClient.invalidateQueries({ + queryKey: queryKeys.approvals.list(createdCompanyId) + }); + } + const agent = hire.agent; setCreatedAgentId(agent.id); queryClient.invalidateQueries({ queryKey: queryKeys.agents.list(createdCompanyId) diff --git a/ui/src/pages/CompanyAccess.tsx b/ui/src/pages/CompanyAccess.tsx index 51ff8d69c7..b1ba9aa581 100644 --- a/ui/src/pages/CompanyAccess.tsx +++ b/ui/src/pages/CompanyAccess.tsx @@ -33,6 +33,7 @@ const permissionLabels: Record = { "users:manage_permissions": "Manage members and grants", "tasks:assign": "Assign tasks", "tasks:assign_scope": "Assign scoped tasks", + "tasks:manage_active_checkouts": "Manage active task checkouts", "joins:approve": "Approve join requests", }; diff --git a/ui/src/pages/CompanyExport.tsx b/ui/src/pages/CompanyExport.tsx index 8dc87d3efc..0b910e7c8c 100644 --- a/ui/src/pages/CompanyExport.tsx +++ b/ui/src/pages/CompanyExport.tsx @@ -727,7 +727,7 @@ export function CompanyExport() { const downloadMutation = useMutation({ mutationFn: () => - companiesApi.exportPackage(selectedCompanyId!, { + companiesApi.exportBundle(selectedCompanyId!, { include: { company: true, agents: true, projects: true, issues: true }, selectedFiles: Array.from(checkedFiles).sort(), sidebarOrder,