diff --git a/packages/shared/src/validators/approval.ts b/packages/shared/src/validators/approval.ts index d6a6cf0d..ae6b2eb3 100644 --- a/packages/shared/src/validators/approval.ts +++ b/packages/shared/src/validators/approval.ts @@ -12,14 +12,12 @@ export type CreateApproval = z.infer; export const resolveApprovalSchema = z.object({ decisionNote: z.string().optional().nullable(), - decidedByUserId: z.string().optional().default("board"), }); export type ResolveApproval = z.infer; export const requestApprovalRevisionSchema = z.object({ decisionNote: z.string().optional().nullable(), - decidedByUserId: z.string().optional().default("board"), }); export type RequestApprovalRevision = z.infer; diff --git a/server/src/__tests__/activity-routes.test.ts b/server/src/__tests__/activity-routes.test.ts index e8b50dba..974fcb2a 100644 --- a/server/src/__tests__/activity-routes.test.ts +++ b/server/src/__tests__/activity-routes.test.ts @@ -28,7 +28,15 @@ vi.mock("../services/index.js", () => ({ heartbeatService: () => mockHeartbeatService, })); -async function createApp() { +async function createApp( + actor: Record = { + type: "board", + userId: "user-1", + companyIds: ["company-1"], + source: "session", + isInstanceAdmin: false, + }, +) { const [{ errorHandler }, { activityRoutes }] = await Promise.all([ import("../middleware/index.js"), import("../routes/activity.js"), @@ -36,13 +44,7 @@ async function createApp() { const app = express(); app.use(express.json()); app.use((req, _res, next) => { - (req as any).actor = { - type: "board", - userId: "user-1", - companyIds: ["company-1"], - source: "session", - isInstanceAdmin: false, - }; + (req as any).actor = actor; next(); }); app.use("/api", activityRoutes({} as any)); @@ -105,4 +107,13 @@ describe("activity routes", () => { expect(res.status).toBe(403); expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); }); + + it("rejects anonymous heartbeat run issue lookups before run existence checks", async () => { + const app = await createApp({ type: "none", source: "none" }); + const res = await request(app).get("/api/heartbeat-runs/missing-run/issues"); + + expect(res.status).toBe(401); + expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); + expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts new file mode 100644 index 00000000..34b7dec4 --- /dev/null +++ b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts @@ -0,0 +1,267 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { errorHandler } from "../middleware/index.js"; +import { agentRoutes } from "../routes/agents.js"; + +const agentId = "11111111-1111-4111-8111-111111111111"; +const companyId = "22222222-2222-4222-8222-222222222222"; +const keyId = "33333333-3333-4333-8333-333333333333"; + +const baseAgent = { + id: agentId, + companyId, + name: "Builder", + urlKey: "builder", + role: "engineer", + title: "Builder", + icon: null, + status: "idle", + reportsTo: null, + capabilities: null, + adapterType: "process", + adapterConfig: {}, + runtimeConfig: {}, + budgetMonthlyCents: 0, + spentMonthlyCents: 0, + pauseReason: null, + pausedAt: null, + permissions: { canCreateAgents: false }, + lastHeartbeatAt: null, + metadata: null, + createdAt: new Date("2026-04-11T00:00:00.000Z"), + updatedAt: new Date("2026-04-11T00:00:00.000Z"), +}; + +const baseKey = { + id: keyId, + agentId, + companyId, + name: "exploit", + createdAt: new Date("2026-04-11T00:00:00.000Z"), + revokedAt: null, +}; + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), + pause: vi.fn(), + resume: vi.fn(), + terminate: vi.fn(), + remove: vi.fn(), + listKeys: vi.fn(), + createApiKey: vi.fn(), + getKeyById: vi.fn(), + revokeKey: vi.fn(), +})); + +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + hasPermission: vi.fn(), + getMembership: vi.fn(), + ensureMembership: vi.fn(), + listPrincipalGrants: vi.fn(), + setPrincipalPermission: vi.fn(), +})); + +const mockApprovalService = vi.hoisted(() => ({ + create: vi.fn(), + getById: vi.fn(), +})); + +const mockBudgetService = vi.hoisted(() => ({ + upsertPolicy: vi.fn(), +})); + +const mockHeartbeatService = vi.hoisted(() => ({ + cancelActiveForAgent: vi.fn(), +})); + +const mockIssueApprovalService = vi.hoisted(() => ({ + linkManyForApproval: vi.fn(), +})); + +const mockIssueService = vi.hoisted(() => ({ + list: vi.fn(), +})); + +const mockSecretService = vi.hoisted(() => ({ + normalizeAdapterConfigForPersistence: vi.fn(), + resolveAdapterConfigForRuntime: vi.fn(), +})); + +const mockAgentInstructionsService = vi.hoisted(() => ({ + materializeManagedBundle: vi.fn(), +})); + +const mockCompanySkillService = vi.hoisted(() => ({ + listRuntimeSkillEntries: vi.fn(), + resolveRequestedSkillKeys: vi.fn(), +})); + +const mockWorkspaceOperationService = vi.hoisted(() => ({})); +const mockLogActivity = vi.hoisted(() => vi.fn()); +const mockGetTelemetryClient = vi.hoisted(() => vi.fn()); + +vi.mock("@paperclipai/shared/telemetry", () => ({ + trackAgentCreated: vi.fn(), + trackErrorHandlerCrash: vi.fn(), +})); + +vi.mock("../telemetry.js", () => ({ + getTelemetryClient: mockGetTelemetryClient, +})); + +vi.mock("../services/index.js", () => ({ + agentService: () => mockAgentService, + agentInstructionsService: () => mockAgentInstructionsService, + accessService: () => mockAccessService, + approvalService: () => mockApprovalService, + companySkillService: () => mockCompanySkillService, + budgetService: () => mockBudgetService, + heartbeatService: () => mockHeartbeatService, + issueApprovalService: () => mockIssueApprovalService, + issueService: () => mockIssueService, + logActivity: mockLogActivity, + secretService: () => mockSecretService, + syncInstructionsBundleConfigFromFilePath: vi.fn((_agent, config) => config), + workspaceOperationService: () => mockWorkspaceOperationService, +})); + +vi.mock("../services/instance-settings.js", () => ({ + instanceSettingsService: () => ({ + getGeneral: vi.fn(async () => ({ censorUsernameInLogs: false })), + }), +})); + +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", agentRoutes({} as any)); + app.use(errorHandler); + return app; +} + +describe("agent cross-tenant route authorization", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); + mockAgentService.getById.mockResolvedValue(baseAgent); + mockAgentService.pause.mockResolvedValue(baseAgent); + mockAgentService.resume.mockResolvedValue(baseAgent); + mockAgentService.terminate.mockResolvedValue(baseAgent); + mockAgentService.remove.mockResolvedValue(baseAgent); + mockAgentService.listKeys.mockResolvedValue([]); + mockAgentService.createApiKey.mockResolvedValue({ + id: keyId, + name: baseKey.name, + token: "pcp_test_token", + createdAt: baseKey.createdAt, + }); + mockAgentService.getKeyById.mockResolvedValue(baseKey); + mockAgentService.revokeKey.mockResolvedValue({ + ...baseKey, + revokedAt: new Date("2026-04-11T00:05:00.000Z"), + }); + mockHeartbeatService.cancelActiveForAgent.mockResolvedValue(undefined); + mockLogActivity.mockResolvedValue(undefined); + }); + + it("rejects cross-tenant board pause before mutating the agent", async () => { + const app = createApp({ + type: "board", + userId: "mallory", + companyIds: [], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app).post(`/api/agents/${agentId}/pause`).send({}); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("User does not have access to this company"); + expect(mockAgentService.getById).toHaveBeenCalledWith(agentId); + expect(mockAgentService.pause).not.toHaveBeenCalled(); + expect(mockHeartbeatService.cancelActiveForAgent).not.toHaveBeenCalled(); + }); + + it("rejects cross-tenant board key listing before reading any keys", async () => { + const app = createApp({ + type: "board", + userId: "mallory", + companyIds: [], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app).get(`/api/agents/${agentId}/keys`); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("User does not have access to this company"); + expect(mockAgentService.getById).toHaveBeenCalledWith(agentId); + expect(mockAgentService.listKeys).not.toHaveBeenCalled(); + }); + + it("rejects cross-tenant board key creation before minting a token", async () => { + const app = createApp({ + type: "board", + userId: "mallory", + companyIds: [], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .post(`/api/agents/${agentId}/keys`) + .send({ name: "exploit" }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("User does not have access to this company"); + expect(mockAgentService.getById).toHaveBeenCalledWith(agentId); + expect(mockAgentService.createApiKey).not.toHaveBeenCalled(); + }); + + it("rejects cross-tenant board key revocation before touching the key", async () => { + const app = createApp({ + type: "board", + userId: "mallory", + companyIds: [], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app).delete(`/api/agents/${agentId}/keys/${keyId}`); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("User does not have access to this company"); + expect(mockAgentService.getById).toHaveBeenCalledWith(agentId); + expect(mockAgentService.getKeyById).not.toHaveBeenCalled(); + expect(mockAgentService.revokeKey).not.toHaveBeenCalled(); + }); + + it("requires the key to belong to the route agent before revocation", async () => { + mockAgentService.getKeyById.mockResolvedValue({ + ...baseKey, + agentId: "44444444-4444-4444-8444-444444444444", + }); + mockAccessService.canUser.mockResolvedValue(true); + + const app = createApp({ + type: "board", + userId: "board-user", + companyIds: [companyId], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app).delete(`/api/agents/${agentId}/keys/${keyId}`); + + expect(res.status).toBe(404); + expect(res.body.error).toContain("Key not found"); + expect(mockAgentService.getKeyById).toHaveBeenCalledWith(keyId); + expect(mockAgentService.revokeKey).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index af9f7658..bbb0c2a9 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -205,6 +205,196 @@ describe("agent permission routes", () => { mockLogActivity.mockResolvedValue(undefined); }); + it("redacts agent detail for authenticated company members without agent admin 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).get(`/api/agents/${agentId}`); + + expect(res.status).toBe(200); + expect(res.body.adapterConfig).toEqual({}); + expect(res.body.runtimeConfig).toEqual({}); + }); + + it("redacts company agent list for authenticated company members without agent admin 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).get(`/api/companies/${companyId}/agents`); + + expect(res.status).toBe(200); + expect(res.body).toEqual([ + expect.objectContaining({ + id: agentId, + adapterConfig: {}, + runtimeConfig: {}, + }), + ]); + }); + + it("blocks agent updates for authenticated company members without agent admin 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) + .patch(`/api/agents/${agentId}`) + .send({ title: "Compromised" }); + + expect(res.status).toBe(403); + }); + + it("blocks api key creation for authenticated company members without agent admin 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/agents/${agentId}/keys`) + .send({ name: "backdoor" }); + + expect(res.status).toBe(403); + }); + + it("blocks wakeups for authenticated company members without agent admin 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/agents/${agentId}/wakeup`) + .send({}); + + expect(res.status).toBe(403); + }); + + it("blocks agent-authenticated self-updates that set host-executed workspace commands", async () => { + const app = await createApp({ + type: "agent", + agentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/agents/${agentId}`) + .send({ + adapterConfig: { + workspaceStrategy: { + type: "git_worktree", + provisionCommand: "touch /tmp/paperclip-rce", + }, + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("blocks agent-authenticated self-updates that set instructions bundle roots", async () => { + const app = await createApp({ + type: "agent", + agentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/agents/${agentId}`) + .send({ + adapterConfig: { + instructionsRootPath: "/etc", + instructionsEntryFile: "passwd", + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("instructions path or bundle configuration"); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("blocks agent-authenticated instructions-path updates", async () => { + const app = await createApp({ + type: "agent", + agentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/agents/${agentId}/instructions-path`) + .send({ path: "/etc/passwd" }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("instructions path or bundle configuration"); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("blocks agent-authenticated hires that set instructions bundle config", async () => { + mockAccessService.hasPermission.mockResolvedValue(true); + + const app = await createApp({ + type: "agent", + agentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/agent-hires`) + .send({ + name: "Injected", + role: "engineer", + adapterType: "codex_local", + adapterConfig: { + instructionsRootPath: "/etc", + instructionsEntryFile: "passwd", + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("instructions path or bundle configuration"); + expect(mockAgentService.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__/app-private-hostname-gate.test.ts b/server/src/__tests__/app-private-hostname-gate.test.ts new file mode 100644 index 00000000..cf60a1b5 --- /dev/null +++ b/server/src/__tests__/app-private-hostname-gate.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { shouldEnablePrivateHostnameGuard } from "../app.ts"; + +describe("shouldEnablePrivateHostnameGuard", () => { + it("enables the hostname guard for local_trusted private deployments", () => { + expect(shouldEnablePrivateHostnameGuard({ + deploymentMode: "local_trusted", + deploymentExposure: "private", + })).toBe(true); + }); + + it("does not enable the hostname guard for local_trusted public deployments", () => { + expect(shouldEnablePrivateHostnameGuard({ + deploymentMode: "local_trusted", + deploymentExposure: "public", + })).toBe(false); + }); + + it("enables the hostname guard for authenticated private deployments", () => { + expect(shouldEnablePrivateHostnameGuard({ + deploymentMode: "authenticated", + deploymentExposure: "private", + })).toBe(true); + }); + + it("does not enable the hostname guard for authenticated public deployments", () => { + expect(shouldEnablePrivateHostnameGuard({ + deploymentMode: "authenticated", + deploymentExposure: "public", + })).toBe(false); + }); +}); diff --git a/server/src/__tests__/approval-routes-idempotency.test.ts b/server/src/__tests__/approval-routes-idempotency.test.ts index 338bfe32..d87561a9 100644 --- a/server/src/__tests__/approval-routes-idempotency.test.ts +++ b/server/src/__tests__/approval-routes-idempotency.test.ts @@ -196,6 +196,90 @@ describe("approval routes idempotent retries", () => { expect(mockApprovalService.requestRevision).not.toHaveBeenCalled(); }); + it("derives approval attribution from the authenticated actor on approve", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-4", + companyId: "company-1", + type: "hire_agent", + status: "pending", + payload: {}, + requestedByAgentId: null, + }); + mockApprovalService.approve.mockResolvedValue({ + approval: { + id: "approval-4", + companyId: "company-1", + type: "hire_agent", + status: "approved", + payload: {}, + requestedByAgentId: null, + }, + applied: true, + }); + + const res = await request(await createApp()) + .post("/api/approvals/approval-4/approve") + .send({ decidedByUserId: "forged-user", decisionNote: "ship it" }); + + expect(res.status).toBe(200); + expect(mockApprovalService.approve).toHaveBeenCalledWith("approval-4", "user-1", "ship it"); + }); + + it("derives approval attribution from the authenticated actor on reject", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-5", + companyId: "company-1", + type: "hire_agent", + status: "pending", + payload: {}, + }); + mockApprovalService.reject.mockResolvedValue({ + approval: { + id: "approval-5", + companyId: "company-1", + type: "hire_agent", + status: "rejected", + payload: {}, + }, + applied: true, + }); + + const res = await request(await createApp()) + .post("/api/approvals/approval-5/reject") + .send({ decidedByUserId: "forged-user", decisionNote: "not now" }); + + expect(res.status).toBe(200); + expect(mockApprovalService.reject).toHaveBeenCalledWith("approval-5", "user-1", "not now"); + }); + + it("derives approval attribution from the authenticated actor on request revision", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-6", + companyId: "company-1", + type: "hire_agent", + status: "pending", + payload: {}, + }); + mockApprovalService.requestRevision.mockResolvedValue({ + id: "approval-6", + companyId: "company-1", + type: "hire_agent", + status: "revision_requested", + payload: {}, + }); + + const res = await request(await createApp()) + .post("/api/approvals/approval-6/request-revision") + .send({ decidedByUserId: "forged-user", decisionNote: "Need changes" }); + + expect(res.status).toBe(200); + expect(mockApprovalService.requestRevision).toHaveBeenCalledWith( + "approval-6", + "user-1", + "Need changes", + ); + }); + it("lets agents create generic issue-linked board approval requests", async () => { mockApprovalService.create.mockResolvedValue({ id: "approval-1", diff --git a/server/src/__tests__/cli-auth-routes.test.ts b/server/src/__tests__/cli-auth-routes.test.ts index 006321d2..121d959d 100644 --- a/server/src/__tests__/cli-auth-routes.test.ts +++ b/server/src/__tests__/cli-auth-routes.test.ts @@ -45,7 +45,7 @@ function registerModuleMocks() { })); } -async function createApp(actor: any) { +async function createApp(actor: any, db: any = {} as any) { const [{ accessRoutes }, { errorHandler }] = await Promise.all([ vi.importActual("../routes/access.js"), vi.importActual("../middleware/index.js"), @@ -58,7 +58,7 @@ async function createApp(actor: any) { }); app.use( "/api", - accessRoutes({} as any, { + accessRoutes(db, { deploymentMode: "authenticated", deploymentExposure: "private", bindHost: "127.0.0.1", @@ -101,14 +101,56 @@ describe("cli auth routes", () => { expect(res.body).toMatchObject({ id: "challenge-1", token: "pcp_cli_auth_secret", - boardApiToken: "pcp_board_token", approvalPath: "/cli-auth/challenge-1?token=pcp_cli_auth_secret", pollPath: "/cli-auth/challenges/challenge-1", expiresAt: "2026-03-23T13:00:00.000Z", }); + expect(res.body.boardApiToken).toBe("pcp_board_token"); expect(res.body.approvalUrl).toContain("/cli-auth/challenge-1?token=pcp_cli_auth_secret"); }); + it("rejects anonymous access to generic skill documents", async () => { + const app = await createApp({ type: "none", source: "none" }); + const [indexRes, skillRes] = await Promise.all([ + request(app).get("/api/skills/index"), + request(app).get("/api/skills/paperclip"), + ]); + + expect(indexRes.status).toBe(401); + expect(skillRes.status).toBe(401); + }); + + it("serves the invite-scoped paperclip skill anonymously for active invites", async () => { + const invite = { + id: "invite-1", + companyId: "company-1", + inviteType: "company_join", + allowedJoinTypes: "agent", + tokenHash: "hash", + defaultsPayload: null, + expiresAt: new Date(Date.now() + 60_000), + invitedByUserId: null, + revokedAt: null, + acceptedAt: null, + createdAt: new Date(), + updatedAt: new Date(), + }; + const db = { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn().mockResolvedValue([invite]), + })), + })), + }; + + const app = await createApp({ type: "none", source: "none" }, db); + const res = await request(app).get("/api/invites/token-123/skills/paperclip"); + + expect(res.status).toBe(200); + expect(res.headers["content-type"]).toContain("text/markdown"); + expect(res.text).toContain("# Paperclip Skill"); + }); + it("marks challenge status as requiring sign-in for anonymous viewers", async () => { mockBoardAuthService.describeCliAuthChallenge.mockResolvedValue({ id: "challenge-1", diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index b43f7b44..f7a6e88b 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -59,6 +59,11 @@ const assetSvc = { create: vi.fn(), }; +const secretSvc = { + normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record) => config), + resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record) => ({ config, secretKeys: new Set() })), +}; + const agentInstructionsSvc = { exportFiles: vi.fn(), materializeManagedBundle: vi.fn(), @@ -96,6 +101,10 @@ vi.mock("../services/assets.js", () => ({ assetService: () => assetSvc, })); +vi.mock("../services/secrets.js", () => ({ + secretService: () => secretSvc, +})); + vi.mock("../services/agent-instructions.js", () => ({ agentInstructionsService: () => agentInstructionsSvc, })); @@ -117,6 +126,11 @@ describe("company portability", () => { beforeEach(() => { vi.clearAllMocks(); + secretSvc.normalizeAdapterConfigForPersistence.mockImplementation(async (_companyId, config) => config); + secretSvc.resolveAdapterConfigForRuntime.mockImplementation(async (_companyId, config) => ({ + config, + secretKeys: new Set(), + })); companySvc.getById.mockResolvedValue({ id: "company-1", name: "Paperclip", @@ -127,6 +141,11 @@ describe("company portability", () => { logoUrl: null, requireBoardApprovalForNewAgents: true, }); + companySvc.create.mockResolvedValue({ + id: "company-imported", + name: "Imported Paperclip", + requireBoardApprovalForNewAgents: true, + }); agentSvc.list.mockResolvedValue([ { id: "agent-1", @@ -1509,7 +1528,7 @@ describe("company portability", () => { }), ]); - await portability.importBundle({ + const result = await portability.importBundle({ source: { type: "inline", rootPath: "paperclip-demo", files }, include: { company: true, agents: true, projects: true, issues: true, skills: false }, target: { mode: "new_company", newCompanyName: "Imported Paperclip" }, @@ -1520,12 +1539,15 @@ describe("company portability", () => { expect(routineSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({ projectId: "project-created", title: "Monday Review", - assigneeAgentId: "agent-created", + assigneeAgentId: null, priority: "high", status: "paused", concurrencyPolicy: "always_enqueue", catchUpPolicy: "enqueue_missed_with_cap", }), expect.any(Object)); + expect(result.warnings).toContain( + "Task monday-review assignee claudecoder is pending_approval; imported work was left unassigned.", + ); expect(routineSvc.createTrigger).toHaveBeenCalledTimes(2); expect(routineSvc.createTrigger).toHaveBeenCalledWith("routine-created", expect.objectContaining({ kind: "schedule", @@ -2418,4 +2440,178 @@ describe("company portability", () => { expect(nestedMaterializedFiles?.["AGENTS.md"]).not.toMatch(/^---\n/); expect(nestedMaterializedFiles?.["AGENTS.md"]).not.toContain('name: "ClaudeCoder"'); }); + + it("rejects dangerous adapter types on agent-safe imports", async () => { + const portability = companyPortabilityService({} as any); + const exported = await portability.exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + }); + + agentSvc.list.mockResolvedValue([]); + + await expect(portability.importBundle({ + source: { + type: "inline", + rootPath: exported.rootPath, + files: exported.files, + }, + include: { + company: false, + agents: true, + projects: false, + issues: false, + }, + target: { + mode: "existing_company", + companyId: "company-1", + }, + agents: ["claudecoder"], + collisionStrategy: "rename", + adapterOverrides: { + claudecoder: { + adapterType: "process", + adapterConfig: { + command: "/bin/sh", + args: ["-c", "id"], + }, + }, + }, + }, "user-1", { + mode: "agent_safe", + sourceCompanyId: "company-1", + })).rejects.toThrow('Adapter type "process" is not allowed in safe imports'); + + expect(agentSvc.create).not.toHaveBeenCalled(); + }); + + it("imports new agents through approval and adapter-config normalization", async () => { + const portability = companyPortabilityService({} as any); + const exported = await portability.exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + }); + + agentSvc.list.mockResolvedValue([]); + secretSvc.normalizeAdapterConfigForPersistence.mockResolvedValueOnce({ + normalized: true, + env: { + OPENAI_API_KEY: { + type: "secret_ref", + secretId: "secret-1", + version: "latest", + }, + }, + }); + agentSvc.create.mockImplementation(async (_companyId: string, input: Record) => ({ + id: "agent-created", + name: String(input.name), + adapterType: input.adapterType, + adapterConfig: input.adapterConfig, + status: input.status, + })); + + await portability.importBundle({ + source: { + type: "inline", + rootPath: exported.rootPath, + files: exported.files, + }, + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + target: { + mode: "new_company", + newCompanyName: "Imported Paperclip", + }, + agents: ["claudecoder"], + collisionStrategy: "rename", + }, "user-1"); + + expect(secretSvc.normalizeAdapterConfigForPersistence).toHaveBeenCalledWith( + "company-imported", + expect.any(Object), + { strictMode: false }, + ); + expect(agentSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({ + adapterType: "claude_local", + adapterConfig: expect.objectContaining({ + normalized: true, + }), + status: "pending_approval", + })); + }); + + it("normalizes adapter config on replace imports before updating existing agents", async () => { + const portability = companyPortabilityService({} as any); + const exported = await portability.exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + }); + + secretSvc.normalizeAdapterConfigForPersistence.mockResolvedValueOnce({ + normalized: "updated", + }); + agentSvc.update.mockImplementation(async (id: string, patch: Record) => ({ + id, + name: "ClaudeCoder", + adapterType: patch.adapterType, + adapterConfig: patch.adapterConfig, + })); + + await portability.importBundle({ + source: { + type: "inline", + rootPath: exported.rootPath, + files: exported.files, + }, + include: { + company: false, + agents: true, + projects: false, + issues: false, + }, + target: { + mode: "existing_company", + companyId: "company-1", + }, + agents: ["claudecoder"], + collisionStrategy: "replace", + adapterOverrides: { + claudecoder: { + adapterType: "codex_local", + adapterConfig: { + model: "gpt-5.4", + }, + }, + }, + }, "user-1"); + + expect(secretSvc.normalizeAdapterConfigForPersistence).toHaveBeenCalledWith( + "company-1", + expect.any(Object), + { strictMode: false }, + ); + expect(agentSvc.update).toHaveBeenCalledWith("agent-1", expect.objectContaining({ + adapterType: "codex_local", + adapterConfig: { + normalized: "updated", + }, + })); + }); }); diff --git a/server/src/__tests__/health.test.ts b/server/src/__tests__/health.test.ts index ce3c002a..e941776e 100644 --- a/server/src/__tests__/health.test.ts +++ b/server/src/__tests__/health.test.ts @@ -34,7 +34,7 @@ describe("GET /health", () => { const res = await request(app).get("/health"); expect(res.status).toBe(200); expect(res.body).toEqual({ status: "ok", version: serverVersion }); - }); + }, 15_000); it("returns 200 when the database probe succeeds", async () => { const db = { @@ -63,4 +63,120 @@ describe("GET /health", () => { error: "database_unreachable", }); }); + + it("redacts detailed metadata for anonymous requests in authenticated mode", async () => { + const devServerStatus = await import("../dev-server-status.js"); + vi.spyOn(devServerStatus, "readPersistedDevServerStatus").mockReturnValue(undefined); + const { healthRoutes } = await import("../routes/health.js"); + const db = { + execute: vi.fn().mockResolvedValue([{ "?column?": 1 }]), + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn().mockResolvedValue([{ count: 1 }]), + })), + })), + } as unknown as Db; + const app = express(); + app.use((req, _res, next) => { + (req as any).actor = { type: "none", source: "none" }; + next(); + }); + app.use( + "/health", + healthRoutes(db, { + deploymentMode: "authenticated", + deploymentExposure: "public", + authReady: true, + companyDeletionEnabled: false, + }), + ); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + status: "ok", + deploymentMode: "authenticated", + bootstrapStatus: "ready", + bootstrapInviteActive: false, + }); + }); + + it("redacts detailed metadata when authenticated mode is reached without auth middleware", async () => { + const devServerStatus = await import("../dev-server-status.js"); + vi.spyOn(devServerStatus, "readPersistedDevServerStatus").mockReturnValue(undefined); + const { healthRoutes } = await import("../routes/health.js"); + const db = { + execute: vi.fn().mockResolvedValue([{ "?column?": 1 }]), + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn().mockResolvedValue([{ count: 1 }]), + })), + })), + } as unknown as Db; + const app = express(); + app.use( + "/health", + healthRoutes(db, { + deploymentMode: "authenticated", + deploymentExposure: "public", + authReady: true, + companyDeletionEnabled: false, + }), + ); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + status: "ok", + deploymentMode: "authenticated", + bootstrapStatus: "ready", + bootstrapInviteActive: false, + }); + }); + + it("keeps detailed metadata for authenticated requests in authenticated mode", async () => { + const devServerStatus = await import("../dev-server-status.js"); + vi.spyOn(devServerStatus, "readPersistedDevServerStatus").mockReturnValue(undefined); + const { healthRoutes } = await import("../routes/health.js"); + const db = { + execute: vi.fn().mockResolvedValue([{ "?column?": 1 }]), + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn().mockResolvedValue([{ count: 1 }]), + })), + })), + } as unknown as Db; + const app = express(); + app.use((req, _res, next) => { + (req as any).actor = { type: "board", userId: "user-1", source: "session" }; + next(); + }); + app.use( + "/health", + healthRoutes(db, { + deploymentMode: "authenticated", + deploymentExposure: "public", + authReady: true, + companyDeletionEnabled: false, + }), + ); + + const res = await request(app).get("/health"); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + status: "ok", + version: serverVersion, + deploymentMode: "authenticated", + deploymentExposure: "public", + authReady: true, + bootstrapStatus: "ready", + bootstrapInviteActive: false, + features: { + companyDeletionEnabled: false, + }, + }); + }); }); diff --git a/server/src/__tests__/invite-onboarding-text.test.ts b/server/src/__tests__/invite-onboarding-text.test.ts index 8ba30115..fae84a63 100644 --- a/server/src/__tests__/invite-onboarding-text.test.ts +++ b/server/src/__tests__/invite-onboarding-text.test.ts @@ -41,6 +41,7 @@ describe("buildInviteOnboardingTextDocument", () => { expect(text).toContain("/api/invites/token-123/accept"); expect(text).toContain("/api/join-requests/{requestId}/claim-api-key"); expect(text).toContain("/api/invites/token-123/onboarding.txt"); + expect(text).toContain("/api/invites/token-123/skills/paperclip"); expect(text).toContain("Suggested Paperclip base URLs to try"); expect(text).toContain("http://localhost:3100"); expect(text).toContain("host.docker.internal"); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 1c887138..2ef33886 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -148,7 +148,7 @@ async function normalizePolicy(input: { return normalizeIssueExecutionPolicy(input); } -function makeIssue(status: "todo" | "done") { +function makeIssue(status: "todo" | "done" | "blocked") { return { id: "11111111-1111-4111-8111-111111111111", companyId: "company-1", @@ -430,6 +430,34 @@ describe("issue comment reopen routes", () => { expect(res.status).toBe(201); expect(mockIssueService.update).not.toHaveBeenCalled(); }); + + it("wakes the assignee when an assigned blocked issue moves back to todo", async () => { + const issue = makeIssue("blocked"); + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...issue, + ...patch, + updatedAt: new Date(), + })); + + const res = await request(await installActor(createApp())) + .patch("/api/issues/11111111-1111-4111-8111-111111111111") + .send({ status: "todo" }); + + expect(res.status).toBe(200); + expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( + "22222222-2222-4222-8222-222222222222", + expect.objectContaining({ + source: "automation", + triggerDetail: "system", + reason: "issue_status_changed", + payload: expect.objectContaining({ + issueId: "11111111-1111-4111-8111-111111111111", + mutation: "update", + }), + }), + ); + }); it("interrupts an active run before a combined comment update", async () => { const issue = { ...makeIssue("todo"), diff --git a/server/src/__tests__/issue-workspace-command-authz.test.ts b/server/src/__tests__/issue-workspace-command-authz.test.ts new file mode 100644 index 00000000..cf9a7847 --- /dev/null +++ b/server/src/__tests__/issue-workspace-command-authz.test.ts @@ -0,0 +1,165 @@ +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 mockIssueService = vi.hoisted(() => ({ + addComment: vi.fn(), + assertCheckoutOwner: vi.fn(), + create: vi.fn(), + findMentionedAgents: vi.fn(), + getByIdentifier: vi.fn(), + getById: vi.fn(), + getRelationSummaries: vi.fn(), + getWakeableParentAfterChildCompletion: vi.fn(), + listWakeableBlockedDependents: vi.fn(), + update: vi.fn(), +})); + +vi.mock("../services/index.js", () => ({ + accessService: () => ({ + canUser: vi.fn(async () => true), + hasPermission: vi.fn(async () => true), + }), + agentService: () => ({ + getById: vi.fn(async () => null), + }), + documentService: () => ({}), + executionWorkspaceService: () => ({ + getById: vi.fn(async () => null), + }), + 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 () => ["company-1"]), + }), + issueApprovalService: () => ({}), + issueService: () => mockIssueService, + logActivity: vi.fn(async () => undefined), + projectService: () => ({}), + routineService: () => ({ + syncRunStatusForIssue: vi.fn(async () => undefined), + }), + workProductService: () => ({}), +})); + +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, {} as any)); + app.use(errorHandler); + return app; +} + +function makeIssue(overrides: Record = {}) { + return { + id: "issue-1", + companyId: "company-1", + status: "todo", + priority: "medium", + projectId: null, + goalId: null, + parentId: null, + assigneeAgentId: null, + assigneeUserId: null, + createdByUserId: "board-user", + identifier: "PAP-1000", + title: "Workspace authz", + executionPolicy: null, + executionState: null, + executionWorkspaceId: null, + hiddenAt: null, + ...overrides, + }; +} + +describe("issue workspace command authorization", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockIssueService.addComment.mockResolvedValue(null); + mockIssueService.create.mockResolvedValue(makeIssue()); + mockIssueService.findMentionedAgents.mockResolvedValue([]); + mockIssueService.getByIdentifier.mockResolvedValue(null); + mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] }); + mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null); + mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]); + mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null }); + mockIssueService.update.mockResolvedValue(makeIssue()); + }); + + it("rejects agent callers that create issue workspace provision commands", async () => { + const app = createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post("/api/companies/company-1/issues") + .send({ + title: "Exploit", + executionWorkspaceSettings: { + workspaceStrategy: { + type: "git_worktree", + provisionCommand: "touch /tmp/paperclip-rce", + }, + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockIssueService.create).not.toHaveBeenCalled(); + }); + + it("rejects agent callers that patch assignee adapter workspace teardown commands", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue()); + const app = createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/issues/issue-1") + .send({ + assigneeAdapterOverrides: { + adapterConfig: { + workspaceStrategy: { + type: "git_worktree", + teardownCommand: "rm -rf /tmp/paperclip-rce", + }, + }, + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts new file mode 100644 index 00000000..3150a182 --- /dev/null +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -0,0 +1,168 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockRegistry = vi.hoisted(() => ({ + getById: vi.fn(), + getByKey: vi.fn(), +})); + +const mockLifecycle = vi.hoisted(() => ({ + load: vi.fn(), + upgrade: vi.fn(), +})); + +vi.mock("../services/plugin-registry.js", () => ({ + pluginRegistryService: () => mockRegistry, +})); + +vi.mock("../services/plugin-lifecycle.js", () => ({ + pluginLifecycleManager: () => mockLifecycle, +})); + +vi.mock("../services/activity-log.js", () => ({ + logActivity: vi.fn(), +})); + +vi.mock("../services/live-events.js", () => ({ + publishGlobalLiveEvent: vi.fn(), +})); + +async function createApp(actor: Record, loaderOverrides: Record = {}) { + const [{ pluginRoutes }, { errorHandler }] = await Promise.all([ + import("../routes/plugins.js"), + import("../middleware/index.js"), + ]); + + const loader = { + installPlugin: vi.fn(), + ...loaderOverrides, + }; + + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = actor as typeof req.actor; + next(); + }); + app.use("/api", pluginRoutes({} as never, loader as never)); + app.use(errorHandler); + + return { app, loader }; +} + +describe("plugin install and upgrade authz", () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it("rejects plugin installation for non-admin board users", async () => { + const { app, loader } = await createApp({ + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: false, + companyIds: ["company-1"], + }); + + const res = await request(app) + .post("/api/plugins/install") + .send({ packageName: "paperclip-plugin-example" }); + + expect(res.status).toBe(403); + expect(loader.installPlugin).not.toHaveBeenCalled(); + }, 20_000); + + it("allows instance admins to install plugins", async () => { + const pluginId = "11111111-1111-4111-8111-111111111111"; + const pluginKey = "paperclip.example"; + const discovered = { + manifest: { + id: pluginKey, + }, + }; + + mockRegistry.getByKey.mockResolvedValue({ + id: pluginId, + pluginKey, + packageName: "paperclip-plugin-example", + version: "1.0.0", + }); + mockRegistry.getById.mockResolvedValue({ + id: pluginId, + pluginKey, + packageName: "paperclip-plugin-example", + version: "1.0.0", + }); + mockLifecycle.load.mockResolvedValue(undefined); + + const { app, loader } = await createApp( + { + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: [], + }, + { installPlugin: vi.fn().mockResolvedValue(discovered) }, + ); + + const res = await request(app) + .post("/api/plugins/install") + .send({ packageName: "paperclip-plugin-example" }); + + expect(res.status).toBe(200); + expect(loader.installPlugin).toHaveBeenCalledWith({ + packageName: "paperclip-plugin-example", + version: undefined, + }); + expect(mockLifecycle.load).toHaveBeenCalledWith(pluginId); + }, 20_000); + + it("rejects plugin upgrades for non-admin board users", async () => { + const pluginId = "11111111-1111-4111-8111-111111111111"; + const { app } = await createApp({ + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: false, + companyIds: ["company-1"], + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/upgrade`) + .send({}); + + expect(res.status).toBe(403); + expect(mockRegistry.getById).not.toHaveBeenCalled(); + expect(mockLifecycle.upgrade).not.toHaveBeenCalled(); + }, 20_000); + + it("allows instance admins to upgrade plugins", async () => { + const pluginId = "11111111-1111-4111-8111-111111111111"; + mockRegistry.getById.mockResolvedValue({ + id: pluginId, + pluginKey: "paperclip.example", + version: "1.0.0", + }); + mockLifecycle.upgrade.mockResolvedValue({ + id: pluginId, + version: "1.1.0", + }); + + const { app } = await createApp({ + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: [], + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/upgrade`) + .send({ version: "1.1.0" }); + + expect(res.status).toBe(200); + expect(mockLifecycle.upgrade).toHaveBeenCalledWith(pluginId, "1.1.0"); + }, 20_000); +}); diff --git a/server/src/__tests__/workspace-runtime-routes-authz.test.ts b/server/src/__tests__/workspace-runtime-routes-authz.test.ts new file mode 100644 index 00000000..de5b490c --- /dev/null +++ b/server/src/__tests__/workspace-runtime-routes-authz.test.ts @@ -0,0 +1,437 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockProjectService = vi.hoisted(() => ({ + create: vi.fn(), + createWorkspace: vi.fn(), + getById: vi.fn(), + listWorkspaces: vi.fn(), + resolveByReference: vi.fn(), + update: vi.fn(), + updateWorkspace: vi.fn(), +})); + +const mockExecutionWorkspaceService = vi.hoisted(() => ({ + getById: vi.fn(), + update: vi.fn(), +})); + +const mockSecretService = vi.hoisted(() => ({ + normalizeEnvBindingsForPersistence: vi.fn(), +})); + +const mockWorkspaceOperationService = vi.hoisted(() => ({})); +const mockLogActivity = vi.hoisted(() => vi.fn()); +const mockGetTelemetryClient = vi.hoisted(() => vi.fn()); +const mockAssertCanManageProjectWorkspaceRuntimeServices = vi.hoisted(() => vi.fn()); +const mockAssertCanManageExecutionWorkspaceRuntimeServices = vi.hoisted(() => vi.fn()); + +function registerModuleMocks() { + vi.doMock("../telemetry.js", () => ({ + getTelemetryClient: mockGetTelemetryClient, + })); + + vi.doMock("../services/index.js", () => ({ + executionWorkspaceService: () => mockExecutionWorkspaceService, + logActivity: mockLogActivity, + projectService: () => mockProjectService, + secretService: () => mockSecretService, + workspaceOperationService: () => mockWorkspaceOperationService, + })); + + vi.doMock("../services/workspace-runtime.js", () => ({ + cleanupExecutionWorkspaceArtifacts: vi.fn(), + startRuntimeServicesForWorkspaceControl: vi.fn(), + stopRuntimeServicesForExecutionWorkspace: vi.fn(), + stopRuntimeServicesForProjectWorkspace: vi.fn(), + })); + + vi.doMock("../routes/workspace-runtime-service-authz.js", () => ({ + assertCanManageProjectWorkspaceRuntimeServices: mockAssertCanManageProjectWorkspaceRuntimeServices, + assertCanManageExecutionWorkspaceRuntimeServices: mockAssertCanManageExecutionWorkspaceRuntimeServices, + })); +} + +async function createProjectApp(actor: Record) { + const { projectRoutes } = await import("../routes/projects.js"); + const { errorHandler } = await import("../middleware/index.js"); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = actor; + next(); + }); + app.use("/api", projectRoutes({} as any)); + app.use(errorHandler); + return app; +} + +async function createExecutionWorkspaceApp(actor: Record) { + const { executionWorkspaceRoutes } = await import("../routes/execution-workspaces.js"); + const { errorHandler } = await import("../middleware/index.js"); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = actor; + next(); + }); + app.use("/api", executionWorkspaceRoutes({} as any)); + app.use(errorHandler); + return app; +} + +function buildProject(overrides: Record = {}) { + return { + id: "project-1", + companyId: "company-1", + urlKey: "project-1", + goalId: null, + goalIds: [], + goals: [], + name: "Project", + description: null, + status: "backlog", + leadAgentId: null, + targetDate: null, + color: null, + env: null, + pauseReason: null, + pausedAt: null, + executionWorkspacePolicy: null, + codebase: null, + workspaces: [], + primaryWorkspace: null, + archivedAt: null, + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +function buildExecutionWorkspace(overrides: Record = {}) { + return { + id: "workspace-1", + companyId: "company-1", + projectId: "project-1", + projectWorkspaceId: null, + sourceIssueId: null, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "Workspace", + status: "active", + cwd: "/tmp/workspace", + repoUrl: null, + baseRef: "main", + branchName: "feature/test", + providerType: "git_worktree", + providerRef: null, + derivedFromExecutionWorkspaceId: null, + lastUsedAt: new Date(), + openedAt: new Date(), + closedAt: null, + cleanupEligibleAt: null, + cleanupReason: null, + config: null, + metadata: null, + runtimeServices: [], + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +describe("workspace runtime service route authorization", () => { + const projectId = "11111111-1111-4111-8111-111111111111"; + const workspaceId = "22222222-2222-4222-8222-222222222222"; + const executionWorkspaceId = "33333333-3333-4333-8333-333333333333"; + + beforeEach(() => { + vi.resetModules(); + registerModuleMocks(); + vi.clearAllMocks(); + mockSecretService.normalizeEnvBindingsForPersistence.mockImplementation(async (_companyId, env) => env); + mockProjectService.resolveByReference.mockResolvedValue({ ambiguous: false, project: null }); + mockProjectService.create.mockResolvedValue(buildProject()); + mockProjectService.update.mockResolvedValue(buildProject()); + mockProjectService.createWorkspace.mockResolvedValue({ + id: workspaceId, + companyId: "company-1", + projectId, + name: "Workspace", + sourceType: "local_path", + cwd: "/tmp/project", + repoUrl: null, + repoRef: null, + defaultRef: null, + visibility: "default", + setupCommand: null, + cleanupCommand: null, + remoteProvider: null, + remoteWorkspaceRef: null, + sharedWorkspaceKey: null, + metadata: null, + runtimeConfig: null, + isPrimary: false, + runtimeServices: [], + createdAt: new Date(), + updatedAt: new Date(), + }); + mockProjectService.listWorkspaces.mockResolvedValue([{ + id: workspaceId, + companyId: "company-1", + projectId, + name: "Workspace", + sourceType: "local_path", + cwd: "/tmp/project", + repoUrl: null, + repoRef: null, + defaultRef: null, + visibility: "default", + setupCommand: null, + cleanupCommand: null, + remoteProvider: null, + remoteWorkspaceRef: null, + sharedWorkspaceKey: null, + metadata: null, + runtimeConfig: null, + isPrimary: false, + runtimeServices: [], + createdAt: new Date(), + updatedAt: new Date(), + }]); + mockProjectService.updateWorkspace.mockResolvedValue({ + id: workspaceId, + companyId: "company-1", + projectId, + name: "Workspace", + sourceType: "local_path", + cwd: "/tmp/project", + repoUrl: null, + repoRef: null, + defaultRef: null, + visibility: "default", + setupCommand: null, + cleanupCommand: null, + remoteProvider: null, + remoteWorkspaceRef: null, + sharedWorkspaceKey: null, + metadata: null, + runtimeConfig: null, + isPrimary: false, + runtimeServices: [], + createdAt: new Date(), + updatedAt: new Date(), + }); + mockExecutionWorkspaceService.update.mockResolvedValue(buildExecutionWorkspace()); + mockAssertCanManageProjectWorkspaceRuntimeServices.mockResolvedValue(undefined); + mockAssertCanManageExecutionWorkspaceRuntimeServices.mockResolvedValue(undefined); + }); + + it("rejects agent callers for project workspace runtime service mutations when workspace auth denies access", async () => { + const { forbidden } = await import("../errors.js"); + mockProjectService.getById.mockResolvedValue(buildProject({ + id: projectId, + workspaces: [{ + id: workspaceId, + companyId: "company-1", + projectId, + name: "Workspace", + sourceType: "local_path", + cwd: "/tmp/project", + repoUrl: null, + repoRef: null, + defaultRef: null, + visibility: "default", + setupCommand: null, + cleanupCommand: null, + remoteProvider: null, + remoteWorkspaceRef: null, + sharedWorkspaceKey: null, + metadata: null, + runtimeConfig: null, + isPrimary: false, + runtimeServices: [], + createdAt: new Date(), + updatedAt: new Date(), + }], + })); + mockAssertCanManageProjectWorkspaceRuntimeServices.mockRejectedValue( + forbidden("Missing permission to manage workspace runtime services"), + ); + const app = await createProjectApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post(`/api/projects/${projectId}/workspaces/${workspaceId}/runtime-services/start`) + .send({}); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Missing permission"); + expect(mockProjectService.getById).toHaveBeenCalledWith(projectId); + expect(mockAssertCanManageProjectWorkspaceRuntimeServices).toHaveBeenCalled(); + }, 15000); + + it("rejects agent callers that create project execution workspace commands", async () => { + const app = await createProjectApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post("/api/companies/company-1/projects") + .send({ + name: "Exploit", + executionWorkspacePolicy: { + enabled: true, + workspaceStrategy: { + type: "git_worktree", + provisionCommand: "touch /tmp/paperclip-rce", + }, + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockProjectService.create).not.toHaveBeenCalled(); + }); + + it("rejects agent callers that update project workspace cleanup commands", async () => { + mockProjectService.getById.mockResolvedValue(buildProject()); + const app = await createProjectApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/projects/${projectId}/workspaces/${workspaceId}`) + .send({ + cleanupCommand: "rm -rf /tmp/paperclip-rce", + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockProjectService.updateWorkspace).not.toHaveBeenCalled(); + }); + + it("allows board callers through the project workspace runtime auth gate", async () => { + mockProjectService.getById.mockResolvedValue(null); + const app = await createProjectApp({ + type: "board", + userId: "board-1", + companyIds: ["company-1"], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .post(`/api/projects/${projectId}/workspaces/${workspaceId}/runtime-services/start`) + .send({}); + + expect(res.status).toBe(404); + expect(res.body.error).toContain("Project not found"); + expect(mockProjectService.getById).toHaveBeenCalledWith(projectId); + }); + + it("rejects agent callers for execution workspace runtime service mutations when workspace auth denies access", async () => { + const { forbidden } = await import("../errors.js"); + mockExecutionWorkspaceService.getById.mockResolvedValue(buildExecutionWorkspace({ id: executionWorkspaceId })); + mockAssertCanManageExecutionWorkspaceRuntimeServices.mockRejectedValue( + forbidden("Missing permission to manage workspace runtime services"), + ); + const app = await createExecutionWorkspaceApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post(`/api/execution-workspaces/${executionWorkspaceId}/runtime-services/restart`) + .send({}); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Missing permission"); + expect(mockExecutionWorkspaceService.getById).toHaveBeenCalledWith(executionWorkspaceId); + expect(mockAssertCanManageExecutionWorkspaceRuntimeServices).toHaveBeenCalled(); + }, 15000); + + it("rejects agent callers that patch execution workspace command config", async () => { + mockExecutionWorkspaceService.getById.mockResolvedValue(buildExecutionWorkspace({ id: executionWorkspaceId })); + const app = await createExecutionWorkspaceApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/execution-workspaces/${executionWorkspaceId}`) + .send({ + config: { + cleanupCommand: "rm -rf /tmp/paperclip-rce", + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockExecutionWorkspaceService.update).not.toHaveBeenCalled(); + }); + + it("rejects agent callers that smuggle execution workspace commands through metadata.config", async () => { + mockExecutionWorkspaceService.getById.mockResolvedValue(buildExecutionWorkspace({ id: executionWorkspaceId })); + const app = await createExecutionWorkspaceApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch(`/api/execution-workspaces/${executionWorkspaceId}`) + .send({ + metadata: { + config: { + provisionCommand: "touch /tmp/paperclip-rce", + }, + }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("host-executed workspace commands"); + expect(mockExecutionWorkspaceService.update).not.toHaveBeenCalled(); + }); + + it("allows board callers through the execution workspace runtime auth gate", async () => { + mockExecutionWorkspaceService.getById.mockResolvedValue(null); + const app = await createExecutionWorkspaceApp({ + type: "board", + userId: "board-1", + companyIds: ["company-1"], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .post(`/api/execution-workspaces/${executionWorkspaceId}/runtime-services/restart`) + .send({}); + + expect(res.status).toBe(404); + expect(res.body.error).toContain("Execution workspace not found"); + expect(mockExecutionWorkspaceService.getById).toHaveBeenCalledWith(executionWorkspaceId); + }); +}); diff --git a/server/src/__tests__/workspace-runtime-service-authz.test.ts b/server/src/__tests__/workspace-runtime-service-authz.test.ts new file mode 100644 index 00000000..87460487 --- /dev/null +++ b/server/src/__tests__/workspace-runtime-service-authz.test.ts @@ -0,0 +1,279 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + agents, + companies, + createDb, + executionWorkspaces, + issues, + projectWorkspaces, + projects, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { + assertCanManageExecutionWorkspaceRuntimeServices, + assertCanManageProjectWorkspaceRuntimeServices, +} from "../routes/workspace-runtime-service-authz.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres workspace runtime auth tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +describeEmbeddedPostgres("workspace runtime service authz helper", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-workspace-runtime-authz-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(issues); + await db.delete(executionWorkspaces); + await db.delete(projectWorkspaces); + await db.delete(projects); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `PAP-${companyId.slice(0, 8)}`, + requireBoardApprovalForNewAgents: false, + }); + return companyId; + } + + async function seedProjectWorkspace(companyId: string) { + const projectId = randomUUID(); + const projectWorkspaceId = randomUUID(); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Workspace authz", + status: "in_progress", + }); + await db.insert(projectWorkspaces).values({ + id: projectWorkspaceId, + companyId, + projectId, + name: "Primary", + sourceType: "local_path", + cwd: "/tmp/paperclip-authz-project", + isPrimary: true, + }); + return { projectId, projectWorkspaceId }; + } + + async function seedExecutionWorkspace(companyId: string, projectId: string, projectWorkspaceId: string) { + const executionWorkspaceId = randomUUID(); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + projectWorkspaceId, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "Execution workspace", + status: "active", + providerType: "local_fs", + cwd: "/tmp/paperclip-authz-execution", + }); + return executionWorkspaceId; + } + + async function seedAgent( + companyId: string, + input: { role?: string; reportsTo?: string | null; name?: string } = {}, + ) { + const agentId = randomUUID(); + await db.insert(agents).values({ + id: agentId, + companyId, + name: input.name ?? "Agent", + role: input.role ?? "engineer", + reportsTo: input.reportsTo ?? null, + }); + return agentId; + } + + it("allows board actors to manage project workspace runtime services", async () => { + const companyId = await seedCompany(); + const { projectWorkspaceId } = await seedProjectWorkspace(companyId); + + await expect(assertCanManageProjectWorkspaceRuntimeServices(db, { + actor: { + type: "board", + userId: "board-1", + companyIds: [companyId], + source: "session", + isInstanceAdmin: false, + }, + } as any, { + companyId, + projectWorkspaceId, + })).resolves.toBeUndefined(); + }); + + it("allows CEO agents to manage any project workspace runtime services in their company", async () => { + const companyId = await seedCompany(); + const { projectWorkspaceId } = await seedProjectWorkspace(companyId); + const ceoAgentId = await seedAgent(companyId, { role: "ceo", name: "CEO" }); + + await expect(assertCanManageProjectWorkspaceRuntimeServices(db, { + actor: { + type: "agent", + agentId: ceoAgentId, + companyId, + source: "agent_key", + }, + } as any, { + companyId, + projectWorkspaceId, + })).resolves.toBeUndefined(); + }); + + it("allows agents with a non-terminal assigned issue in the target project workspace", async () => { + const companyId = await seedCompany(); + const { projectId, projectWorkspaceId } = await seedProjectWorkspace(companyId); + const agentId = await seedAgent(companyId, { name: "Engineer" }); + + await db.insert(issues).values({ + id: randomUUID(), + companyId, + projectId, + projectWorkspaceId, + title: "Use this workspace", + status: "todo", + priority: "medium", + assigneeAgentId: agentId, + }); + + await expect(assertCanManageProjectWorkspaceRuntimeServices(db, { + actor: { + type: "agent", + agentId, + companyId, + source: "agent_key", + }, + } as any, { + companyId, + projectWorkspaceId, + })).resolves.toBeUndefined(); + }); + + it("allows managers to manage execution workspace runtime services for their reporting subtree", async () => { + const companyId = await seedCompany(); + const { projectId, projectWorkspaceId } = await seedProjectWorkspace(companyId); + const executionWorkspaceId = await seedExecutionWorkspace(companyId, projectId, projectWorkspaceId); + const managerId = await seedAgent(companyId, { role: "cto", name: "Manager" }); + const reportId = await seedAgent(companyId, { reportsTo: managerId, name: "Report" }); + + await db.insert(issues).values({ + id: randomUUID(), + companyId, + projectId, + projectWorkspaceId, + executionWorkspaceId, + title: "Use execution workspace", + status: "in_progress", + priority: "medium", + assigneeAgentId: reportId, + }); + + await expect(assertCanManageExecutionWorkspaceRuntimeServices(db, { + actor: { + type: "agent", + agentId: managerId, + companyId, + source: "agent_key", + }, + } as any, { + companyId, + executionWorkspaceId, + })).resolves.toBeUndefined(); + }); + + it("rejects unrelated same-company agents without matching workspace assignments", async () => { + const companyId = await seedCompany(); + const { projectId, projectWorkspaceId } = await seedProjectWorkspace(companyId); + const executionWorkspaceId = await seedExecutionWorkspace(companyId, projectId, projectWorkspaceId); + const assignedAgentId = await seedAgent(companyId, { name: "Assigned" }); + const unrelatedAgentId = await seedAgent(companyId, { name: "Unrelated" }); + + await db.insert(issues).values({ + id: randomUUID(), + companyId, + projectId, + projectWorkspaceId, + executionWorkspaceId, + title: "Assigned issue", + status: "todo", + priority: "medium", + assigneeAgentId: assignedAgentId, + }); + + await expect(assertCanManageExecutionWorkspaceRuntimeServices(db, { + actor: { + type: "agent", + agentId: unrelatedAgentId, + companyId, + source: "agent_key", + }, + } as any, { + companyId, + executionWorkspaceId, + })).rejects.toMatchObject({ + status: 403, + message: "Missing permission to manage workspace runtime services", + }); + }); + + it("rejects completed workspace assignments so stale issues do not keep access alive", async () => { + const companyId = await seedCompany(); + const { projectId, projectWorkspaceId } = await seedProjectWorkspace(companyId); + const agentId = await seedAgent(companyId, { name: "Engineer" }); + + await db.insert(issues).values({ + id: randomUUID(), + companyId, + projectId, + projectWorkspaceId, + title: "Completed issue", + status: "done", + priority: "medium", + assigneeAgentId: agentId, + }); + + await expect(assertCanManageProjectWorkspaceRuntimeServices(db, { + actor: { + type: "agent", + agentId, + companyId, + source: "agent_key", + }, + } as any, { + companyId, + projectWorkspaceId, + })).rejects.toMatchObject({ + status: 403, + message: "Missing permission to manage workspace runtime services", + }); + }); +}); diff --git a/server/src/app.ts b/server/src/app.ts index 7daf24c1..317b8799 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -86,6 +86,16 @@ function shouldServeViteDevHtml(req: ExpressRequest): boolean { return req.accepts(["html"]) === "html"; } +export function shouldEnablePrivateHostnameGuard(opts: { + deploymentMode: DeploymentMode; + deploymentExposure: DeploymentExposure; +}): boolean { + return ( + opts.deploymentExposure === "private" && + (opts.deploymentMode === "local_trusted" || opts.deploymentMode === "authenticated") + ); +} + export async function createApp( db: Db, opts: { @@ -123,8 +133,10 @@ export async function createApp( }, })); app.use(httpLogger); - const privateHostnameGateEnabled = - opts.deploymentMode === "authenticated" && opts.deploymentExposure === "private"; + const privateHostnameGateEnabled = shouldEnablePrivateHostnameGuard({ + deploymentMode: opts.deploymentMode, + deploymentExposure: opts.deploymentExposure, + }); const privateHostnameAllowSet = resolvePrivateHostnameAllowSet({ allowedHostnames: opts.allowedHostnames, bindHost: opts.bindHost, diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index e4893765..0be3adab 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -48,7 +48,7 @@ import { logActivity, notifyHireApproved } from "../services/index.js"; -import { assertCompanyAccess } from "./authz.js"; +import { assertAuthenticated, assertCompanyAccess } from "./authz.js"; import { claimBoardOwnership, inspectBoardClaimChallenge @@ -863,6 +863,7 @@ function toInviteSummaryResponse( const baseUrl = requestBaseUrl(req); const onboardingPath = `/api/invites/${token}/onboarding`; const onboardingTextPath = `/api/invites/${token}/onboarding.txt`; + const skillIndexPath = `/api/invites/${token}/skills/index`; const inviteMessage = extractInviteMessage(invite); return { id: invite.id, @@ -877,10 +878,10 @@ function toInviteSummaryResponse( onboardingTextUrl: baseUrl ? `${baseUrl}${onboardingTextPath}` : onboardingTextPath, - skillIndexPath: "/api/skills/index", + skillIndexPath, skillIndexUrl: baseUrl - ? `${baseUrl}/api/skills/index` - : "/api/skills/index", + ? `${baseUrl}${skillIndexPath}` + : skillIndexPath, inviteMessage }; } @@ -1004,7 +1005,7 @@ function buildInviteOnboardingManifest( } ) { const baseUrl = requestBaseUrl(req); - const skillPath = "/api/skills/paperclip"; + const skillPath = `/api/invites/${token}/skills/paperclip`; const skillUrl = baseUrl ? `${baseUrl}${skillPath}` : skillPath; const registrationEndpointPath = `/api/invites/${token}/accept`; const registrationEndpointUrl = baseUrl @@ -1906,11 +1907,13 @@ export function accessRoutes( return company?.name ?? null; } - router.get("/skills/available", (_req, res) => { + router.get("/skills/available", (req, res) => { + assertAuthenticated(req); res.json({ skills: listAvailableSkills() }); }); - router.get("/skills/index", (_req, res) => { + router.get("/skills/index", (req, res) => { + assertAuthenticated(req); res.json({ skills: [ { name: "paperclip", path: "/api/skills/paperclip" }, @@ -1927,6 +1930,7 @@ export function accessRoutes( }); router.get("/skills/:skillName", (req, res) => { + assertAuthenticated(req); const skillName = (req.params.skillName as string).trim().toLowerCase(); const markdown = readSkillMarkdown(skillName); if (!markdown) throw notFound("Skill not found"); @@ -2100,6 +2104,47 @@ export function accessRoutes( ); }); + router.get("/invites/:token/skills/index", async (req, res) => { + const token = (req.params.token as string).trim(); + if (!token) throw notFound("Invite not found"); + const invite = await db + .select() + .from(invites) + .where(eq(invites.tokenHash, hashToken(token))) + .then((rows) => rows[0] ?? null); + if (!invite || invite.revokedAt || inviteExpired(invite)) { + throw notFound("Invite not found"); + } + + res.json({ + skills: [ + { + name: "paperclip", + path: `/api/invites/${token}/skills/paperclip`, + }, + ], + }); + }); + + router.get("/invites/:token/skills/:skillName", async (req, res) => { + const token = (req.params.token as string).trim(); + if (!token) throw notFound("Invite not found"); + const invite = await db + .select() + .from(invites) + .where(eq(invites.tokenHash, hashToken(token))) + .then((rows) => rows[0] ?? null); + if (!invite || invite.revokedAt || inviteExpired(invite)) { + throw notFound("Invite not found"); + } + + const skillName = (req.params.skillName as string).trim().toLowerCase(); + if (skillName !== "paperclip") throw notFound("Skill not found"); + const markdown = readSkillMarkdown(skillName); + if (!markdown) throw notFound("Skill not found"); + res.type("text/markdown").send(markdown); + }); + router.get("/invites/:token/test-resolution", async (req, res) => { const token = (req.params.token as string).trim(); if (!token) throw notFound("Invite not found"); diff --git a/server/src/routes/activity.ts b/server/src/routes/activity.ts index 20f2ed53..53083b1b 100644 --- a/server/src/routes/activity.ts +++ b/server/src/routes/activity.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import type { Db } from "@paperclipai/db"; import { validate } from "../middleware/validate.js"; import { activityService } from "../services/activity.js"; -import { assertBoard, assertCompanyAccess } from "./authz.js"; +import { assertAuthenticated, assertBoard, assertCompanyAccess } from "./authz.js"; import { heartbeatService, issueService } from "../services/index.js"; import { sanitizeRecord } from "../redaction.js"; @@ -81,6 +81,7 @@ export function activityRoutes(db: Db) { }); router.get("/heartbeat-runs/:runId/issues", async (req, res) => { + assertAuthenticated(req); const runId = req.params.runId as string; const run = await heartbeat.getRun(runId); if (!run) { diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index faf27f24..9511d6d0 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1,4 +1,4 @@ -import { Router, type Request } from "express"; +import { Router, type Request, type Response } from "express"; import { generateKeyPairSync, randomUUID } from "node:crypto"; import path from "node:path"; import type { Db } from "@paperclipai/db"; @@ -46,6 +46,10 @@ import { } from "../services/index.js"; import { conflict, forbidden, notFound, unprocessable } from "../errors.js"; import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; +import { + assertNoAgentHostWorkspaceCommandMutation, + collectAgentAdapterWorkspaceCommandPaths, +} from "./workspace-command-authz.js"; import { detectAdapterModel, findActiveServerAdapter, @@ -231,10 +235,33 @@ export function agentRoutes(db: Db) { return actorAgent; } + async function assertBoardCanManageAgentsForCompany(req: Request, companyId: string) { + assertBoard(req); + assertCompanyAccess(req, companyId); + if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; + const allowed = await access.canUser(companyId, req.actor.userId, "agents:create"); + if (!allowed) { + throw forbidden("Missing permission: agents:create"); + } + } + async function assertCanReadConfigurations(req: Request, companyId: string) { return assertCanCreateAgentsForCompany(req, companyId); } + async function getAccessibleAgent(req: Request, res: Response, id: string) { + const agent = await svc.getById(id); + if (!agent) { + res.status(404).json({ error: "Agent not found" }); + return null; + } + assertCompanyAccess(req, agent.companyId); + if (req.actor.type === "board") { + await assertBoardCanManageAgentsForCompany(req, agent.companyId); + } + return agent; + } + async function actorCanReadConfigurationsForCompany(req: Request, companyId: string) { assertCompanyAccess(req, companyId); if (req.actor.type === "board") { @@ -317,7 +344,10 @@ export function agentRoutes(db: Db) { async function assertCanUpdateAgent(req: Request, targetAgent: { id: string; companyId: string }) { assertCompanyAccess(req, targetAgent.companyId); - if (req.actor.type === "board") return; + if (req.actor.type === "board") { + await assertBoardCanManageAgentsForCompany(req, targetAgent.companyId); + return; + } if (!req.actor.agentId) throw forbidden("Agent authentication required"); const actorAgent = await svc.getById(req.actor.agentId); @@ -339,7 +369,10 @@ export function agentRoutes(db: Db) { async function assertCanReadAgent(req: Request, targetAgent: { companyId: string }) { assertCompanyAccess(req, targetAgent.companyId); - if (req.actor.type === "board") return; + if (req.actor.type === "board") { + await assertCanReadConfigurations(req, targetAgent.companyId); + return; + } if (!req.actor.agentId) throw forbidden("Agent authentication required"); const actorAgent = await svc.getById(req.actor.agentId); @@ -610,19 +643,24 @@ export function agentRoutes(db: Db) { async function assertCanManageInstructionsPath(req: Request, targetAgent: { id: string; companyId: string }) { assertCompanyAccess(req, targetAgent.companyId); - if (req.actor.type === "board") return; - if (!req.actor.agentId) throw forbidden("Agent authentication required"); - - const actorAgent = await svc.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== targetAgent.companyId) { - throw forbidden("Agent key cannot access another company"); + if (req.actor.type !== "board") { + throw forbidden( + "Only board-authenticated callers can manage instructions path or bundle configuration", + ); } - if (actorAgent.id === targetAgent.id) return; + await assertBoardCanManageAgentsForCompany(req, targetAgent.companyId); + } - const chainOfCommand = await svc.getChainOfCommand(targetAgent.id); - if (chainOfCommand.some((manager) => manager.id === actorAgent.id)) return; - - throw forbidden("Only the target agent or an ancestor manager can update instructions path"); + function assertNoAgentInstructionsConfigMutation( + req: Request, + adapterConfig: Record | null | undefined, + ) { + if (req.actor.type !== "agent" || !adapterConfig) return; + const changedSensitiveKeys = KNOWN_INSTRUCTIONS_BUNDLE_KEYS.filter((key) => adapterConfig[key] !== undefined); + if (changedSensitiveKeys.length === 0) return; + throw forbidden( + `Agent-authenticated callers cannot modify instructions path or bundle configuration (${changedSensitiveKeys.join(", ")})`, + ); } function summarizeAgentUpdateDetails(patch: Record) { @@ -997,7 +1035,7 @@ export function agentRoutes(db: Db) { } const result = await svc.list(companyId); const canReadConfigs = await actorCanReadConfigurationsForCompany(req, companyId); - if (canReadConfigs || req.actor.type === "board") { + if (canReadConfigs) { res.json(result); return; } @@ -1173,12 +1211,13 @@ export function agentRoutes(db: Db) { return; } assertCompanyAccess(req, agent.companyId); - if (req.actor.type === "agent" && req.actor.agentId !== id) { - const canRead = await actorCanReadConfigurationsForCompany(req, agent.companyId); - if (!canRead) { - res.json(await buildAgentDetail(agent, { restricted: true })); - return; - } + const isSelf = req.actor.type === "agent" && req.actor.agentId === id; + const canReadSensitiveDetail = isSelf + ? true + : await actorCanReadConfigurationsForCompany(req, agent.companyId); + if (!canReadSensitiveDetail) { + res.json(await buildAgentDetail(agent, { restricted: true })); + return; } res.json(await buildAgentDetail(agent)); }); @@ -1266,6 +1305,7 @@ export function agentRoutes(db: Db) { res.status(404).json({ error: "Agent not found" }); return; } + await assertBoardCanManageAgentsForCompany(req, agent.companyId); assertCompanyAccess(req, agent.companyId); const state = await heartbeat.getRuntimeState(id); @@ -1280,6 +1320,7 @@ export function agentRoutes(db: Db) { res.status(404).json({ error: "Agent not found" }); return; } + await assertBoardCanManageAgentsForCompany(req, agent.companyId); assertCompanyAccess(req, agent.companyId); const sessions = await heartbeat.listTaskSessions(id); @@ -1299,6 +1340,7 @@ export function agentRoutes(db: Db) { res.status(404).json({ error: "Agent not found" }); return; } + await assertBoardCanManageAgentsForCompany(req, agent.companyId); assertCompanyAccess(req, agent.companyId); const taskKey = @@ -1331,6 +1373,14 @@ export function agentRoutes(db: Db) { ...hireInput } = req.body; hireInput.adapterType = assertKnownAdapterType(hireInput.adapterType); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectAgentAdapterWorkspaceCommandPaths(hireInput.adapterConfig), + ); + assertNoAgentInstructionsConfigMutation( + req, + (hireInput.adapterConfig ?? {}) as Record, + ); const requestedAdapterConfig = applyCreateDefaultsByAdapterType( hireInput.adapterType, ((hireInput.adapterConfig ?? {}) as Record), @@ -1590,6 +1640,8 @@ export function agentRoutes(db: Db) { res.status(403).json({ error: "Only CEO can manage permissions" }); return; } + } else { + await assertBoardCanManageAgentsForCompany(req, existing.companyId); } const agent = await svc.updatePermissions(id, req.body); @@ -1890,10 +1942,15 @@ export function agentRoutes(db: Db) { res.status(422).json({ error: "adapterConfig must be an object" }); return; } - const changingInstructionsPath = Object.keys(adapterConfig).some((key) => - KNOWN_INSTRUCTIONS_PATH_KEYS.has(key), + assertNoAgentInstructionsConfigMutation(req, adapterConfig); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectAgentAdapterWorkspaceCommandPaths(adapterConfig), ); - if (changingInstructionsPath) { + const changingInstructionsConfig = Object.keys(adapterConfig).some((key) => + KNOWN_INSTRUCTIONS_BUNDLE_KEYS.includes(key as (typeof KNOWN_INSTRUCTIONS_BUNDLE_KEYS)[number]), + ); + if (changingInstructionsConfig) { await assertCanManageInstructionsPath(req, existing); } patchData.adapterConfig = adapterConfig; @@ -1994,6 +2051,9 @@ export function agentRoutes(db: Db) { router.post("/agents/:id/pause", async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await getAccessibleAgent(req, res, id))) { + return; + } const agent = await svc.pause(id); if (!agent) { res.status(404).json({ error: "Agent not found" }); @@ -2017,6 +2077,9 @@ export function agentRoutes(db: Db) { router.post("/agents/:id/resume", async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await getAccessibleAgent(req, res, id))) { + return; + } const agent = await svc.resume(id); if (!agent) { res.status(404).json({ error: "Agent not found" }); @@ -2038,6 +2101,9 @@ export function agentRoutes(db: Db) { router.post("/agents/:id/terminate", async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await getAccessibleAgent(req, res, id))) { + return; + } const agent = await svc.terminate(id); if (!agent) { res.status(404).json({ error: "Agent not found" }); @@ -2061,6 +2127,9 @@ export function agentRoutes(db: Db) { router.delete("/agents/:id", async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await getAccessibleAgent(req, res, id))) { + return; + } const agent = await svc.remove(id); if (!agent) { res.status(404).json({ error: "Agent not found" }); @@ -2082,6 +2151,10 @@ export function agentRoutes(db: Db) { router.get("/agents/:id/keys", async (req, res) => { assertBoard(req); const id = req.params.id as string; + const agent = await getAccessibleAgent(req, res, id); + if (!agent) { + return; + } const keys = await svc.listKeys(id); res.json(keys); }); @@ -2089,32 +2162,56 @@ export function agentRoutes(db: Db) { router.post("/agents/:id/keys", validate(createAgentKeySchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; + const agent = await getAccessibleAgent(req, res, id); + if (!agent) { + return; + } const key = await svc.createApiKey(id, req.body.name); - const agent = await svc.getById(id); - if (agent) { - await logActivity(db, { - companyId: agent.companyId, - actorType: "user", - actorId: req.actor.userId ?? "board", - action: "agent.key_created", - entityType: "agent", - entityId: agent.id, - details: { keyId: key.id, name: key.name }, - }); - } + await logActivity(db, { + companyId: agent.companyId, + actorType: "user", + actorId: req.actor.userId ?? "board", + action: "agent.key_created", + entityType: "agent", + entityId: agent.id, + details: { keyId: key.id, name: key.name }, + }); res.status(201).json(key); }); router.delete("/agents/:id/keys/:keyId", async (req, res) => { assertBoard(req); + const id = req.params.id as string; const keyId = req.params.keyId as string; - const revoked = await svc.revokeKey(keyId); + const agent = await getAccessibleAgent(req, res, id); + if (!agent) { + return; + } + + const key = await svc.getKeyById(keyId); + if (!key || key.agentId !== agent.id) { + res.status(404).json({ error: "Key not found" }); + return; + } + + const revoked = await svc.revokeKey(agent.id, keyId); if (!revoked) { res.status(404).json({ error: "Key not found" }); return; } + + await logActivity(db, { + companyId: agent.companyId, + actorType: "user", + actorId: req.actor.userId ?? "board", + action: "agent.key_revoked", + entityType: "agent", + entityId: agent.id, + details: { keyId: key.id, name: key.name }, + }); + res.json({ ok: true }); }); @@ -2127,9 +2224,13 @@ export function agentRoutes(db: Db) { } assertCompanyAccess(req, agent.companyId); - if (req.actor.type === "agent" && req.actor.agentId !== id) { - res.status(403).json({ error: "Agent can only invoke itself" }); - return; + if (req.actor.type === "agent") { + if (req.actor.agentId !== id) { + res.status(403).json({ error: "Agent can only invoke itself" }); + return; + } + } else { + await assertBoardCanManageAgentsForCompany(req, agent.companyId); } const run = await heartbeat.wakeup(id, { @@ -2177,9 +2278,13 @@ export function agentRoutes(db: Db) { } assertCompanyAccess(req, agent.companyId); - if (req.actor.type === "agent" && req.actor.agentId !== id) { - res.status(403).json({ error: "Agent can only invoke itself" }); - return; + if (req.actor.type === "agent") { + if (req.actor.agentId !== id) { + res.status(403).json({ error: "Agent can only invoke itself" }); + return; + } + } else { + await assertBoardCanManageAgentsForCompany(req, agent.companyId); } const run = await heartbeat.invoke( @@ -2225,6 +2330,7 @@ export function agentRoutes(db: Db) { res.status(404).json({ error: "Agent not found" }); return; } + await assertBoardCanManageAgentsForCompany(req, agent.companyId); assertCompanyAccess(req, agent.companyId); if (agent.adapterType !== "claude_local") { res.status(400).json({ error: "Login is only supported for claude_local agents" }); diff --git a/server/src/routes/approvals.ts b/server/src/routes/approvals.ts index 3ed20374..d1743cc0 100644 --- a/server/src/routes/approvals.ts +++ b/server/src/routes/approvals.ts @@ -134,11 +134,8 @@ export function approvalRoutes(db: Db) { res.status(404).json({ error: "Approval not found" }); return; } - const { approval, applied } = await svc.approve( - id, - req.body.decidedByUserId ?? "board", - req.body.decisionNote, - ); + const decidedByUserId = req.actor.userId ?? "board"; + const { approval, applied } = await svc.approve(id, decidedByUserId, req.body.decisionNote); if (applied) { const linkedIssues = await issueApprovalsSvc.listIssuesForApproval(approval.id); @@ -233,11 +230,8 @@ export function approvalRoutes(db: Db) { res.status(404).json({ error: "Approval not found" }); return; } - const { approval, applied } = await svc.reject( - id, - req.body.decidedByUserId ?? "board", - req.body.decisionNote, - ); + const decidedByUserId = req.actor.userId ?? "board"; + const { approval, applied } = await svc.reject(id, decidedByUserId, req.body.decisionNote); if (applied) { await logActivity(db, { @@ -264,11 +258,8 @@ export function approvalRoutes(db: Db) { res.status(404).json({ error: "Approval not found" }); return; } - const approval = await svc.requestRevision( - id, - req.body.decidedByUserId ?? "board", - req.body.decisionNote, - ); + const decidedByUserId = req.actor.userId ?? "board"; + const approval = await svc.requestRevision(id, decidedByUserId, req.body.decisionNote); await logActivity(db, { companyId: approval.companyId, diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index a881d4ff..e9017c93 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -1,6 +1,12 @@ import type { Request } from "express"; import { forbidden, unauthorized } from "../errors.js"; +export function assertAuthenticated(req: Request) { + if (req.actor.type === "none") { + throw unauthorized(); + } +} + export function assertBoard(req: Request) { if (req.actor.type !== "board") { throw forbidden("Board access required"); @@ -16,9 +22,7 @@ export function assertInstanceAdmin(req: Request) { } export function assertCompanyAccess(req: Request, companyId: string) { - if (req.actor.type === "none") { - throw unauthorized(); - } + assertAuthenticated(req); if (req.actor.type === "agent" && req.actor.companyId !== companyId) { throw forbidden("Agent key cannot access another company"); } @@ -31,9 +35,7 @@ export function assertCompanyAccess(req: Request, companyId: string) { } export function getActorInfo(req: Request) { - if (req.actor.type === "none") { - throw unauthorized(); - } + assertAuthenticated(req); if (req.actor.type === "agent") { return { actorType: "agent" as const, diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index 963be2ed..faabd53c 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -23,6 +23,11 @@ import { stopRuntimeServicesForExecutionWorkspace, } from "../services/workspace-runtime.js"; import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { + assertNoAgentHostWorkspaceCommandMutation, + collectExecutionWorkspaceCommandPaths, +} from "./workspace-command-authz.js"; +import { assertCanManageExecutionWorkspaceRuntimeServices } from "./workspace-runtime-service-authz.js"; export function executionWorkspaceRoutes(db: Db) { const router = Router(); @@ -96,6 +101,12 @@ export function executionWorkspaceRoutes(db: Db) { } assertCompanyAccess(req, existing.companyId); + await assertCanManageExecutionWorkspaceRuntimeServices(db, req, { + companyId: existing.companyId, + executionWorkspaceId: existing.id, + sourceIssueId: existing.sourceIssueId, + }); + const workspaceCwd = existing.cwd; if (!workspaceCwd) { res.status(422).json({ error: "Execution workspace needs a local path before Paperclip can run workspace commands" }); @@ -428,6 +439,13 @@ export function executionWorkspaceRoutes(db: Db) { return; } assertCompanyAccess(req, existing.companyId); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectExecutionWorkspaceCommandPaths({ + config: req.body.config, + metadata: req.body.metadata, + }), + ); const patch: Record = { ...(req.body.name === undefined ? {} : { name: req.body.name }), ...(req.body.cwd === undefined ? {} : { cwd: req.body.cwd }), diff --git a/server/src/routes/health.ts b/server/src/routes/health.ts index 795eb9a5..d5216e72 100644 --- a/server/src/routes/health.ts +++ b/server/src/routes/health.ts @@ -7,6 +7,14 @@ import { readPersistedDevServerStatus, toDevServerHealthStatus } from "../dev-se import { instanceSettingsService } from "../services/instance-settings.js"; import { serverVersion } from "../version.js"; +function shouldExposeFullHealthDetails( + actorType: "none" | "board" | "agent" | null | undefined, + deploymentMode: DeploymentMode, +) { + if (deploymentMode !== "authenticated") return true; + return actorType === "board" || actorType === "agent"; +} + export function healthRoutes( db?: Db, opts: { @@ -23,9 +31,19 @@ export function healthRoutes( ) { const router = Router(); - router.get("/", async (_req, res) => { + router.get("/", async (req, res) => { + const actorType = "actor" in req ? req.actor?.type : null; + const exposeFullDetails = shouldExposeFullHealthDetails( + actorType, + opts.deploymentMode, + ); + if (!db) { - res.json({ status: "ok", version: serverVersion }); + res.json( + exposeFullDetails + ? { status: "ok", version: serverVersion } + : { status: "ok", deploymentMode: opts.deploymentMode }, + ); return; } @@ -70,7 +88,7 @@ export function healthRoutes( const persistedDevServerStatus = readPersistedDevServerStatus(); let devServer: ReturnType | undefined; - if (persistedDevServerStatus) { + if (persistedDevServerStatus && typeof (db as { select?: unknown }).select === "function") { const instanceSettings = instanceSettingsService(db); const experimentalSettings = await instanceSettings.getExperimental(); const activeRunCount = await db @@ -85,6 +103,16 @@ export function healthRoutes( }); } + if (!exposeFullDetails) { + res.json({ + status: "ok", + deploymentMode: opts.deploymentMode, + bootstrapStatus, + bootstrapInviteActive, + }); + return; + } + res.json({ status: "ok", version: serverVersion, diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 4f49c31d..c5d2d40d 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -48,6 +48,10 @@ import { import { logger } from "../middleware/logger.js"; import { conflict, forbidden, HttpError, notFound, unauthorized } from "../errors.js"; import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { + assertNoAgentHostWorkspaceCommandMutation, + collectIssueWorkspaceCommandPaths, +} from "./workspace-command-authz.js"; import { shouldWakeAssigneeOnCheckout } from "./issues-checkout-wakeup.js"; import { isInlineAttachmentContentType, @@ -1323,6 +1327,7 @@ export function issueRoutes( router.post("/companies/:companyId/issues", validate(createIssueSchema), async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body)); if (req.body.assigneeAgentId || req.body.assigneeUserId) { await assertCanAssignTasks(req, companyId); } @@ -1373,6 +1378,7 @@ export function issueRoutes( return; } assertCompanyAccess(req, existing.companyId); + assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body)); if (!(await assertAgentRunCheckoutOwnership(req, res, existing))) return; const actor = getActorInfo(req); diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index 95215d89..a6368be9 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -11,7 +11,8 @@ * - Retrieving UI slot contributions for frontend rendering * - Discovering and executing plugin-contributed agent tools * - * All routes require board-level authentication (assertBoard middleware). + * All routes require board-level authentication, and sensitive instance-wide + * mutations such as install/upgrade require instance-admin privileges. * * @module server/routes/plugins * @see doc/plugins/PLUGIN_SPEC.md for the full plugin specification @@ -47,7 +48,7 @@ import type { PluginStreamBus } from "../services/plugin-stream-bus.js"; import type { PluginToolDispatcher } from "../services/plugin-tool-dispatcher.js"; import type { ToolRunContext } from "@paperclipai/plugin-sdk"; import { JsonRpcCallError, PLUGIN_RPC_ERROR_CODES } from "@paperclipai/plugin-sdk"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; import { validateInstanceConfig } from "../services/plugin-config-validator.js"; /** UI slot declaration extracted from plugin manifest */ @@ -583,6 +584,9 @@ export function pluginRoutes( * * Install a plugin from npm or a local filesystem path. * + * Instance-wide plugin installation is restricted to instance admins because + * the install flow fetches and inspects package contents on the host. + * * Request body: * - packageName: npm package name or local path (required) * - version: Target version for npm packages (optional) @@ -601,7 +605,7 @@ export function pluginRoutes( * - `500` — installation succeeded but manifest is missing (indicates a loader bug) */ router.post("/plugins/install", async (req, res) => { - assertBoard(req); + assertInstanceAdmin(req); const { packageName, version, isLocalPath } = req.body as PluginInstallRequest; // Input validation @@ -1450,6 +1454,9 @@ export function pluginRoutes( * * Upgrade a plugin to a newer version. * + * Upgrades are restricted to instance admins because they fetch and inspect + * new package contents on the host before activation. + * * Request body (optional): * - version: Target version (defaults to latest) * @@ -1461,7 +1468,7 @@ export function pluginRoutes( * Errors: 404 if plugin not found, 400 for lifecycle errors */ router.post("/plugins/:pluginId/upgrade", async (req, res) => { - assertBoard(req); + assertInstanceAdmin(req); const { pluginId } = req.params; const body = req.body as { version?: string } | undefined; const version = body?.version; diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index 71cd792b..f4d3e848 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -22,6 +22,12 @@ import { startRuntimeServicesForWorkspaceControl, stopRuntimeServicesForProjectWorkspace, } from "../services/workspace-runtime.js"; +import { + assertNoAgentHostWorkspaceCommandMutation, + collectProjectExecutionWorkspaceCommandPaths, + collectProjectWorkspaceCommandPaths, +} from "./workspace-command-authz.js"; +import { assertCanManageProjectWorkspaceRuntimeServices } from "./workspace-runtime-service-authz.js"; import { getTelemetryClient } from "../telemetry.js"; export function projectRoutes(db: Db) { @@ -93,6 +99,13 @@ export function projectRoutes(db: Db) { }; const { workspace, ...projectData } = req.body as CreateProjectPayload; + assertNoAgentHostWorkspaceCommandMutation( + req, + [ + ...collectProjectExecutionWorkspaceCommandPaths(projectData.executionWorkspacePolicy), + ...collectProjectWorkspaceCommandPaths(workspace, "workspace"), + ], + ); if (projectData.env !== undefined) { projectData.env = await secretsSvc.normalizeEnvBindingsForPersistence( companyId, @@ -144,6 +157,10 @@ export function projectRoutes(db: Db) { } assertCompanyAccess(req, existing.companyId); const body = { ...req.body }; + assertNoAgentHostWorkspaceCommandMutation( + req, + collectProjectExecutionWorkspaceCommandPaths(body.executionWorkspacePolicy), + ); if (typeof body.archivedAt === "string") { body.archivedAt = new Date(body.archivedAt); } @@ -200,6 +217,10 @@ export function projectRoutes(db: Db) { return; } assertCompanyAccess(req, existing.companyId); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectProjectWorkspaceCommandPaths(req.body), + ); const workspace = await svc.createWorkspace(id, req.body); if (!workspace) { res.status(422).json({ error: "Invalid project workspace payload" }); @@ -238,6 +259,10 @@ export function projectRoutes(db: Db) { return; } assertCompanyAccess(req, existing.companyId); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectProjectWorkspaceCommandPaths(req.body), + ); const workspaceExists = (await svc.listWorkspaces(id)).some((workspace) => workspace.id === workspaceId); if (!workspaceExists) { res.status(404).json({ error: "Project workspace not found" }); @@ -290,6 +315,11 @@ export function projectRoutes(db: Db) { return; } + await assertCanManageProjectWorkspaceRuntimeServices(db, req, { + companyId: project.companyId, + projectWorkspaceId: workspace.id, + }); + const workspaceCwd = workspace.cwd; if (!workspaceCwd) { res.status(422).json({ error: "Project workspace needs a local path before Paperclip can run workspace commands" }); diff --git a/server/src/routes/workspace-command-authz.ts b/server/src/routes/workspace-command-authz.ts new file mode 100644 index 00000000..e7999d11 --- /dev/null +++ b/server/src/routes/workspace-command-authz.ts @@ -0,0 +1,115 @@ +import type { Request } from "express"; +import { forbidden } from "../errors.js"; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function hasOwn(value: Record, key: string) { + return Object.prototype.hasOwnProperty.call(value, key); +} + +function prefixPath(prefix: string, key: string) { + return prefix.length > 0 ? `${prefix}.${key}` : key; +} + +function collectWorkspaceStrategyCommandPaths(raw: unknown, prefix: string): string[] { + if (!isRecord(raw)) return []; + const paths: string[] = []; + if (hasOwn(raw, "provisionCommand")) { + paths.push(prefixPath(prefix, "provisionCommand")); + } + if (hasOwn(raw, "teardownCommand")) { + paths.push(prefixPath(prefix, "teardownCommand")); + } + return paths; +} + +function collectExecutionWorkspaceConfigCommandPaths(raw: unknown, prefix: string): string[] { + if (!isRecord(raw)) return []; + const paths: string[] = []; + if (hasOwn(raw, "provisionCommand")) { + paths.push(prefixPath(prefix, "provisionCommand")); + } + if (hasOwn(raw, "teardownCommand")) { + paths.push(prefixPath(prefix, "teardownCommand")); + } + if (hasOwn(raw, "cleanupCommand")) { + paths.push(prefixPath(prefix, "cleanupCommand")); + } + return paths; +} + +export function assertNoAgentHostWorkspaceCommandMutation(req: Request, paths: string[]) { + if (req.actor.type !== "agent" || paths.length === 0) return; + throw forbidden( + `Agent keys cannot modify host-executed workspace commands (${paths.join(", ")}).`, + ); +} + +export function collectAgentAdapterWorkspaceCommandPaths(adapterConfig: unknown): string[] { + if (!isRecord(adapterConfig)) return []; + return collectWorkspaceStrategyCommandPaths( + adapterConfig.workspaceStrategy, + "adapterConfig.workspaceStrategy", + ); +} + +export function collectProjectExecutionWorkspaceCommandPaths(policy: unknown): string[] { + if (!isRecord(policy)) return []; + return collectWorkspaceStrategyCommandPaths( + policy.workspaceStrategy, + "executionWorkspacePolicy.workspaceStrategy", + ); +} + +export function collectProjectWorkspaceCommandPaths( + workspacePatch: unknown, + prefix = "", +): string[] { + if (!isRecord(workspacePatch)) return []; + return hasOwn(workspacePatch, "cleanupCommand") + ? [prefixPath(prefix, "cleanupCommand")] + : []; +} + +export function collectIssueWorkspaceCommandPaths(input: { + executionWorkspaceSettings?: unknown; + assigneeAdapterOverrides?: unknown; +}): string[] { + const paths: string[] = []; + if (isRecord(input.executionWorkspaceSettings)) { + paths.push( + ...collectWorkspaceStrategyCommandPaths( + input.executionWorkspaceSettings.workspaceStrategy, + "executionWorkspaceSettings.workspaceStrategy", + ), + ); + } + if (isRecord(input.assigneeAdapterOverrides)) { + const adapterConfig = input.assigneeAdapterOverrides.adapterConfig; + if (isRecord(adapterConfig)) { + paths.push( + ...collectWorkspaceStrategyCommandPaths( + adapterConfig.workspaceStrategy, + "assigneeAdapterOverrides.adapterConfig.workspaceStrategy", + ), + ); + } + } + return paths; +} + +export function collectExecutionWorkspaceCommandPaths(input: { + config?: unknown; + metadata?: unknown; +}): string[] { + const paths: string[] = []; + if (input.config !== undefined) { + paths.push(...collectExecutionWorkspaceConfigCommandPaths(input.config, "config")); + } + if (isRecord(input.metadata) && hasOwn(input.metadata, "config")) { + paths.push(...collectExecutionWorkspaceConfigCommandPaths(input.metadata.config, "metadata.config")); + } + return paths; +} diff --git a/server/src/routes/workspace-runtime-service-authz.ts b/server/src/routes/workspace-runtime-service-authz.ts new file mode 100644 index 00000000..16cff277 --- /dev/null +++ b/server/src/routes/workspace-runtime-service-authz.ts @@ -0,0 +1,138 @@ +import { and, eq, inArray, isNull, ne, or } from "drizzle-orm"; +import type { Db } from "@paperclipai/db"; +import { agents, issues } from "@paperclipai/db"; +import type { Request } from "express"; +import { forbidden } from "../errors.js"; +import { assertCompanyAccess } from "./authz.js"; + +const WORKSPACE_RUNTIME_ELIGIBLE_ISSUE_STATUSES: string[] = [ + "backlog", + "todo", + "in_progress", + "in_review", + "blocked", +]; + +async function listReportingSubtreeAgentIds(db: Db, companyId: string, actorAgentId: string) { + const companyAgents = await db + .select({ + id: agents.id, + reportsTo: agents.reportsTo, + }) + .from(agents) + .where(and(eq(agents.companyId, companyId), ne(agents.status, "terminated"))); + + const reportsByManager = new Map(); + for (const agent of companyAgents) { + if (!agent.reportsTo) continue; + const reports = reportsByManager.get(agent.reportsTo) ?? []; + reports.push(agent.id); + reportsByManager.set(agent.reportsTo, reports); + } + + const visited = new Set([actorAgentId]); + const queue = [actorAgentId]; + while (queue.length > 0) { + const current = queue.shift(); + if (!current) continue; + const reports = reportsByManager.get(current) ?? []; + for (const reportId of reports) { + if (visited.has(reportId)) continue; + visited.add(reportId); + queue.push(reportId); + } + } + + return [...visited]; +} + +async function assertAgentCanManageRuntimeServicesForWorkspace( + db: Db, + req: Request, + input: { + companyId: string; + projectWorkspaceId?: string | null; + executionWorkspaceId?: string | null; + sourceIssueId?: string | null; + }, +) { + if (req.actor.type !== "agent" || !req.actor.agentId) { + throw forbidden("Agent authentication required"); + } + + const actorAgent = await db + .select({ + id: agents.id, + companyId: agents.companyId, + role: agents.role, + }) + .from(agents) + .where(eq(agents.id, req.actor.agentId)) + .then((rows) => rows[0] ?? null); + + if (!actorAgent || actorAgent.companyId !== input.companyId) { + throw forbidden("Agent key cannot access another company"); + } + + if (actorAgent.role === "ceo") { + return; + } + + const eligibleAgentIds = await listReportingSubtreeAgentIds(db, input.companyId, actorAgent.id); + const workspaceScopeConditions = [ + input.projectWorkspaceId ? eq(issues.projectWorkspaceId, input.projectWorkspaceId) : null, + input.executionWorkspaceId ? eq(issues.executionWorkspaceId, input.executionWorkspaceId) : null, + input.sourceIssueId ? eq(issues.id, input.sourceIssueId) : null, + ].filter((condition): condition is NonNullable => condition !== null); + + if (workspaceScopeConditions.length === 0) { + throw forbidden("Missing permission to manage workspace runtime services"); + } + + const linkedIssue = await db + .select({ id: issues.id }) + .from(issues) + .where(and( + eq(issues.companyId, input.companyId), + isNull(issues.hiddenAt), + inArray(issues.status, WORKSPACE_RUNTIME_ELIGIBLE_ISSUE_STATUSES), + inArray(issues.assigneeAgentId, eligibleAgentIds), + workspaceScopeConditions.length === 1 + ? workspaceScopeConditions[0]! + : or(...workspaceScopeConditions), + )) + .then((rows) => rows[0] ?? null); + + if (linkedIssue) { + return; + } + + throw forbidden("Missing permission to manage workspace runtime services"); +} + +export async function assertCanManageProjectWorkspaceRuntimeServices( + db: Db, + req: Request, + input: { + companyId: string; + projectWorkspaceId: string; + }, +) { + assertCompanyAccess(req, input.companyId); + if (req.actor.type === "board") return; + await assertAgentCanManageRuntimeServicesForWorkspace(db, req, input); +} + +export async function assertCanManageExecutionWorkspaceRuntimeServices( + db: Db, + req: Request, + input: { + companyId: string; + executionWorkspaceId: string; + sourceIssueId?: string | null; + }, +) { + assertCompanyAccess(req, input.companyId); + if (req.actor.type === "board") return; + await assertAgentCanManageRuntimeServicesForWorkspace(db, req, input); +} diff --git a/server/src/services/agents.ts b/server/src/services/agents.ts index abba7414..6cacb441 100644 --- a/server/src/services/agents.ts +++ b/server/src/services/agents.ts @@ -619,11 +619,25 @@ export function agentService(db: Db) { .from(agentApiKeys) .where(eq(agentApiKeys.agentId, id)), - revokeKey: async (keyId: string) => { + getKeyById: async (keyId: string) => + db + .select({ + id: agentApiKeys.id, + agentId: agentApiKeys.agentId, + companyId: agentApiKeys.companyId, + name: agentApiKeys.name, + createdAt: agentApiKeys.createdAt, + revokedAt: agentApiKeys.revokedAt, + }) + .from(agentApiKeys) + .where(eq(agentApiKeys.id, keyId)) + .then((rows) => rows[0] ?? null), + + revokeKey: async (agentId: string, keyId: string) => { const rows = await db .update(agentApiKeys) .set({ revokedAt: new Date() }) - .where(eq(agentApiKeys.id, keyId)) + .where(and(eq(agentApiKeys.id, keyId), eq(agentApiKeys.agentId, agentId))) .returning(); return rows[0] ?? null; }, diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index ff29a180..305f6a76 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -47,7 +47,9 @@ import { readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference, } from "@paperclipai/adapter-utils/server-utils"; -import { notFound, unprocessable } from "../errors.js"; +import { ensureOpenCodeModelConfiguredAndAvailable } from "@paperclipai/adapter-opencode-local/server"; +import { findServerAdapter } from "../adapters/index.js"; +import { forbidden, notFound, unprocessable } from "../errors.js"; import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js"; import type { StorageService } from "../storage/types.js"; import { accessService } from "./access.js"; @@ -62,6 +64,7 @@ import { validateCron } from "./cron.js"; import { issueService } from "./issues.js"; import { projectService } from "./projects.js"; import { routineService } from "./routines.js"; +import { secretService } from "./secrets.js"; /** Build OrgNode tree from manifest agent list (slug + reportsToSlug). */ function buildOrgTreeFromManifest(agents: CompanyPortabilityManifest["agents"]): OrgNode[] { @@ -117,6 +120,7 @@ const DEFAULT_INCLUDE: CompanyPortabilityInclude = { }; const DEFAULT_COLLISION_STRATEGY: CompanyPortabilityCollisionStrategy = "rename"; +const IMPORT_FORBIDDEN_ADAPTER_TYPES = new Set(["process", "http"]); const execFileAsync = promisify(execFile); let bundledSkillsCommitPromise: Promise | null = null; @@ -2747,6 +2751,94 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const projects = projectService(db); const issues = issueService(db); const companySkills = companySkillService(db); + const secrets = secretService(db); + const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true"; + + function assertKnownImportAdapterType(type: string | null | undefined): string { + const adapterType = typeof type === "string" ? type.trim() : ""; + if (!adapterType) { + throw unprocessable("Adapter type is required"); + } + if (!findServerAdapter(adapterType)) { + throw unprocessable(`Unknown adapter type: ${adapterType}`); + } + return adapterType; + } + + async function assertImportAdapterConfigConstraints( + companyId: string, + adapterType: string, + adapterConfig: Record, + ) { + if (adapterType !== "opencode_local") return; + const { config: runtimeConfig } = await secrets.resolveAdapterConfigForRuntime(companyId, adapterConfig); + const runtimeEnv = isPlainRecord(runtimeConfig.env) ? runtimeConfig.env : {}; + try { + await ensureOpenCodeModelConfiguredAndAvailable({ + model: runtimeConfig.model, + command: runtimeConfig.command, + cwd: runtimeConfig.cwd, + env: runtimeEnv, + }); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw unprocessable(`Invalid opencode_local adapterConfig: ${reason}`); + } + } + + async function prepareImportedAgentAdapter( + companyId: string, + adapterType: string | null | undefined, + adapterConfig: Record, + desiredSkills: string[], + mode: ImportMode, + ) { + const effectiveAdapterType = assertKnownImportAdapterType(adapterType); + if (mode === "agent_safe" && IMPORT_FORBIDDEN_ADAPTER_TYPES.has(effectiveAdapterType)) { + throw forbidden(`Adapter type "${effectiveAdapterType}" is not allowed in safe imports`); + } + const nextAdapterConfig = writePaperclipSkillSyncPreference({ ...adapterConfig }, desiredSkills); + delete nextAdapterConfig.promptTemplate; + delete nextAdapterConfig.bootstrapPromptTemplate; + delete nextAdapterConfig.instructionsFilePath; + delete nextAdapterConfig.instructionsBundleMode; + delete nextAdapterConfig.instructionsRootPath; + delete nextAdapterConfig.instructionsEntryFile; + const normalizedAdapterConfig = await secrets.normalizeAdapterConfigForPersistence( + companyId, + nextAdapterConfig, + { strictMode: strictSecretsMode }, + ); + await assertImportAdapterConfigConstraints(companyId, effectiveAdapterType, normalizedAdapterConfig); + return { + adapterType: effectiveAdapterType, + adapterConfig: normalizedAdapterConfig, + }; + } + + function resolveImportedAssigneeAgentId( + assigneeSlug: string | null | undefined, + importedSlugToAgentId: Map, + existingSlugToAgentId: Map, + agentStatusById: Map, + warnings: string[], + subjectLabel: string, + ) { + if (!assigneeSlug) return null; + const assigneeAgentId = + importedSlugToAgentId.get(assigneeSlug) + ?? existingSlugToAgentId.get(assigneeSlug) + ?? null; + if (!assigneeAgentId) return null; + const assigneeStatus = agentStatusById.get(assigneeAgentId) ?? null; + if (assigneeStatus === "pending_approval" || assigneeStatus === "terminated") { + warnings.push( + `${subjectLabel} assignee ${assigneeSlug} is ${assigneeStatus}; imported work was left unassigned.`, + ); + return null; + } + return assigneeAgentId; + } async function resolveSource(source: CompanyPortabilityPreview["source"]): Promise { if (source.type === "inline") { @@ -3856,7 +3948,11 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const warnings = [...plan.preview.warnings]; const include = plan.include; - let targetCompany: { id: string; name: string } | null = null; + let targetCompany: { + id: string; + name: string; + requireBoardApprovalForNewAgents?: boolean | null; + } | null = null; let companyAction: "created" | "updated" | "unchanged" = "unchanged"; if (input.target.mode === "new_company") { @@ -3977,9 +4073,11 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const resultProjects: CompanyPortabilityImportResult["projects"] = []; const importedSlugToAgentId = new Map(); const existingSlugToAgentId = new Map(); + const agentStatusById = new Map(); const existingAgents = await agents.list(targetCompany.id); for (const existing of existingAgents) { existingSlugToAgentId.set(normalizeAgentUrlKey(existing.name) ?? existing.id, existing.id); + agentStatusById.set(existing.id, existing.status); } const importedSlugToProjectId = new Map(); const importedProjectWorkspaceIdByProjectSlug = new Map>(); @@ -4049,22 +4147,18 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { // Apply adapter overrides from request if present const adapterOverride = input.adapterOverrides?.[planAgent.slug]; - const effectiveAdapterType = adapterOverride?.adapterType ?? manifestAgent.adapterType; const baseAdapterConfig = adapterOverride?.adapterConfig ? { ...adapterOverride.adapterConfig } : { ...manifestAgent.adapterConfig } as Record; const desiredSkills = (manifestAgent.skills ?? []).map((skillRef) => desiredSkillRefMap.get(skillRef) ?? skillRef); - const adapterConfigWithSkills = writePaperclipSkillSyncPreference( + const normalizedAdapter = await prepareImportedAgentAdapter( + targetCompany.id, + adapterOverride?.adapterType ?? manifestAgent.adapterType, baseAdapterConfig, desiredSkills, + mode, ); - delete adapterConfigWithSkills.promptTemplate; - delete adapterConfigWithSkills.bootstrapPromptTemplate; // deprecated - delete adapterConfigWithSkills.instructionsFilePath; - delete adapterConfigWithSkills.instructionsBundleMode; - delete adapterConfigWithSkills.instructionsRootPath; - delete adapterConfigWithSkills.instructionsEntryFile; const patch = { name: planAgent.plannedName, role: manifestAgent.role, @@ -4072,8 +4166,8 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { icon: manifestAgent.icon, capabilities: manifestAgent.capabilities, reportsTo: null, - adapterType: effectiveAdapterType, - adapterConfig: adapterConfigWithSkills, + adapterType: normalizedAdapter.adapterType, + adapterConfig: normalizedAdapter.adapterConfig, runtimeConfig: disableImportedTimerHeartbeat(manifestAgent.runtimeConfig), budgetMonthlyCents: manifestAgent.budgetMonthlyCents, permissions: manifestAgent.permissions, @@ -4102,6 +4196,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } catch (err) { warnings.push(`Failed to materialize instructions bundle for ${manifestAgent.slug}: ${err instanceof Error ? err.message : String(err)}`); } + agentStatusById.set(updated.id, updated.status ?? agentStatusById.get(updated.id) ?? null); importedSlugToAgentId.set(planAgent.slug, updated.id); existingSlugToAgentId.set(normalizeAgentUrlKey(updated.name) ?? updated.id, updated.id); resultAgents.push({ @@ -4114,7 +4209,17 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { continue; } - let created = await agents.create(targetCompany.id, patch); + const requiresApproval = + typeof targetCompany.requireBoardApprovalForNewAgents === "boolean" + ? targetCompany.requireBoardApprovalForNewAgents + : include.company + ? (sourceManifest.company?.requireBoardApprovalForNewAgents ?? true) + : true; + const createdStatus = requiresApproval ? "pending_approval" : "idle"; + let created = await agents.create(targetCompany.id, { + ...patch, + status: createdStatus, + }); await access.ensureMembership(targetCompany.id, "agent", created.id, "member", "active"); await access.setPrincipalPermission( targetCompany.id, @@ -4133,6 +4238,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } catch (err) { warnings.push(`Failed to materialize instructions bundle for ${manifestAgent.slug}: ${err instanceof Error ? err.message : String(err)}`); } + agentStatusById.set(created.id, created.status ?? createdStatus); importedSlugToAgentId.set(planAgent.slug, created.id); existingSlugToAgentId.set(normalizeAgentUrlKey(created.name) ?? created.id, created.id); resultAgents.push({ @@ -4275,11 +4381,14 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { const markdownRaw = readPortableTextFile(plan.source.files, manifestIssue.path); const parsed = markdownRaw ? parseFrontmatterMarkdown(markdownRaw) : null; const description = parsed?.body || manifestIssue.description || null; - const assigneeAgentId = manifestIssue.assigneeAgentSlug - ? importedSlugToAgentId.get(manifestIssue.assigneeAgentSlug) - ?? existingSlugToAgentId.get(manifestIssue.assigneeAgentSlug) - ?? null - : null; + const assigneeAgentId = resolveImportedAssigneeAgentId( + manifestIssue.assigneeAgentSlug, + importedSlugToAgentId, + existingSlugToAgentId, + agentStatusById, + warnings, + `Task ${manifestIssue.slug}`, + ); const projectId = manifestIssue.projectSlug ? importedSlugToProjectId.get(manifestIssue.projectSlug) ?? existingProjectSlugToId.get(manifestIssue.projectSlug) @@ -4292,8 +4401,8 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { warnings.push(`Task ${manifestIssue.slug} references workspace key ${manifestIssue.projectWorkspaceKey}, but that workspace was not imported.`); } if (manifestIssue.recurring) { - if (!projectId || !assigneeAgentId) { - throw unprocessable(`Recurring task ${manifestIssue.slug} is missing the project or assignee required to create a routine.`); + if (!projectId) { + throw unprocessable(`Recurring task ${manifestIssue.slug} is missing the project required to create a routine.`); } const resolvedRoutine = resolvePortableRoutineDefinition(manifestIssue, parsed?.frontmatter.schedule); if (resolvedRoutine.errors.length > 0) { @@ -4373,15 +4482,20 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } continue; } + let issueStatus = manifestIssue.status && ISSUE_STATUSES.includes(manifestIssue.status as any) + ? manifestIssue.status as typeof ISSUE_STATUSES[number] + : "backlog"; + if (!assigneeAgentId && issueStatus === "in_progress") { + warnings.push(`Task ${manifestIssue.slug} was downgraded to todo because its assignee could not be imported as assignable work.`); + issueStatus = "todo"; + } await issues.create(targetCompany.id, { projectId, projectWorkspaceId, title: manifestIssue.title, description, assigneeAgentId, - status: manifestIssue.status && ISSUE_STATUSES.includes(manifestIssue.status as any) - ? manifestIssue.status as typeof ISSUE_STATUSES[number] - : "backlog", + status: issueStatus, priority: manifestIssue.priority && ISSUE_PRIORITIES.includes(manifestIssue.priority as any) ? manifestIssue.priority as typeof ISSUE_PRIORITIES[number] : "medium", diff --git a/ui/src/components/InlineEditor.test.tsx b/ui/src/components/InlineEditor.test.tsx index 1f474f7b..830d2ced 100644 --- a/ui/src/components/InlineEditor.test.tsx +++ b/ui/src/components/InlineEditor.test.tsx @@ -164,6 +164,62 @@ describe("InlineEditor", () => { }); outside.remove(); }); + + it("syncs a new multiline value while focused when the user has not edited locally", () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + + act(() => { + root.render(); + }); + + const textarea = container.querySelector('[data-testid="multiline-md-mock"]'); + expect(textarea).not.toBeNull(); + expect(textarea?.value).toBe(""); + + act(() => { + textarea!.focus(); + }); + + act(() => { + root.render(); + }); + + expect(textarea?.value).toBe("Loaded description"); + + act(() => { + root.unmount(); + }); + }); + + it("preserves focused multiline local edits when the prop value changes underneath them", () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + + act(() => { + root.render(); + }); + + const textarea = container.querySelector('[data-testid="multiline-md-mock"]'); + expect(textarea).not.toBeNull(); + + act(() => { + textarea!.focus(); + }); + act(() => { + setNativeTextareaValue(textarea!, "Local draft"); + }); + + act(() => { + root.render(); + }); + + expect(textarea?.value).toBe("Local draft"); + + act(() => { + root.unmount(); + }); + }); }); describe("queueContainedBlurCommit", () => { diff --git a/ui/src/components/InlineEditor.tsx b/ui/src/components/InlineEditor.tsx index f509fe7a..2c878b3f 100644 --- a/ui/src/components/InlineEditor.tsx +++ b/ui/src/components/InlineEditor.tsx @@ -54,6 +54,7 @@ export function InlineEditor({ const [editing, setEditing] = useState(false); const [multilineFocused, setMultilineFocused] = useState(false); const [draft, setDraft] = useState(value); + const lastPropValueRef = useRef(value); const inputRef = useRef(null); const markdownRef = useRef(null); const autosaveDebounceRef = useRef | null>(null); @@ -66,8 +67,14 @@ export function InlineEditor({ } = useAutosaveIndicator(); useEffect(() => { - if (multiline && multilineFocused) return; - setDraft(value); + const previousValue = lastPropValueRef.current; + lastPropValueRef.current = value; + setDraft((currentDraft) => { + if (multiline && multilineFocused && currentDraft !== previousValue) { + return currentDraft; + } + return value; + }); }, [value, multiline, multilineFocused]); useEffect(() => { diff --git a/ui/src/components/IssueLinkQuicklook.tsx b/ui/src/components/IssueLinkQuicklook.tsx index b6311eff..1d66b2d0 100644 --- a/ui/src/components/IssueLinkQuicklook.tsx +++ b/ui/src/components/IssueLinkQuicklook.tsx @@ -6,8 +6,7 @@ import { useQuery, useQueryClient } from "@tanstack/react-query"; import { timeAgo } from "@/lib/timeAgo"; import { createIssueDetailPath, withIssueDetailHeaderSeed } from "@/lib/issueDetailBreadcrumb"; import { - fetchIssueDetail, - getCachedIssueDetail, + getIssueDetailQueryOptions, ISSUE_DETAIL_STALE_TIME_MS, prefetchIssueDetail, } from "@/lib/issueDetailCache"; @@ -98,12 +97,9 @@ export const IssueLinkQuicklook = React.forwardRef< const queryClient = useQueryClient(); const [open, setOpen] = useState(false); const prefetchedState = issuePrefetch ? withIssueDetailHeaderSeed(state, issuePrefetch) : state; - const cachedIssue = getCachedIssueDetail(queryClient, issuePathId, issuePrefetch ?? undefined); const { data, isLoading } = useQuery({ - queryKey: queryKeys.issues.detail(issuePathId), - queryFn: () => fetchIssueDetail(queryClient, issuePathId), + ...getIssueDetailQueryOptions(queryClient, issuePathId, { placeholderIssue: issuePrefetch ?? undefined }), enabled: open, - initialData: () => cachedIssue, staleTime: ISSUE_DETAIL_STALE_TIME_MS, }); diff --git a/ui/src/components/MarkdownBody.test.tsx b/ui/src/components/MarkdownBody.test.tsx index 814d83b0..ebbf6953 100644 --- a/ui/src/components/MarkdownBody.test.tsx +++ b/ui/src/components/MarkdownBody.test.tsx @@ -96,6 +96,13 @@ describe("MarkdownBody", () => { expect(html).toContain('data-mention-kind="skill"'); }); + it("sanitizes unsafe javascript markdown links", () => { + const html = renderMarkdown("[click me](javascript:alert(document.cookie))"); + + expect(html).toContain('click me'); + expect(html).not.toContain("javascript:"); + }); + it("uses soft-break styling by default", () => { const html = renderMarkdown("First line\nSecond line"); diff --git a/ui/src/components/MarkdownBody.tsx b/ui/src/components/MarkdownBody.tsx index c9fdf859..642fb798 100644 --- a/ui/src/components/MarkdownBody.tsx +++ b/ui/src/components/MarkdownBody.tsx @@ -1,6 +1,6 @@ import { isValidElement, useEffect, useId, useState, type ReactNode } from "react"; import { useQuery } from "@tanstack/react-query"; -import Markdown, { type Components, type Options } from "react-markdown"; +import Markdown, { defaultUrlTransform, type Components, type Options } from "react-markdown"; import remarkGfm from "remark-gfm"; import { cn } from "../lib/utils"; import { useTheme } from "../context/ThemeContext"; @@ -71,6 +71,10 @@ function extractMermaidSource(children: ReactNode): string | null { return flattenText(childProps.children).replace(/\n$/, ""); } +function safeMarkdownUrlTransform(url: string): string { + return parseMentionChipHref(url) ? url : defaultUrlTransform(url); +} + function MermaidDiagramBlock({ source, darkMode }: { source: string; darkMode: boolean }) { const renderId = useId().replace(/[^a-zA-Z0-9_-]/g, ""); const [svg, setSvg] = useState(null); @@ -215,7 +219,11 @@ export function MarkdownBody({ )} style={style} > - url}> + {children} diff --git a/ui/src/components/MarkdownEditor.test.tsx b/ui/src/components/MarkdownEditor.test.tsx index 106a394b..976ad51c 100644 --- a/ui/src/components/MarkdownEditor.test.tsx +++ b/ui/src/components/MarkdownEditor.test.tsx @@ -19,6 +19,7 @@ const mdxEditorMockState = vi.hoisted(() => ({ emitMountParseError: false, emitMountSilentEmptyState: false, markdownValues: [] as string[], + suppressHtmlProcessingValues: [] as boolean[], })); vi.mock("@mdxeditor/editor", async () => { @@ -41,16 +42,19 @@ vi.mock("@mdxeditor/editor", async () => { onChange, onError, className, + suppressHtmlProcessing, }: { markdown: string; placeholder?: string; onChange?: (value: string) => void; onError?: (error: unknown) => void; + suppressHtmlProcessing?: boolean; className?: string; }, forwardedRef: React.ForwardedRef<{ setMarkdown: (value: string) => void; focus: () => void } | null>, ) { mdxEditorMockState.markdownValues.push(markdown); + mdxEditorMockState.suppressHtmlProcessingValues.push(Boolean(suppressHtmlProcessing)); const [content, setContent] = React.useState(markdown); const editableRef = React.useRef(null); const handle = React.useMemo(() => ({ @@ -59,8 +63,16 @@ vi.mock("@mdxeditor/editor", async () => { }), []); React.useEffect(() => { + if (!suppressHtmlProcessing && markdown.includes(" { setForwardedRef(forwardedRef, null); @@ -165,6 +177,7 @@ describe("MarkdownEditor", () => { mdxEditorMockState.emitMountParseError = false; mdxEditorMockState.emitMountSilentEmptyState = false; mdxEditorMockState.markdownValues = []; + mdxEditorMockState.suppressHtmlProcessingValues = []; }); it("applies async external value updates once the editor ref becomes ready", async () => { @@ -238,6 +251,7 @@ describe("MarkdownEditor", () => { await flush(); expect(mdxEditorMockState.markdownValues.at(-1)).toContain("![image](https://example.com/test.png)"); expect(mdxEditorMockState.markdownValues.at(-1)).not.toContain(" { }); await flush(); - await vi.waitFor(() => { expect(container.querySelector("textarea")).not.toBeNull(); }); - const textarea = container.querySelector("textarea"); expect(textarea).not.toBeNull(); expect(textarea?.value).toBe("Affected versions: <= v0.3.1"); @@ -294,11 +306,9 @@ describe("MarkdownEditor", () => { }); await flush(); - await vi.waitFor(() => { expect(container.querySelector("textarea")).not.toBeNull(); }); - const textarea = container.querySelector("textarea"); expect(textarea).not.toBeNull(); expect(textarea?.value).toBe("Affected versions: <= v0.3.1"); @@ -309,7 +319,6 @@ describe("MarkdownEditor", () => { root.unmount(); }); }); - it("anchors the mention menu inside the visual viewport when mobile offsets are present", () => { expect( computeMentionMenuPosition( diff --git a/ui/src/lib/issueDetailCache.ts b/ui/src/lib/issueDetailCache.ts index 8e02e46e..a2770b31 100644 --- a/ui/src/lib/issueDetailCache.ts +++ b/ui/src/lib/issueDetailCache.ts @@ -84,6 +84,20 @@ export async function fetchIssueDetail( return seedIssueDetailCache(queryClient, issue, { issueRef }); } +export function getIssueDetailQueryOptions( + queryClient: QueryClient, + issueRef: string, + options?: { + placeholderIssue?: Pick | null; + }, +) { + return { + queryKey: queryKeys.issues.detail(issueRef), + queryFn: () => fetchIssueDetail(queryClient, issueRef), + placeholderData: getCachedIssueDetail(queryClient, issueRef, options?.placeholderIssue ?? undefined), + }; +} + export function prefetchIssueDetail( queryClient: QueryClient, issueRef: string, diff --git a/ui/src/lib/issueDetailQuery.test.tsx b/ui/src/lib/issueDetailQuery.test.tsx new file mode 100644 index 00000000..83be05dd --- /dev/null +++ b/ui/src/lib/issueDetailQuery.test.tsx @@ -0,0 +1,129 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import type { Issue } from "@paperclipai/shared"; +import { QueryClient, QueryClientProvider, useQuery, useQueryClient } from "@tanstack/react-query"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { issuesApi } from "@/api/issues"; +import { queryKeys } from "@/lib/queryKeys"; +import { getIssueDetailQueryOptions } from "./issueDetailCache"; + +vi.mock("@/api/issues", () => ({ + issuesApi: { + get: vi.fn(), + }, +})); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +function makeIssue(overrides: Partial = {}): Issue { + const now = new Date("2026-04-13T20:00:00.000Z"); + return { + id: "issue-1", + companyId: "company-1", + projectId: null, + projectWorkspaceId: null, + goalId: null, + parentId: null, + title: "Issue title", + description: null, + status: "todo", + priority: "medium", + assigneeAgentId: null, + assigneeUserId: null, + checkoutRunId: null, + executionRunId: null, + executionAgentNameKey: null, + executionLockedAt: null, + createdByAgentId: null, + createdByUserId: null, + issueNumber: 1442, + identifier: "PAP-1442", + requestDepth: 0, + billingCode: null, + assigneeAdapterOverrides: null, + executionWorkspaceId: null, + executionWorkspacePreference: null, + executionWorkspaceSettings: null, + startedAt: null, + completedAt: null, + cancelledAt: null, + hiddenAt: null, + createdAt: now, + updatedAt: now, + ...overrides, + }; +} + +function IssueDetailQueryHarness({ + issueRef, + placeholderIssue, +}: { + issueRef: string; + placeholderIssue?: Pick | null; +}) { + const queryClient = useQueryClient(); + const query = useQuery({ + ...getIssueDetailQueryOptions(queryClient, issueRef, { placeholderIssue }), + }); + + return
{query.data?.description ?? "EMPTY"}
; +} + +async function flush() { + // Multiple act cycles to allow React Query to process the async queryFn + for (let i = 0; i < 5; i++) { + await act(async () => { + await new Promise((r) => setTimeout(r, 0)); + }); + } +} + +describe("getIssueDetailQueryOptions", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it("treats cached issue data as placeholder and still fetches full detail", async () => { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + const partialIssue = makeIssue({ description: null }); + const fullIssue = makeIssue({ description: "GitHub Security Advisory body" }); + + queryClient.setQueryData(queryKeys.issues.detail("issue-1"), partialIssue); + queryClient.setQueryData(queryKeys.issues.detail("PAP-1442"), partialIssue); + vi.mocked(issuesApi.get).mockResolvedValue(fullIssue); + + await act(async () => { + root.render( + + + , + ); + }); + + await flush(); + + expect(issuesApi.get).toHaveBeenCalledWith("PAP-1442"); + expect(container.textContent).toContain("GitHub Security Advisory body"); + + await act(async () => { + root.unmount(); + }); + queryClient.clear(); + container.remove(); + }); +}); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 0b2912b8..de2a390d 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -28,7 +28,8 @@ import { readIssueDetailHeaderSeed, rememberIssueDetailLocationState, } from "../lib/issueDetailBreadcrumb"; -import { fetchIssueDetail, getCachedIssueDetail } from "../lib/issueDetailCache"; +import { resolveIssueActiveRun, shouldTrackIssueActiveRun } from "../lib/issueActiveRun"; +import { getIssueDetailQueryOptions } from "../lib/issueDetailCache"; import { hasBlockingShortcutDialog, resolveIssueDetailGoKeyAction, @@ -882,22 +883,15 @@ export function IssueDetail() { () => readIssueDetailHeaderSeed(location.state) ?? readIssueDetailHeaderSeed(resolvedIssueDetailState), [location.state, resolvedIssueDetailState], ); - const cachedIssue = useMemo( - () => - issueId - ? getCachedIssueDetail(queryClient, issueId, issueHeaderSeed ? { - id: issueHeaderSeed.id, - identifier: issueHeaderSeed.identifier, - } : null) - : undefined, - [issueHeaderSeed, issueId, queryClient], - ); const { data: issue, isLoading, error } = useQuery({ - queryKey: queryKeys.issues.detail(issueId!), - queryFn: () => fetchIssueDetail(queryClient, issueId!), + ...getIssueDetailQueryOptions(queryClient, issueId!, { + placeholderIssue: issueHeaderSeed ? { + id: issueHeaderSeed.id, + identifier: issueHeaderSeed.identifier, + } : null, + }), enabled: !!issueId, - initialData: () => cachedIssue, }); const resolvedCompanyId = issue?.companyId ?? selectedCompanyId; const commentComposerDisabledReason = useMemo(() => {