From 93fdb9c2186318b958f3fa3bdc7bd26bd3339689 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 22 Jun 2026 10:34:13 -0500 Subject: [PATCH] [codex] Harden same-company CEO authorization (#8276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Companies are the primary tenant boundary for agents, board users, settings, and portability operations. > - CEO agents need limited same-company management powers, but those powers must not cross into another company. > - The affected company routes mixed route-level access checks with validation middleware and repeated CEO checks, which left adjacent mutation and portability surfaces easy to drift. > - This pull request centralizes same-company CEO-or-board authorization for those company-scoped surfaces. > - It also adds regression coverage proving a CEO agent from one company cannot read, mutate, archive, delete, export, or import against another company. > - The benefit is tighter company isolation without removing legitimate same-company CEO operations. ## Linked Issues or Issue Description Internal task: PAP-11205 / PAP-11347. Bug report: - What happened: same-company CEO authorization for company settings, branding, and portability routes was implemented per-route, making it possible for adjacent surfaces to drift and risking company-boundary mistakes. - Expected behavior: an agent API key must only manage the company that owns the authenticated agent, and only CEO agents should get the limited same-company management permissions. - Steps to reproduce: authenticate as a CEO agent from Company A and call Company B company routes such as `PATCH /api/companies/:companyId`, `PATCH /api/companies/:companyId/branding`, export/preview export, safe import preview/apply, archive, delete, or read. - Version/commit: fixed on this branch at `45edaccb8` on top of `public/master` `ddc193c2b`. - Deployment mode: server API behavior; no UI change. Related search results reviewed, not duplicates of this exact route hardening: - Refs #2212 - Refs #1083 - Refs #8053 ## What Changed - Added a shared `assertSameCompanyCeoAgentOrBoard` company route guard and reused it for company settings, branding, export, and safe import endpoints. - Moved request parsing after authorization on sensitive company mutation/export/import routes so unauthorized cross-company callers are rejected before route body validation side effects or service calls. - Tightened archive and delete ordering to assert route company access before board-only mutation authorization. - Added a cross-company company-route authorization regression suite covering read, settings, branding, archive, delete, export, export preview, import preview, and import apply paths. - Extended adjacent route/service tests to prove non-CEO and cross-company agent keys are rejected on the relevant company-scoped surfaces. - Addressed Greptile follow-ups by removing a redundant board assertion and inlining the portability authorization wrapper. ## Verification Local focused verification on rebased head `45edaccb8`: - `pnpm exec vitest run server/src/__tests__/companies-route-cross-company-authz.test.ts server/src/__tests__/company-branding-route.test.ts server/src/__tests__/company-portability-routes.test.ts server/src/__tests__/agent-permissions-routes.test.ts server/src/__tests__/authorization-service.test.ts server/src/__tests__/openclaw-invite-prompt-route.test.ts` - 6 test files passed - 127 tests passed Remote PR checks on rebased head `45edaccb8`: - Paperclip PR workflow: green, including policy, typecheck, build, e2e, canary dry run, workspace tests, server general tests, and all serialized server shards. - Greptile Review: success with 5/5 confidence on `45edaccb8`. - Greptile review threads: all resolved. ## Risks Low to moderate risk. This intentionally tightens company route authorization and changes whether some unauthorized requests fail at the authz layer before schema validation. Legitimate board users and same-company CEO agents remain covered by tests, but callers that depended on validation errors from unauthorized company routes will now receive authorization errors first. No migrations. No `pnpm-lock.yaml` changes. No `.github/workflows` changes. No screenshots or design images added. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5.5-based coding agent with tool use, terminal execution, repository inspection, and GitHub connector access. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (N/A: no UI change) - [x] I have updated relevant documentation to reflect my changes (N/A: no docs needed beyond this PR description) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../agent-permissions-routes.test.ts | 19 + .../__tests__/authorization-service.test.ts | 19 + ...ompanies-route-cross-company-authz.test.ts | 353 ++++++++++++++++++ .../__tests__/company-branding-route.test.ts | 145 +++++++ .../company-portability-routes.test.ts | 131 +++++++ .../openclaw-invite-prompt-route.test.ts | 22 ++ server/src/routes/companies.ts | 114 +++--- 7 files changed, 740 insertions(+), 63 deletions(-) create mode 100644 server/src/__tests__/companies-route-cross-company-authz.test.ts diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 73ecd39e3b..b839913892 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -1594,6 +1594,25 @@ describe.sequential("agent permission routes", () => { expect(res.body.access.taskAssignSource).toBe("agent_creator"); }); + it("rejects CEO permission updates outside the caller company scope", async () => { + const app = await createApp({ + type: "agent", + agentId: "ceo-agent", + companyId: "33333333-3333-4333-8333-333333333333", + runId: "run-1", + source: "agent_key", + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl) + .patch(`/api/agents/${agentId}/permissions`) + .send({ canCreateAgents: true, canAssignTasks: true })); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("another company"); + expect(mockAgentService.updatePermissions).not.toHaveBeenCalled(); + expect(mockAccessService.setPrincipalPermission).not.toHaveBeenCalled(); + }); + it("exposes a dedicated agent route for the inbox mine view", async () => { mockIssueService.list.mockResolvedValue([ { diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index 09b98af27b..2c1dd29427 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -1021,6 +1021,25 @@ describeEmbeddedPostgres("authorization service", () => { }); }); + it("denies active-checkout management outside the CEO caller company scope", async () => { + const sourceCompany = await createCompany(db, "CheckoutSource"); + const targetCompany = await createCompany(db, "CheckoutTarget"); + const actorAgent = await createAgent(db, sourceCompany.id, { role: "ceo" }); + const targetAgent = await createAgent(db, targetCompany.id, { role: "engineer" }); + + const decision = await authorizationService(db).decide({ + actor: { type: "agent", agentId: actorAgent.id, companyId: sourceCompany.id, source: "agent_jwt" }, + action: "tasks:manage_active_checkouts", + resource: { type: "issue", companyId: targetCompany.id, assigneeAgentId: targetAgent.id }, + }); + + expect(decision).toMatchObject({ + allowed: false, + reason: "deny_company_boundary", + }); + expect(decision.explanation).toContain("another company"); + }); + it("allows scoped assignment inside a granted project and denies other projects", async () => { const company = await createCompany(db, "ProjectScope"); const project = await createProject(db, company.id, "Allowed"); diff --git a/server/src/__tests__/companies-route-cross-company-authz.test.ts b/server/src/__tests__/companies-route-cross-company-authz.test.ts new file mode 100644 index 0000000000..d6aee5b30c --- /dev/null +++ b/server/src/__tests__/companies-route-cross-company-authz.test.ts @@ -0,0 +1,353 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const companyAId = "11111111-1111-4111-8111-111111111111"; +const companyBId = "22222222-2222-4222-8222-222222222222"; +const ceoAgentId = "ceo-agent-a"; + +const mockCompanyService = vi.hoisted(() => ({ + list: vi.fn(), + stats: vi.fn(), + getById: vi.fn(), + create: vi.fn(), + update: vi.fn(), + archive: vi.fn(), + remove: vi.fn(), +})); + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), +})); + +const mockAccessService = vi.hoisted(() => ({ + ensureMembership: vi.fn(), + ensureRoleDefaultGrants: vi.fn(), +})); + +const mockBudgetService = vi.hoisted(() => ({ + upsertPolicy: vi.fn(), +})); + +const mockCompanyPortabilityService = vi.hoisted(() => ({ + exportBundle: vi.fn(), + previewExport: vi.fn(), + previewImport: vi.fn(), + importBundle: vi.fn(), +})); + +const mockCompanyArtifactsService = vi.hoisted(() => ({ + list: vi.fn(), +})); + +const mockFeedbackService = vi.hoisted(() => ({ + listFeedbackTraces: vi.fn(), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn()); + +function registerCompanyRouteMocks() { + vi.doMock("../services/index.js", () => ({ + accessService: () => mockAccessService, + agentService: () => mockAgentService, + budgetService: () => mockBudgetService, + companyArtifactsService: () => mockCompanyArtifactsService, + companyPortabilityService: () => mockCompanyPortabilityService, + companyService: () => mockCompanyService, + feedbackService: () => mockFeedbackService, + logActivity: mockLogActivity, + })); +} + +let appImportCounter = 0; + +async function createApp(actor: Record) { + registerCompanyRouteMocks(); + appImportCounter += 1; + const routeModulePath = `../routes/companies.js?cross-company-authz-${appImportCounter}`; + const middlewareModulePath = `../middleware/index.js?cross-company-authz-${appImportCounter}`; + const [{ companyRoutes }, { errorHandler }] = await Promise.all([ + import(routeModulePath) as Promise, + import(middlewareModulePath) as Promise, + ]); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = actor; + next(); + }); + app.use("/api/companies", companyRoutes({} as any)); + app.use(errorHandler); + return app; +} + +function createCompany(id: string) { + const now = new Date("2026-06-18T00:00:00.000Z"); + return { + id, + name: id === companyAId ? "Company A" : "Company B", + description: null, + status: "active", + issuePrefix: id === companyAId ? "CPA" : "CPB", + issueCounter: 1, + budgetMonthlyCents: 0, + spentMonthlyCents: 0, + requireBoardApprovalForNewAgents: false, + feedbackDataSharingEnabled: false, + brandColor: "#123456", + logoAssetId: null, + logoUrl: null, + attachmentMaxBytes: 25_000_000, + createdAt: now, + updatedAt: now, + }; +} + +const exportRequest = { + include: { company: true, agents: true, projects: true }, +}; + +function exportResult() { + return { + rootPath: "paperclip", + manifest: { + agents: [], + skills: [], + projects: [], + issues: [], + envInputs: [], + includes: { company: true, agents: true, projects: true, issues: false, skills: false }, + company: null, + schemaVersion: 1, + generatedAt: "2026-06-18T00:00:00.000Z", + source: null, + }, + files: {}, + warnings: [], + }; +} + +function exportPreviewResult() { + return { + ...exportResult(), + fileInventory: [], + counts: { files: 0, agents: 0, skills: 0, projects: 0, issues: 0 }, + paperclipExtensionPath: ".paperclip.yaml", + }; +} + +function importRequest(targetCompanyId = companyBId) { + return { + source: { type: "inline", files: { "COMPANY.md": "---\nname: Imported\n---\n" } }, + include: { company: true, agents: true, projects: false, issues: false }, + target: { mode: "existing_company", companyId: targetCompanyId }, + collisionStrategy: "rename", + }; +} + +function importResult(companyId = companyBId) { + return { + company: { id: companyId, action: "updated" }, + agents: [], + warnings: [], + }; +} + +function resetMockDefaults() { + mockCompanyService.getById.mockImplementation(async (id: string) => { + if (id === companyAId || id === companyBId) return createCompany(id); + return null; + }); + mockCompanyService.update.mockImplementation(async (id: string, body: Record) => ({ + ...createCompany(id), + ...body, + })); + mockCompanyService.archive.mockImplementation(async (id: string) => ({ + ...createCompany(id), + status: "archived", + })); + mockCompanyService.remove.mockImplementation(async (id: string) => createCompany(id)); + mockAgentService.getById.mockImplementation(async (id: string) => { + if (id === ceoAgentId) return { id, companyId: companyAId, role: "ceo" }; + return null; + }); + mockCompanyPortabilityService.exportBundle.mockResolvedValue(exportResult()); + mockCompanyPortabilityService.previewExport.mockResolvedValue(exportPreviewResult()); + mockCompanyPortabilityService.previewImport.mockResolvedValue({ ok: true }); + mockCompanyPortabilityService.importBundle.mockResolvedValue(importResult()); +} + +function assertNoTargetMutationSideEffects() { + expect(mockCompanyService.update).not.toHaveBeenCalled(); + expect(mockCompanyService.archive).not.toHaveBeenCalled(); + expect(mockCompanyService.remove).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.exportBundle).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.importBundle).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); +} + +function companyACeoActor() { + return { + type: "agent", + agentId: ceoAgentId, + companyId: companyAId, + source: "agent_key", + runId: "run-1", + }; +} + +function boardActor(input: { + userId: string; + companyIds?: string[]; + memberships?: Array<{ companyId: string; membershipRole: string; status: string }>; + isInstanceAdmin?: boolean; + source?: string; +}) { + return { + type: "board", + userId: input.userId, + source: input.source ?? "session", + companyIds: input.companyIds ?? [], + memberships: input.memberships ?? [], + isInstanceAdmin: input.isInstanceAdmin ?? false, + }; +} + +describe.sequential("company route cross-company authorization", () => { + beforeEach(() => { + vi.resetModules(); + vi.doUnmock("../routes/authz.js"); + vi.doUnmock("../middleware/index.js"); + vi.clearAllMocks(); + resetMockDefaults(); + }); + + it.each([ + { + label: "GET /api/companies/:companyId", + request: (app: express.Express) => request(app).get(`/api/companies/${companyBId}`), + }, + { + label: "PATCH /api/companies/:companyId", + request: (app: express.Express) => request(app).patch(`/api/companies/${companyBId}`).send({ description: "Nope" }), + }, + { + label: "PATCH /api/companies/:companyId/branding", + request: (app: express.Express) => request(app).patch(`/api/companies/${companyBId}/branding`).send({ brandColor: "#654321" }), + }, + { + label: "POST /api/companies/:companyId/archive", + request: (app: express.Express) => request(app).post(`/api/companies/${companyBId}/archive`).send({}), + }, + { + label: "DELETE /api/companies/:companyId", + request: (app: express.Express) => request(app).delete(`/api/companies/${companyBId}`), + }, + { + label: "POST /api/companies/:companyId/export", + request: (app: express.Express) => request(app).post(`/api/companies/${companyBId}/export`).send(exportRequest), + }, + { + label: "POST /api/companies/:companyId/exports/preview", + request: (app: express.Express) => request(app).post(`/api/companies/${companyBId}/exports/preview`).send(exportRequest), + }, + { + label: "POST /api/companies/:companyId/imports/preview", + request: (app: express.Express) => request(app).post(`/api/companies/${companyBId}/imports/preview`).send(importRequest()), + }, + { + label: "POST /api/companies/:companyId/imports/apply", + request: (app: express.Express) => request(app).post(`/api/companies/${companyBId}/imports/apply`).send(importRequest()), + }, + ])("rejects a company A CEO attempting company B operation: $label", async ({ request: buildRequest }) => { + const app = await createApp(companyACeoActor()); + + const res = await buildRequest(app); + + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/another company|access to this company|active company access/i); + assertNoTargetMutationSideEffects(); + }); + + it("allows a same-company CEO to use CEO-safe company routes without allowing board-only lifecycle routes", async () => { + const app = await createApp(companyACeoActor()); + + await request(app).get(`/api/companies/${companyAId}`).expect(200); + await request(app).patch(`/api/companies/${companyAId}`).send({ brandColor: "#abcdef" }).expect(200); + await request(app).patch(`/api/companies/${companyAId}/branding`).send({ brandColor: "#abcdef" }).expect(200); + await request(app).post(`/api/companies/${companyAId}/export`).send(exportRequest).expect(200); + await request(app).post(`/api/companies/${companyAId}/exports/preview`).send(exportRequest).expect(200); + await request(app).post(`/api/companies/${companyAId}/imports/preview`).send(importRequest(companyAId)).expect(200); + await request(app).post(`/api/companies/${companyAId}/imports/apply`).send(importRequest(companyAId)).expect(200); + + const archive = await request(app).post(`/api/companies/${companyAId}/archive`).send({}); + expect(archive.status).toBe(403); + expect(archive.body.error).toContain("Board access required"); + const remove = await request(app).delete(`/api/companies/${companyAId}`); + expect(remove.status).toBe(403); + expect(remove.body.error).toContain("Board access required"); + }); + + it("covers board actor access for non-member, viewer, active member, local trusted board, and instance admin without target membership", async () => { + const nonMemberApp = await createApp(boardActor({ userId: "outsider" })); + const nonMember = await request(nonMemberApp).get(`/api/companies/${companyBId}`); + expect(nonMember.status).toBe(403); + expect(nonMember.body.error).toContain("access to this company"); + + vi.clearAllMocks(); + resetMockDefaults(); + const viewerApp = await createApp(boardActor({ + userId: "viewer", + companyIds: [companyBId], + memberships: [{ companyId: companyBId, membershipRole: "viewer", status: "active" }], + })); + await request(viewerApp).get(`/api/companies/${companyBId}`).expect(200); + const viewerWrite = await request(viewerApp).patch(`/api/companies/${companyBId}`).send({ description: "Nope" }); + expect(viewerWrite.status).toBe(403); + expect(viewerWrite.body.error).toContain("Viewer access is read-only"); + expect(mockCompanyService.update).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + + vi.clearAllMocks(); + resetMockDefaults(); + const memberApp = await createApp(boardActor({ + userId: "member", + companyIds: [companyBId], + memberships: [{ companyId: companyBId, membershipRole: "member", status: "active" }], + })); + await request(memberApp).patch(`/api/companies/${companyBId}`).send({ description: "Updated" }).expect(200); + await request(memberApp).patch(`/api/companies/${companyBId}/branding`).send({ brandColor: "#abcdef" }).expect(200); + await request(memberApp).post(`/api/companies/${companyBId}/archive`).send({}).expect(200); + await request(memberApp).delete(`/api/companies/${companyBId}`).expect(200); + await request(memberApp).post(`/api/companies/${companyBId}/export`).send(exportRequest).expect(200); + await request(memberApp).post(`/api/companies/${companyBId}/exports/preview`).send(exportRequest).expect(200); + await request(memberApp).post(`/api/companies/${companyBId}/imports/preview`).send(importRequest()).expect(200); + await request(memberApp).post(`/api/companies/${companyBId}/imports/apply`).send(importRequest()).expect(200); + + vi.clearAllMocks(); + resetMockDefaults(); + const localTrustedApp = await createApp(boardActor({ + userId: "local-board", + source: "local_implicit", + isInstanceAdmin: true, + })); + await request(localTrustedApp).get(`/api/companies/${companyBId}`).expect(200); + await request(localTrustedApp).patch(`/api/companies/${companyBId}`).send({ description: "Local" }).expect(200); + + vi.clearAllMocks(); + resetMockDefaults(); + const adminWithoutMembershipApp = await createApp(boardActor({ + userId: "instance-admin", + isInstanceAdmin: true, + })); + const adminRead = await request(adminWithoutMembershipApp).get(`/api/companies/${companyBId}`); + expect(adminRead.status).toBe(403); + expect(adminRead.body.error).toContain("access to this company"); + const adminWrite = await request(adminWithoutMembershipApp).patch(`/api/companies/${companyBId}`).send({ description: "Admin" }); + expect(adminWrite.status).toBe(403); + expect(adminWrite.body.error).toContain("access to this company"); + assertNoTargetMutationSideEffects(); + }); +}); diff --git a/server/src/__tests__/company-branding-route.test.ts b/server/src/__tests__/company-branding-route.test.ts index 6ad52168a2..40a9a692ee 100644 --- a/server/src/__tests__/company-branding-route.test.ts +++ b/server/src/__tests__/company-branding-route.test.ts @@ -122,6 +122,29 @@ describe("PATCH /api/companies/:companyId/branding", () => { expect(mockCompanyService.update).not.toHaveBeenCalled(); }); + it("rejects non-CEO agent callers before validating branding body shape", async () => { + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + role: "engineer", + }); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/companies/company-1/branding") + .send({ status: "archived" }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Only CEO agents"); + expect(mockCompanyService.update).not.toHaveBeenCalled(); + }); + it("allows CEO agent callers to update branding fields", async () => { const company = createCompany(); mockAgentService.getById.mockResolvedValue({ @@ -210,3 +233,125 @@ describe("PATCH /api/companies/:companyId/branding", () => { expect(mockCompanyService.update).not.toHaveBeenCalled(); }); }); + +describe("PATCH /api/companies/:companyId", () => { + beforeEach(() => { + vi.resetModules(); + vi.doUnmock("../routes/companies.js"); + vi.doUnmock("../routes/authz.js"); + vi.doUnmock("../middleware/index.js"); + vi.clearAllMocks(); + }); + + it("rejects non-CEO agent callers before loading the company or validating settings body shape", async () => { + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + role: "engineer", + }); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/companies/company-1") + .send({ status: "archived" }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Only CEO agents"); + expect(mockCompanyService.getById).not.toHaveBeenCalled(); + expect(mockCompanyService.update).not.toHaveBeenCalled(); + }); + + it("allows CEO agent callers to update only branding fields through the general settings route", async () => { + const company = createCompany(); + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + role: "ceo", + }); + mockCompanyService.getById.mockResolvedValue(company); + mockCompanyService.update.mockResolvedValue({ + ...company, + name: "New Name", + }); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/companies/company-1") + .send({ name: "New Name" }); + + expect(res.status).toBe(200); + expect(mockCompanyService.update).toHaveBeenCalledWith("company-1", { name: "New Name" }, expect.objectContaining({ + actorType: "agent", + actorId: "agent-1", + })); + }); + + it("rejects CEO agent attempts to update lifecycle, budget, consent, or prefix fields", async () => { + const company = createCompany(); + mockAgentService.getById.mockResolvedValue({ + id: "agent-1", + companyId: "company-1", + role: "ceo", + }); + mockCompanyService.getById.mockResolvedValue(company); + const app = await createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .patch("/api/companies/company-1") + .send({ + status: "archived", + budgetMonthlyCents: 1000, + spentMonthlyCents: 500, + requireBoardApprovalForNewAgents: true, + feedbackDataSharingEnabled: true, + issuePrefix: "BAD", + }); + + expect(res.status).toBe(400); + expect(res.body.error).toBe("Validation error"); + expect(mockCompanyService.update).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("keeps full company settings updates board-only", async () => { + const company = createCompany(); + mockCompanyService.getById.mockResolvedValue(company); + mockCompanyService.update.mockResolvedValue({ + ...company, + status: "paused", + }); + const app = await createApp({ + type: "board", + userId: "user-1", + source: "local_implicit", + }); + + const res = await request(app) + .patch("/api/companies/company-1") + .send({ status: "paused" }); + + expect(res.status).toBe(200); + expect(mockCompanyService.update).toHaveBeenCalledWith("company-1", { status: "paused" }, expect.objectContaining({ + actorType: "user", + actorId: "user-1", + })); + }); +}); diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index a3701b76ed..4198c10091 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -118,6 +118,7 @@ async function createApp(actor: Record) { } const companyId = "11111111-1111-4111-8111-111111111111"; +const otherCompanyId = "22222222-2222-4222-8222-222222222222"; const ceoAgentId = "ceo-agent"; const engineerAgentId = "engineer-agent"; @@ -241,6 +242,24 @@ describe.sequential("company portability routes", () => { expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); }); + it.sequential("rejects non-CEO export preview callers before validating request shape", async () => { + const app = await createApp({ + type: "agent", + agentId: engineerAgentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/exports/preview`) + .send({ agents: [123] }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Only CEO agents"); + expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); + }); + it.sequential("rejects non-CEO agents from legacy and CEO-safe export bundle routes", async () => { const app = await createApp({ type: "agent", @@ -325,6 +344,30 @@ describe.sequential("company portability routes", () => { expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledTimes(2); }); + it.sequential("rejects CEO agents from exporting another company before services run", async () => { + const app = await createApp({ + type: "agent", + agentId: ceoAgentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + for (const path of [ + `/api/companies/${otherCompanyId}/export`, + `/api/companies/${otherCompanyId}/exports`, + `/api/companies/${otherCompanyId}/exports/preview`, + ]) { + const res = await request(app).post(path).send(exportRequest); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("another company"); + } + expect(mockCompanyPortabilityService.exportBundle).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it.sequential("rejects replace collision strategy on CEO-safe import routes", async () => { const app = await createApp({ type: "agent", @@ -348,6 +391,58 @@ describe.sequential("company portability routes", () => { expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); }); + it.sequential("rejects CEO agents from previewing or applying imports against another route company", async () => { + const app = await createApp({ + type: "agent", + agentId: ceoAgentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + for (const path of [ + `/api/companies/${otherCompanyId}/imports/preview`, + `/api/companies/${otherCompanyId}/imports/apply`, + ]) { + const res = await request(app).post(path).send({ + ...importRequest, + target: { mode: "existing_company", companyId: otherCompanyId }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("another company"); + } + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.importBundle).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it.sequential("rejects CEO-safe import bodies that target a different company than the route", async () => { + const app = await createApp({ + type: "agent", + agentId: ceoAgentId, + companyId, + source: "agent_key", + runId: "run-1", + }); + + for (const path of [ + `/api/companies/${companyId}/imports/preview`, + `/api/companies/${companyId}/imports/apply`, + ]) { + const res = await request(app).post(path).send({ + ...importRequest, + target: { mode: "existing_company", companyId: otherCompanyId }, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("only target the route company"); + } + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.importBundle).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it.sequential("keeps global import preview routes board-only", async () => { const app = await createApp({ type: "agent", @@ -370,6 +465,24 @@ describe.sequential("company portability routes", () => { expect(res.body.error).toContain("Board access required"); }); + it.sequential("keeps global import preview board-only before validating request shape", async () => { + const app = await createApp({ + type: "agent", + agentId: engineerAgentId, + companyId: "11111111-1111-4111-8111-111111111111", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post("/api/companies/import/preview") + .send({ target: { mode: "existing_company", companyId: "not-a-uuid" } }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Board access required"); + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + }); + it.sequential("requires instance admin for new-company import preview", async () => { const app = await createApp({ type: "board", @@ -439,6 +552,24 @@ describe.sequential("company portability routes", () => { expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); }); + it.sequential("rejects non-CEO import preview callers before validating request shape", async () => { + const app = await createApp({ + type: "agent", + agentId: engineerAgentId, + companyId: "11111111-1111-4111-8111-111111111111", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app) + .post("/api/companies/11111111-1111-4111-8111-111111111111/imports/preview") + .send({ target: { mode: "existing_company", companyId: "not-a-uuid" } }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Only CEO agents"); + expect(mockCompanyPortabilityService.previewImport).not.toHaveBeenCalled(); + }); + it.sequential("rejects non-CEO agents from CEO-safe import apply routes", async () => { const app = await createApp({ type: "agent", diff --git a/server/src/__tests__/openclaw-invite-prompt-route.test.ts b/server/src/__tests__/openclaw-invite-prompt-route.test.ts index fd556ea176..7904e275a3 100644 --- a/server/src/__tests__/openclaw-invite-prompt-route.test.ts +++ b/server/src/__tests__/openclaw-invite-prompt-route.test.ts @@ -173,6 +173,28 @@ describe.sequential("POST /companies/:companyId/openclaw/invite-prompt", () => { expect(res.body.error).toContain("Only CEO agents"); }); + it("rejects CEO agent callers outside the target company scope", async () => { + const db = createDbStub(); + const app = createApp( + { + type: "agent", + agentId: "agent-1", + companyId: "company-2", + source: "agent_key", + }, + db, + ); + + const res = await request(app) + .post("/api/companies/company-1/openclaw/invite-prompt") + .send({}); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("another company"); + expect(mockAgentService.getById).not.toHaveBeenCalled(); + expect((db as any).__insertValues).not.toHaveBeenCalled(); + }); + it("allows CEO agent callers and creates an agent-only invite", async () => { const db = createDbStub([companyBranding], [logoAsset]); mockAgentService.getById.mockResolvedValue({ diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index fcc13423fa..038ddaa885 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -68,9 +68,11 @@ export function companyRoutes(db: Db, storage?: StorageService) { assertCompanyAccess(req, target.companyId); } - async function assertCanUpdateBranding(req: Request, companyId: string) { + async function assertSameCompanyCeoAgentOrBoard(req: Request, companyId: string, capability: string) { assertCompanyAccess(req, companyId); - if (req.actor.type === "board") return; + if (req.actor.type === "board") { + return; + } if (!req.actor.agentId) throw forbidden("Agent authentication required"); const actorAgent = await agents.getById(req.actor.agentId); @@ -78,21 +80,7 @@ export function companyRoutes(db: Db, storage?: StorageService) { throw forbidden("Agent key cannot access another company"); } if (actorAgent.role !== "ceo") { - throw forbidden("Only CEO agents can update company branding"); - } - } - - async function assertCanManagePortability(req: Request, companyId: string, capability: "imports" | "exports") { - assertCompanyAccess(req, companyId); - if (req.actor.type === "board") return; - if (!req.actor.agentId) throw forbidden("Agent authentication required"); - - const actorAgent = await agents.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - if (actorAgent.role !== "ceo") { - throw forbidden(`Only CEO agents can manage company ${capability}`); + throw forbidden(`Only CEO agents can manage ${capability}`); } } @@ -178,17 +166,19 @@ export function companyRoutes(db: Db, storage?: StorageService) { res.json(traces); }); - router.post("/:companyId/export", validate(companyPortabilityExportSchema), async (req, res) => { + router.post("/:companyId/export", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanManagePortability(req, companyId, "exports"); - const result = await portability.exportBundle(companyId, req.body); + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); + const body = companyPortabilityExportSchema.parse(req.body); + const result = await portability.exportBundle(companyId, body); res.json(result); }); - router.post("/import/preview", validate(companyPortabilityPreviewSchema), async (req, res) => { + router.post("/import/preview", async (req, res) => { assertBoard(req); - assertImportTargetAccess(req, req.body.target); - const preview = await portability.previewImport(req.body); + const body = companyPortabilityPreviewSchema.parse(req.body); + assertImportTargetAccess(req, body.target); + const preview = await portability.previewImport(body); res.json(preview); }); @@ -236,47 +226,51 @@ export function companyRoutes(db: Db, storage?: StorageService) { res.json(result); }); - router.post("/:companyId/exports/preview", validate(companyPortabilityExportSchema), async (req, res) => { + router.post("/:companyId/exports/preview", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanManagePortability(req, companyId, "exports"); - const preview = await portability.previewExport(companyId, req.body); + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); + const body = companyPortabilityExportSchema.parse(req.body); + const preview = await portability.previewExport(companyId, body); res.json(preview); }); - router.post("/:companyId/exports", validate(companyPortabilityExportSchema), async (req, res) => { + router.post("/:companyId/exports", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanManagePortability(req, companyId, "exports"); - const result = await portability.exportBundle(companyId, req.body); + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); + const body = companyPortabilityExportSchema.parse(req.body); + const result = await portability.exportBundle(companyId, body); res.json(result); }); - router.post("/:companyId/imports/preview", validate(companyPortabilityPreviewSchema), async (req, res) => { + router.post("/:companyId/imports/preview", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanManagePortability(req, companyId, "imports"); - if (req.body.target.mode === "existing_company" && req.body.target.companyId !== companyId) { + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company imports"); + const body = companyPortabilityPreviewSchema.parse(req.body); + if (body.target.mode === "existing_company" && body.target.companyId !== companyId) { throw forbidden("Safe import route can only target the route company"); } - if (req.body.collisionStrategy === "replace") { + if (body.collisionStrategy === "replace") { throw forbidden("Safe import route does not allow replace collision strategy"); } - const preview = await portability.previewImport(req.body, { + const preview = await portability.previewImport(body, { mode: "agent_safe", sourceCompanyId: companyId, }); res.json(preview); }); - router.post("/:companyId/imports/apply", validate(companyPortabilityImportSchema), async (req, res) => { + router.post("/:companyId/imports/apply", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanManagePortability(req, companyId, "imports"); - if (req.body.target.mode === "existing_company" && req.body.target.companyId !== companyId) { + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company imports"); + const body = companyPortabilityImportSchema.parse(req.body); + if (body.target.mode === "existing_company" && body.target.companyId !== companyId) { throw forbidden("Safe import route can only target the route company"); } - if (req.body.collisionStrategy === "replace") { + if (body.collisionStrategy === "replace") { throw forbidden("Safe import route does not allow replace collision strategy"); } const actor = getActorInfo(req); - const result = await portability.importBundle(req.body, req.actor.type === "board" ? req.actor.userId : null, { + const result = await portability.importBundle(body, req.actor.type === "board" ? req.actor.userId : null, { mode: "agent_safe", sourceCompanyId: companyId, }); @@ -290,7 +284,7 @@ export function companyRoutes(db: Db, storage?: StorageService) { runId: actor.runId, action: "company.imported", details: { - include: req.body.include ?? null, + include: body.include ?? null, agentCount: result.agents.length, warningCount: result.warnings.length, companyAction: result.company.action, @@ -340,31 +334,24 @@ export function companyRoutes(db: Db, storage?: StorageService) { router.patch("/:companyId", async (req, res) => { const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company settings"); const actor = getActorInfo(req); + let body: Record; + + if (req.actor.type === "agent") { + body = updateCompanyBrandingSchema.parse(req.body); + } else { + body = updateCompanySchema.parse(req.body); + } + const existingCompany = await svc.getById(companyId); if (!existingCompany) { res.status(404).json({ error: "Company not found" }); return; } - let body: Record; - - if (req.actor.type === "agent") { - // Only CEO agents may update company branding fields - const agentSvc = agentService(db); - const actorAgent = req.actor.agentId ? await agentSvc.getById(req.actor.agentId) : null; - if (!actorAgent || actorAgent.role !== "ceo") { - throw forbidden("Only CEO agents or board users may update company settings"); - } - if (actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - body = updateCompanyBrandingSchema.parse(req.body); - } else { - assertBoard(req); - body = updateCompanySchema.parse(req.body); + if (req.actor.type !== "agent") { if (body.feedbackDataSharingEnabled === true && !existingCompany.feedbackDataSharingEnabled) { body = { ...body, @@ -421,10 +408,11 @@ export function companyRoutes(db: Db, storage?: StorageService) { res.json(company); }); - router.patch("/:companyId/branding", validate(updateCompanyBrandingSchema), async (req, res) => { + router.patch("/:companyId/branding", async (req, res) => { const companyId = req.params.companyId as string; - await assertCanUpdateBranding(req, companyId); - const company = await svc.update(companyId, req.body); + await assertSameCompanyCeoAgentOrBoard(req, companyId, "company branding"); + const body = updateCompanyBrandingSchema.parse(req.body); + const company = await svc.update(companyId, body); if (!company) { res.status(404).json({ error: "Company not found" }); return; @@ -439,15 +427,15 @@ export function companyRoutes(db: Db, storage?: StorageService) { action: "company.branding_updated", entityType: "company", entityId: companyId, - details: req.body, + details: body, }); res.json(company); }); router.post("/:companyId/archive", async (req, res) => { - assertBoard(req); const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + assertBoard(req); const company = await svc.archive(companyId, getActorInfo(req)); if (!company) { res.status(404).json({ error: "Company not found" }); @@ -457,9 +445,9 @@ export function companyRoutes(db: Db, storage?: StorageService) { }); router.delete("/:companyId", async (req, res) => { - assertBoard(req); const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + assertBoard(req); const company = await svc.remove(companyId); if (!company) { res.status(404).json({ error: "Company not found" });