mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:24:09 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server controls secrets, host files, outbound requests, and workspace commands > - A red-team review found cases where restricted callers could cross these trust boundaries > - These cases could expose credentials or let untrusted input reach privileged resources > - This pull request applies least-privilege checks at each affected server boundary > - The benefit is safer agent execution without changing the private-instance bootstrap contract ## Linked Issues or Issue Description **What happened?** Several server paths used authorization, redaction, or content-delivery rules that were too broad. Restricted agent keys could obtain company-level operational data. Some adapter and instruction paths could reach server-owned network or file resources without the required owner approval. **Expected behavior** Paperclip must redact credential values, enforce restricted-key scopes, guard outbound network access, prevent same-origin script execution, and reserve host-level file and command controls for authorized operators. **Steps to reproduce** 1. Configure an authenticated development instance at the parent commit. 2. Exercise the affected APIs with a restricted agent key or a non-instance-admin company user. 3. Observe that the parent commit returns privileged data or accepts a privileged operation. 4. Repeat on this branch and observe a redacted response, a safe download, or an HTTP 403 response. **Paperclip version or commit** The findings reproduce from commit `39898ab22` and are fixed by this pull request. **Deployment mode** Authenticated self-hosted server and local development modes. **Installation method** Built from source with pnpm. ## What Changed - Redact generic secret `value` and `token` fields recursively in structured logs. - Classify exact and separator-suffixed `KEY` environment names as secrets in company exports. - Limit restricted self-identity responses and protect company run, log, and secret catalog APIs. - Route HTTP adapter requests through DNS-pinned SSRF protection with exact private-origin allowlisting. - Download HTML, SVG, and other script-capable assets with `nosniff` and a sandbox CSP. - Require instance-admin access for external instruction roots and exports that read them. - Block agent-authenticated host command persistence across supported workspace runtime shapes. - Apply the central runtime-management decision before workspace command controls. - Keep the documented first-user instance-admin claim contract unchanged. - Add regression tests and server-owner configuration documentation. ## Verification - `pnpm -r typecheck` passes. - The Node 24 remediation suite passes with 365 tests. It skips 25 environment-gated tests. - `pnpm build` passes under Node 24. - `git diff --check` passes. - The full local runner reaches known macOS-only general-server harness failures before the serialized route lane. The Linux PR matrix is the authoritative full-suite gate. ## Risks - Restricted agent keys now receive HTTP 403 responses from company-wide run, log, and secret catalog endpoints. - Script-capable assets now download instead of rendering inline. - External instruction roots now require instance-admin access. - Private HTTP adapter endpoints now require an exact origin in `PAPERCLIP_HTTP_ADAPTER_PRIVATE_ENDPOINT_ALLOWLIST`. - Public HTTP adapter endpoints remain enabled. Redirects and metadata or link-local targets remain blocked. - No database migration is required. > 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. The exact serving snapshot and context-window size are not exposed. The model used tool-enabled reasoning, repository access, code execution, and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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 <noreply@paperclip.ing>
358 lines
13 KiB
TypeScript
358 lines
13 KiB
TypeScript
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(),
|
|
list: 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<string, unknown>) {
|
|
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<typeof import("../routes/companies.js")>,
|
|
import(middlewareModulePath) as Promise<typeof 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/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,
|
|
logoAssetId: null,
|
|
logoUrl: null,
|
|
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<string, unknown>) => ({
|
|
...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;
|
|
});
|
|
mockAgentService.list.mockResolvedValue([]);
|
|
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({ description: "Nope" }),
|
|
},
|
|
{
|
|
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: "GET /api/companies/:companyId/export/fidelity",
|
|
request: (app: express.Express) => request(app).get(`/api/companies/${companyBId}/export/fidelity`),
|
|
},
|
|
{
|
|
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({ description: "Branding" }).expect(200);
|
|
await request(app).patch(`/api/companies/${companyAId}/branding`).send({ description: "Branding" }).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({ description: "Branding" }).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();
|
|
});
|
|
});
|