diff --git a/server/src/__tests__/activity-routes.test.ts b/server/src/__tests__/activity-routes.test.ts index 86ee374d..ffd343df 100644 --- a/server/src/__tests__/activity-routes.test.ts +++ b/server/src/__tests__/activity-routes.test.ts @@ -10,6 +10,10 @@ const mockActivityService = vi.hoisted(() => ({ create: vi.fn(), })); +const mockHeartbeatService = vi.hoisted(() => ({ + getRun: vi.fn(), +})); + const mockIssueService = vi.hoisted(() => ({ getById: vi.fn(), getByIdentifier: vi.fn(), @@ -22,6 +26,7 @@ function registerRouteMocks() { vi.doMock("../services/index.js", () => ({ issueService: () => mockIssueService, + heartbeatService: () => mockHeartbeatService, })); } @@ -75,4 +80,32 @@ describe("activity routes", () => { expect(mockActivityService.runsForIssue).toHaveBeenCalledWith("company-1", "issue-uuid-1"); expect(res.body).toEqual([{ runId: "run-1", adapterType: "codex_local" }]); }); + + it("requires company access before creating activity events", async () => { + const app = await createApp(); + const res = await request(app) + .post("/api/companies/company-2/activity") + .send({ + actorId: "user-1", + action: "test.event", + entityType: "issue", + entityId: "issue-1", + }); + + expect(res.status).toBe(403); + expect(mockActivityService.create).not.toHaveBeenCalled(); + }); + + it("requires company access before listing issues for another company's run", async () => { + mockHeartbeatService.getRun.mockResolvedValue({ + id: "run-2", + companyId: "company-2", + }); + + const app = await createApp(); + const res = await request(app).get("/api/heartbeat-runs/run-2/issues"); + + expect(res.status).toBe(403); + expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 6ef45db1..c5593f03 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -59,6 +59,8 @@ const mockBudgetService = vi.hoisted(() => ({ const mockHeartbeatService = vi.hoisted(() => ({ listTaskSessions: vi.fn(), resetRuntimeSession: vi.fn(), + getRun: vi.fn(), + cancelRun: vi.fn(), })); const mockIssueApprovalService = vi.hoisted(() => ({ @@ -397,4 +399,26 @@ describe("agent permission routes", () => { }, ]); }); + + it("rejects heartbeat cancellation outside the caller company scope", async () => { + mockHeartbeatService.getRun.mockResolvedValue({ + id: "run-1", + companyId: "33333333-3333-4333-8333-333333333333", + agentId, + status: "running", + }); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app).post("/api/heartbeat-runs/run-1/cancel").send({}); + + expect(res.status).toBe(403); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/approval-routes-idempotency.test.ts b/server/src/__tests__/approval-routes-idempotency.test.ts index 83d34cf1..cecee0e8 100644 --- a/server/src/__tests__/approval-routes-idempotency.test.ts +++ b/server/src/__tests__/approval-routes-idempotency.test.ts @@ -39,7 +39,7 @@ vi.mock("../services/index.js", () => ({ secretService: () => mockSecretService, })); -function createApp() { +function createApp(actorOverrides: Record = {}) { const app = express(); app.use(express.json()); app.use((req, _res, next) => { @@ -49,6 +49,7 @@ function createApp() { companyIds: ["company-1"], source: "session", isInstanceAdmin: false, + ...actorOverrides, }; next(); }); @@ -84,6 +85,14 @@ describe("approval routes idempotent retries", () => { }); it("does not emit duplicate approval side effects when approve is already resolved", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-1", + companyId: "company-1", + type: "hire_agent", + status: "approved", + payload: {}, + requestedByAgentId: "agent-1", + }); mockApprovalService.approve.mockResolvedValue({ approval: { id: "approval-1", @@ -107,6 +116,13 @@ describe("approval routes idempotent retries", () => { }); it("does not emit duplicate rejection logs when reject is already resolved", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-1", + companyId: "company-1", + type: "hire_agent", + status: "rejected", + payload: {}, + }); mockApprovalService.reject.mockResolvedValue({ approval: { id: "approval-1", @@ -126,6 +142,40 @@ describe("approval routes idempotent retries", () => { expect(mockLogActivity).not.toHaveBeenCalled(); }); + it("rejects approval decisions for companies outside the caller scope", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-2", + companyId: "company-2", + type: "hire_agent", + status: "pending", + payload: {}, + }); + + const res = await request(createApp()) + .post("/api/approvals/approval-2/approve") + .send({}); + + expect(res.status).toBe(403); + expect(mockApprovalService.approve).not.toHaveBeenCalled(); + }); + + it("rejects approval revision requests for companies outside the caller scope", async () => { + mockApprovalService.getById.mockResolvedValue({ + id: "approval-3", + companyId: "company-2", + type: "hire_agent", + status: "pending", + payload: {}, + }); + + const res = await request(createApp()) + .post("/api/approvals/approval-3/request-revision") + .send({ decisionNote: "Need changes" }); + + expect(res.status).toBe(403); + expect(mockApprovalService.requestRevision).not.toHaveBeenCalled(); + }); + it("lets agents create generic issue-linked board approval requests", async () => { mockApprovalService.create.mockResolvedValue({ id: "approval-1", diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index 8649f631..075f2d9b 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -175,4 +175,50 @@ describe("company portability routes", () => { expect(res.status).toBe(403); expect(res.body.error).toContain("Board access required"); }); + + it("requires instance admin for new-company import preview", async () => { + const app = await createApp({ + type: "board", + userId: "user-1", + companyIds: ["11111111-1111-4111-8111-111111111111"], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .post("/api/companies/import/preview") + .send({ + source: { type: "inline", files: { "COMPANY.md": "---\nname: Test\n---\n" } }, + include: { company: true, agents: true, projects: false, issues: false }, + target: { mode: "new_company", newCompanyName: "Imported Test" }, + collisionStrategy: "rename", + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Instance admin"); + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + }); + + it("requires instance admin for new-company import apply", async () => { + const app = await createApp({ + type: "board", + userId: "user-1", + companyIds: ["11111111-1111-4111-8111-111111111111"], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .post("/api/companies/import") + .send({ + source: { type: "inline", files: { "COMPANY.md": "---\nname: Test\n---\n" } }, + include: { company: true, agents: true, projects: false, issues: false }, + target: { mode: "new_company", newCompanyName: "Imported Test" }, + collisionStrategy: "rename", + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Instance admin"); + expect(mockCompanyPortabilityService.importBundle).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/routes/activity.ts b/server/src/routes/activity.ts index 0884b455..20f2ed53 100644 --- a/server/src/routes/activity.ts +++ b/server/src/routes/activity.ts @@ -4,7 +4,7 @@ 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 { issueService } from "../services/index.js"; +import { heartbeatService, issueService } from "../services/index.js"; import { sanitizeRecord } from "../redaction.js"; const createActivitySchema = z.object({ @@ -20,6 +20,7 @@ const createActivitySchema = z.object({ export function activityRoutes(db: Db) { const router = Router(); const svc = activityService(db); + const heartbeat = heartbeatService(db); const issueSvc = issueService(db); async function resolveIssueByRef(rawId: string) { @@ -46,6 +47,7 @@ export function activityRoutes(db: Db) { router.post("/companies/:companyId/activity", validate(createActivitySchema), async (req, res) => { assertBoard(req); const companyId = req.params.companyId as string; + assertCompanyAccess(req, companyId); const event = await svc.create({ companyId, ...req.body, @@ -80,6 +82,12 @@ export function activityRoutes(db: Db) { router.get("/heartbeat-runs/:runId/issues", async (req, res) => { const runId = req.params.runId as string; + const run = await heartbeat.getRun(runId); + if (!run) { + res.json([]); + return; + } + assertCompanyAccess(req, run.companyId); const result = await svc.issuesForRun(runId); res.json(result); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 266803ec..3d982cdc 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -2296,6 +2296,10 @@ export function agentRoutes(db: Db) { router.post("/heartbeat-runs/:runId/cancel", async (req, res) => { assertBoard(req); const runId = req.params.runId as string; + const existing = await heartbeat.getRun(runId); + if (existing) { + assertCompanyAccess(req, existing.companyId); + } const run = await heartbeat.cancelRun(runId); if (run) { diff --git a/server/src/routes/approvals.ts b/server/src/routes/approvals.ts index 99d33abd..3ed20374 100644 --- a/server/src/routes/approvals.ts +++ b/server/src/routes/approvals.ts @@ -1,4 +1,4 @@ -import { Router } from "express"; +import { Router, type Request } from "express"; import type { Db } from "@paperclipai/db"; import { addApprovalCommentSchema, @@ -34,6 +34,15 @@ export function approvalRoutes(db: Db) { const secretsSvc = secretService(db); const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true"; + async function requireApprovalAccess(req: Request, id: string) { + const approval = await svc.getById(id); + if (!approval) { + return null; + } + assertCompanyAccess(req, approval.companyId); + return approval; + } + router.get("/companies/:companyId/approvals", async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); @@ -121,6 +130,10 @@ export function approvalRoutes(db: Db) { router.post("/approvals/:id/approve", validate(resolveApprovalSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await requireApprovalAccess(req, id))) { + res.status(404).json({ error: "Approval not found" }); + return; + } const { approval, applied } = await svc.approve( id, req.body.decidedByUserId ?? "board", @@ -216,6 +229,10 @@ export function approvalRoutes(db: Db) { router.post("/approvals/:id/reject", validate(resolveApprovalSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await requireApprovalAccess(req, id))) { + res.status(404).json({ error: "Approval not found" }); + return; + } const { approval, applied } = await svc.reject( id, req.body.decidedByUserId ?? "board", @@ -243,6 +260,10 @@ export function approvalRoutes(db: Db) { async (req, res) => { assertBoard(req); const id = req.params.id as string; + if (!(await requireApprovalAccess(req, id))) { + res.status(404).json({ error: "Approval not found" }); + return; + } const approval = await svc.requestRevision( id, req.body.decidedByUserId ?? "board", diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index d1978c9d..22be5f86 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -24,7 +24,7 @@ import { logActivity, } from "../services/index.js"; import type { StorageService } from "../storage/types.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; export function companyRoutes(db: Db, storage?: StorageService) { const router = Router(); @@ -48,6 +48,17 @@ export function companyRoutes(db: Db, storage?: StorageService) { return parsed; } + function assertImportTargetAccess( + req: Request, + target: { mode: "new_company" } | { mode: "existing_company"; companyId: string }, + ) { + if (target.mode === "new_company") { + assertInstanceAdmin(req); + return; + } + assertCompanyAccess(req, target.companyId); + } + async function assertCanUpdateBranding(req: Request, companyId: string) { assertCompanyAccess(req, companyId); if (req.actor.type === "board") return; @@ -160,18 +171,14 @@ export function companyRoutes(db: Db, storage?: StorageService) { router.post("/import/preview", validate(companyPortabilityPreviewSchema), async (req, res) => { assertBoard(req); - if (req.body.target.mode === "existing_company") { - assertCompanyAccess(req, req.body.target.companyId); - } + assertImportTargetAccess(req, req.body.target); const preview = await portability.previewImport(req.body); res.json(preview); }); router.post("/import", validate(companyPortabilityImportSchema), async (req, res) => { assertBoard(req); - if (req.body.target.mode === "existing_company") { - assertCompanyAccess(req, req.body.target.companyId); - } + assertImportTargetAccess(req, req.body.target); const actor = getActorInfo(req); const result = await portability.importBundle(req.body, req.actor.type === "board" ? req.actor.userId : null); await logActivity(db, {