diff --git a/.env.example b/.env.example index d90358bd85..e920780009 100644 --- a/.env.example +++ b/.env.example @@ -18,5 +18,9 @@ PAPERCLIP_TOOL_ACTION_SIGNING_SECRET=paperclip-dev-tool-action-signing-secret-ch # PAPERCLIP_WORKSPACE_GIT_SCAN_TIMEOUT_MS=8000 # PAPERCLIP_WORKSPACE_GIT_SCAN_CACHE_TTL_MS=10000 +# HTTP adapters may call public HTTP(S) origins by default. Opt trusted private +# origins in explicitly; entries are exact origins (scheme, host, and port). +# PAPERCLIP_HTTP_ADAPTER_PRIVATE_ENDPOINT_ALLOWLIST=http://hooks.internal.example:8080 + # Discord webhook for daily merge digest (scripts/discord-daily-digest.sh) # DISCORD_WEBHOOK_URL=https://discord.com/api/webhooks/... diff --git a/doc/DEPLOYMENT-MODES.md b/doc/DEPLOYMENT-MODES.md index 49f6e5f0d5..04a9897973 100644 --- a/doc/DEPLOYMENT-MODES.md +++ b/doc/DEPLOYMENT-MODES.md @@ -163,6 +163,12 @@ only to real browser session actors in `authenticated/private`; unauthenticated requests, agent keys, board API keys, and local implicit board actors are rejected. +This is intentionally a first-claim bootstrap contract: before an instance +admin exists, the first authenticated browser session that completes the claim +wins. Operators must keep a `bootstrap_pending` private deployment on a trusted +network and complete setup before admitting untrusted users. This behavior is +not an account-recovery or public-deployment mechanism. + The CLI fallback remains supported in all authenticated setup states: ```sh diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 3baff160e0..f6b3937f9c 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -996,6 +996,24 @@ broker hostname is resolved once and the request is pinned to the approved address; IPv4 and IPv6 link-local destinations remain denied even when their host is allowlisted. +## HTTP Adapter Private Endpoints + +HTTP adapters can call public HTTP(S) endpoints by default. Requests use the +same DNS-pinning guard as remote connections, do not follow redirects, and +reject loopback, RFC1918/private, and link-local or cloud-metadata destinations. + +Server owners can opt a trusted private service in with a comma-separated list +of exact origins: + +```sh +PAPERCLIP_HTTP_ADAPTER_PRIVATE_ENDPOINT_ALLOWLIST=http://hooks.internal.example:8080,https://10.0.0.42 +``` + +Each entry must contain only a scheme, hostname, and optional port. Paths, +credentials, query strings, fragments, and wildcards are ignored. Matching is +by exact normalized origin, so allowing one port does not allow another. +Link-local destinations remain denied even when explicitly listed. + ## Company Deletion Toggle Company deletion is intended as a dev/debug capability and can be disabled at runtime: diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 3387102b93..542dad3b99 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -414,6 +414,7 @@ Operational policy: - Default upload allowlist includes common images, PDF, plain text/markdown/JSON/CSV/HTML, ZIP, and video artifacts (`video/mp4`, `video/webm`, `video/quicktime`). - Attachment reads are company-scoped and expose stable path metadata: `contentPath`/`openPath` for inline-safe viewing and `downloadPath` for forced download. - Inline-safe responses use `Content-Disposition: inline`; unsafe types and explicit download requests use `attachment`. + - Script-capable content such as HTML is always served as an attachment with `X-Content-Type-Options: nosniff` and a sandboxed, deny-by-default CSP; it is never rendered inline on the Paperclip origin. - Video attachments are inline-safe and support single `Range: bytes=start-end` requests with `206`, `Content-Range`, and `Accept-Ranges: bytes` for browser playback/seeking. - Attachment-backed artifact work products use `type: "artifact"`, `provider: "paperclip"`, and metadata with `attachmentId`, `contentType`, `byteSize`, `contentPath`, `openPath`, `downloadPath`, and optional `originalFilename`. - Workspace-only file references use work product `metadata.resourceRef` with `kind: "workspace_file"`, `issueId`, `workspaceKind` (`execution_workspace` or `project_workspace`), `workspaceId`, `relativePath`, optional `line`/`column`, and `displayPath`. These references point at files in a workspace; they do not replace attachment-backed artifacts for deliverables that must be inspectable without workspace access. @@ -1353,6 +1354,10 @@ Required UX behaviors: - CSRF protection for board session endpoints - rate limit auth and key-management endpoints - strict company boundary checks on every entity fetch/mutation +- restricted `skill_test` and `task_bridge` keys cannot enumerate company-wide run telemetry, workspace-operation logs, or the company secret catalog +- HTTP adapters use DNS-pinned outbound requests, reject redirects and link-local/metadata targets, and require an exact server-owner origin allowlist for private destinations +- external instruction bundle roots and exports that read them require instance-admin access; managed company-scoped bundles remain available through normal company authorization +- agent-authenticated callers cannot persist host-executed workspace commands, and restricted keys cannot invoke preconfigured workspace runtime controls ## 17. Testing Strategy diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index 5fca425834..0a110b7802 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -3,6 +3,7 @@ import request from "supertest"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mockAgentService = vi.hoisted(() => ({ + create: vi.fn(), getById: vi.fn(), update: vi.fn(), resolveByReference: vi.fn(), @@ -69,6 +70,7 @@ vi.mock("../services/environments.js", () => ({ vi.mock("../adapters/index.js", () => ({ findServerAdapter: mockFindServerAdapter, + findActiveServerAdapter: mockFindServerAdapter, listAdapterModels: vi.fn(), })); @@ -100,6 +102,7 @@ function registerModuleMocks() { vi.doMock("../adapters/index.js", () => ({ findServerAdapter: mockFindServerAdapter, + findActiveServerAdapter: mockFindServerAdapter, listAdapterModels: vi.fn(), })); } @@ -114,7 +117,7 @@ function boardActor() { }; } -async function createApp(actor: Record = boardActor()) { +async function createApp(actor: Record = boardActor(), db: Record = {}) { const [{ agentRoutes }, { errorHandler }] = await Promise.all([ vi.importActual("../routes/agents.js"), vi.importActual("../middleware/index.js"), @@ -125,7 +128,7 @@ async function createApp(actor: Record = boardActor()) { (req as any).actor = actor; next(); }); - app.use("/api", agentRoutes({} as any)); + app.use("/api", agentRoutes(db as any)); app.use(errorHandler); return app; } @@ -285,6 +288,202 @@ describe("agent instructions bundle routes", () => { expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled(); }); + it("requires instance-admin access for every external instruction entry point", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent(), + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + instructionsEntryFile: "AGENTS.md", + }, + }); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + const requests = [ + () => request(app).get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle"), + () => request(app) + .patch("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle") + .send({ entryFile: "AGENTS.md" }), + () => request(app) + .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file") + .query({ path: "AGENTS.md" }), + () => request(app) + .put("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file") + .send({ path: "AGENTS.md", content: "# changed" }), + () => request(app) + .delete("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle/file") + .query({ path: "AGENTS.md" }), + ]; + + for (const perform of requests) { + const res = await perform(); + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toContain("Instance admin"); + } + expect(mockAgentInstructionsService.getBundle).not.toHaveBeenCalled(); + expect(mockAgentInstructionsService.readFile).not.toHaveBeenCalled(); + expect(mockAgentInstructionsService.updateBundle).not.toHaveBeenCalled(); + expect(mockAgentInstructionsService.writeFile).not.toHaveBeenCalled(); + expect(mockAgentInstructionsService.deleteFile).not.toHaveBeenCalled(); + }); + + it("treats a host root mislabeled as managed as external", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent(), + adapterConfig: { + instructionsBundleMode: "managed", + instructionsRootPath: "/private/host/instructions", + instructionsEntryFile: "AGENTS.md", + }, + }); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle"); + + expect(res.status).toBe(403); + expect(mockAgentInstructionsService.getBundle).not.toHaveBeenCalled(); + }); + + it("allows an instance admin to read an external instruction bundle", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent(), + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + instructionsEntryFile: "AGENTS.md", + }, + }); + + const res = await requestApp( + await createApp({ + type: "board", + userId: "instance-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: true, + }), + (baseUrl) => request(baseUrl) + .get("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle"), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled(); + }); + + it("rejects a company admin that requests a new external instruction root", async () => { + mockSyncInstructionsBundleConfigFromFilePath.mockImplementation((_agent, config) => ({ + ...config, + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + instructionsEntryFile: "AGENTS.md", + })); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + + const compatibilityRes = await request(app) + .patch("/api/agents/11111111-1111-4111-8111-111111111111/instructions-path") + .send({ path: "/srv/paperclip/external-agent/AGENTS.md" }); + expect(compatibilityRes.status, JSON.stringify(compatibilityRes.body)).toBe(403); + + const bundleRes = await request(app) + .patch("/api/agents/11111111-1111-4111-8111-111111111111/instructions-bundle") + .send({ mode: "external", rootPath: "/srv/paperclip/external-agent" }); + expect(bundleRes.status, JSON.stringify(bundleRes.body)).toBe(403); + expect(mockAgentService.update).not.toHaveBeenCalled(); + expect(mockAgentInstructionsService.updateBundle).not.toHaveBeenCalled(); + }); + + it("rejects external instruction roots through the generic agent patch", async () => { + mockSyncInstructionsBundleConfigFromFilePath.mockImplementation((_agent, config) => config); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + + const res = await request(app) + .patch("/api/agents/11111111-1111-4111-8111-111111111111") + .send({ + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + instructionsEntryFile: "AGENTS.md", + }, + }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.error).toContain("Instance admin"); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + + it("rejects external instruction roots during both hire and direct creation", async () => { + const actor = { + type: "board", + userId: "company-admin", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }; + const db = { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(async () => [{ + id: "company-1", + requireBoardApprovalForNewAgents: false, + }]), + })), + })), + }; + const app = await createApp(actor, db); + const body = { + name: "External agent", + adapterType: "codex_local", + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + instructionsEntryFile: "AGENTS.md", + }, + }; + + const hireRes = await request(app) + .post("/api/companies/company-1/agent-hires") + .send(body); + expect(hireRes.status, JSON.stringify(hireRes.body)).toBe(403); + expect(hireRes.body.error).toContain("Instance admin"); + + const createRes = await request(app) + .post("/api/companies/company-1/agents") + .send(body); + expect(createRes.status, JSON.stringify(createRes.body)).toBe(403); + expect(createRes.body.error).toContain("Instance admin"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + it("denies non-privileged agents from reading peer instructions bundles", async () => { mockAgentService.getById.mockImplementation(async (id: string) => { if (id === "agent-reader") { diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index 7178938e0a..7db8be7be4 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -50,6 +50,16 @@ const mockWorkspaceDiffReprojection = vi.hoisted(() => ({ })); const mockLogActivity = vi.hoisted(() => vi.fn()); const mockQueueRuntimeRequestResolution = vi.hoisted(() => vi.fn()); +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + decide: vi.fn(), + hasPermission: vi.fn(), +})); +const mockWorkspaceOperationService = vi.hoisted(() => ({ + getById: vi.fn(), + listForRun: vi.fn(), + readLog: vi.fn(), +})); const routeAgentId = "11111111-1111-4111-8111-111111111111"; @@ -100,16 +110,7 @@ function registerModuleMocks() { vi.doMock("../services/index.js", () => ({ agentService: () => mockAgentService, agentInstructionsService: () => ({}), - accessService: () => ({ - canUser: vi.fn(async () => true), - decide: vi.fn(async (input: { action?: string }) => ({ - allowed: true, - action: input.action, - reason: "allow_explicit_grant", - explanation: "Allowed by test grant.", - })), - hasPermission: vi.fn(async () => true), - }), + accessService: () => mockAccessService, approvalService: () => ({}), builtInAgentService: () => ({ ensureCompanyDefaultAgentGrants: vi.fn() }), companySkillService: () => ({ listRuntimeSkillEntries: vi.fn() }), @@ -120,7 +121,7 @@ function registerModuleMocks() { logActivity: mockLogActivity, secretService: () => ({}), syncInstructionsBundleConfigFromFilePath: vi.fn((_agent, config) => config), - workspaceOperationService: () => ({}), + workspaceOperationService: () => mockWorkspaceOperationService, })); vi.doMock("../adapters/index.js", () => ({ @@ -236,6 +237,14 @@ describe("agent live run routes", () => { vi.doUnmock("../middleware/index.js"); registerModuleMocks(); vi.clearAllMocks(); + mockAccessService.canUser.mockResolvedValue(true); + mockAccessService.decide.mockImplementation(async (input: { action?: string }) => ({ + allowed: true, + action: input.action, + reason: "allow_explicit_grant", + explanation: "Allowed by test grant.", + })); + mockAccessService.hasPermission.mockResolvedValue(true); mockIssueService.getByIdentifier.mockResolvedValue({ id: "issue-1", companyId: "company-1", @@ -313,6 +322,11 @@ describe("agent live run routes", () => { agentId: "agent-1", status: "succeeded", }); + mockWorkspaceOperationService.getById.mockResolvedValue({ + id: "operation-1", + companyId: "company-1", + runId: "run-1", + }); mockQueueRuntimeRequestResolution.mockReturnValue({ commandId: "command-resolution-1", }); @@ -465,6 +479,52 @@ describe("agent live run routes", () => { }); }); + it.each(["skill_test", "task_bridge"])( + "denies %s keys from company-wide run and workspace logs", + async (kind) => { + mockAccessService.decide.mockImplementation(async (input: { action?: string }) => ({ + allowed: input.action !== "company_scope:read", + action: input.action, + reason: input.action === "company_scope:read" ? "deny_key_scope" : "allow_explicit_grant", + explanation: input.action === "company_scope:read" + ? "Restricted keys cannot read company-wide run telemetry." + : "Allowed by test grant.", + })); + const actor = { + type: "agent", + agentId: routeAgentId, + companyId: "company-1", + source: "agent_key", + keyScope: kind === "skill_test" + ? { kind, issueId: "issue-1" } + : { kind, parentIssueId: "issue-1" }, + }; + const app = await createApp({}, actor); + const paths = [ + "/api/companies/company-1/heartbeat-runs", + "/api/companies/company-1/live-runs", + "/api/heartbeat-runs/run-1", + "/api/heartbeat-runs/run-1/events", + "/api/heartbeat-runs/run-1/log", + "/api/heartbeat-runs/run-1/workspace-operations", + "/api/workspace-operations/operation-1/log", + ]; + + for (const path of paths) { + const res = await requestApp(app, (baseUrl) => request(baseUrl).get(path)); + expect(res.status, `${path}: ${JSON.stringify(res.body)}`).toBe(403); + expect(res.body.error).toContain("Run telemetry"); + } + + expect(mockHeartbeatService.readLog).not.toHaveBeenCalled(); + expect(mockWorkspaceOperationService.readLog).not.toHaveBeenCalled(); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "company_scope:read", + resource: { type: "company", companyId: "company-1" }, + })); + }, + ); + it("caps company live run polling by default", async () => { const rows = Array.from({ length: 75 }, (_, index) => ({ id: `run-${index}`, diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 4eadf5318a..06aee69cf8 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -43,6 +43,7 @@ const baseAgent = { const mockAgentService = vi.hoisted(() => ({ getById: vi.fn(), getConfigRevision: vi.fn(), + listConfigRevisions: vi.fn(), list: vi.fn(), create: vi.fn(), activatePendingApproval: vi.fn(), @@ -182,6 +183,10 @@ function registerModuleMocks() { vi.doMock("../services/agent-instructions.js", () => ({ agentInstructionsService: () => mockAgentInstructionsService, + agentInstructionsBundleMode: (agent: { adapterConfig?: unknown }) => { + const config = agent.adapterConfig as Record | undefined; + return config?.instructionsBundleMode === "external" ? "external" : "managed"; + }, syncInstructionsBundleConfigFromFilePath: mockSyncInstructionsBundleConfigFromFilePath, })); @@ -291,6 +296,7 @@ describe.sequential("agent permission routes", () => { vi.resetAllMocks(); mockAgentService.getById.mockReset(); mockAgentService.getConfigRevision.mockReset(); + mockAgentService.listConfigRevisions.mockReset(); mockAgentService.list.mockReset(); mockAgentService.create.mockReset(); mockAgentService.activatePendingApproval.mockReset(); @@ -337,6 +343,7 @@ describe.sequential("agent permission routes", () => { mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); mockAgentService.getById.mockResolvedValue(baseAgent); mockAgentService.getConfigRevision.mockResolvedValue(null); + mockAgentService.listConfigRevisions.mockResolvedValue([]); mockAgentService.list.mockResolvedValue([baseAgent]); mockAgentService.getChainOfCommand.mockResolvedValue([]); mockAgentService.resolveByReference.mockResolvedValue({ ambiguous: false, agent: baseAgent }); @@ -422,7 +429,8 @@ describe.sequential("agent permission routes", () => { expect(res.body.runtimeConfig).toEqual({}); }, 20_000); - it("keeps board agent detail unredacted for low-trust agents", async () => { + it("redacts env values in board agent detail responses", async () => { + const plaintextValue = "plain-value-must-not-leak"; mockAgentService.getById.mockResolvedValue({ ...baseAgent, permissions: { @@ -431,7 +439,15 @@ describe.sequential("agent permission routes", () => { }, adapterConfig: { command: "pnpm agent:run", - env: { PAPERCLIP_API_KEY: "secret-test-key" }, + env: { + LEGACY_VALUE: plaintextValue, + PLAIN_VALUE: { type: "plain", value: plaintextValue }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "33333333-3333-4333-8333-333333333333", + version: "latest", + }, + }, }, runtimeConfig: { heartbeat: { enabled: false }, @@ -451,14 +467,190 @@ describe.sequential("agent permission routes", () => { expect(res.status).toBe(200); expect(res.body.adapterConfig).toMatchObject({ command: "pnpm agent:run", - env: { PAPERCLIP_API_KEY: "secret-test-key" }, + env: { + LEGACY_VALUE: { type: "plain", value: "***REDACTED***" }, + PLAIN_VALUE: { type: "plain", value: "***REDACTED***" }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "33333333-3333-4333-8333-333333333333", + version: "latest", + }, + }, }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); expect(res.body.runtimeConfig).toMatchObject({ heartbeat: { enabled: false }, }); expect(res.body.permissions).toMatchObject({ trustPreset: LOW_TRUST_REVIEW_PRESET }); }, 20_000); + // TEC-7032 reported the leak against the company agent-list endpoint, which + // serialises rows directly instead of going through buildAgentDetail. + it("redacts env values in board GET /api/companies/:companyId/agents responses", async () => { + const plaintextValue = "listed-value-must-not-leak"; + mockAgentService.list.mockResolvedValue([ + { + ...baseAgent, + adapterConfig: { + command: "pnpm agent:run", + env: { + LEGACY_VALUE: plaintextValue, + PLAIN_VALUE: { type: "plain", value: plaintextValue }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "55555555-5555-4555-8555-555555555555", + version: "latest", + }, + }, + }, + }, + ]); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => + request(baseUrl).get(`/api/companies/${companyId}/agents`), + ); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(1); + expect(res.body[0].adapterConfig).toMatchObject({ + command: "pnpm agent:run", + env: { + LEGACY_VALUE: { type: "plain", value: "***REDACTED***" }, + PLAIN_VALUE: { type: "plain", value: "***REDACTED***" }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "55555555-5555-4555-8555-555555555555", + version: "latest", + }, + }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }, 20_000); + + // Mutation routes echo the stored row back, so they leak the same values the + // GET paths redact. + it("redacts env values in agent mutation responses", async () => { + const plaintextValue = "mutation-value-must-not-leak"; + const storedAgent = { + ...baseAgent, + adapterConfig: { + env: { PLAIN_VALUE: { type: "plain", value: plaintextValue } }, + }, + }; + mockAgentService.getById.mockResolvedValue(storedAgent); + mockAgentService.update.mockResolvedValue(storedAgent); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => + request(baseUrl).patch(`/api/agents/${agentId}`).send({ title: "Renamed" }), + ); + + expect(res.status).toBe(200); + expect(res.body.adapterConfig.env).toEqual({ + PLAIN_VALUE: { type: "plain", value: "***REDACTED***" }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }, 20_000); + + it("redacts env values in GET /api/agents/me responses", async () => { + const plaintextValue = "self-value-must-not-leak"; + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + adapterConfig: { + env: { + EXISTING_VALUE: plaintextValue, + NEW_VALUE: { type: "plain", value: plaintextValue }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "44444444-4444-4444-8444-444444444444", + version: 2, + }, + }, + }, + }); + + const app = await createApp({ + type: "agent", + agentId, + companyId, + runId: null, + source: "agent_key", + }); + + const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/agents/me")); + + expect(res.status).toBe(200); + expect(res.body.adapterConfig.env).toEqual({ + EXISTING_VALUE: { type: "plain", value: "***REDACTED***" }, + NEW_VALUE: { type: "plain", value: "***REDACTED***" }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "44444444-4444-4444-8444-444444444444", + version: 2, + }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }, 20_000); + + it("preserves stored env values when a redacted detail response is submitted unchanged", async () => { + const plaintextValue = "stored-value-must-be-preserved"; + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + adapterConfig: { + env: { + EXISTING_VALUE: { type: "plain", value: plaintextValue }, + }, + }, + }); + mockAgentService.update.mockResolvedValue(baseAgent); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const redactedResponse = await requestApp(app, (baseUrl) => + request(baseUrl).get(`/api/agents/${agentId}`), + ); + expect(redactedResponse.status).toBe(200); + const redactedEnv = redactedResponse.body.adapterConfig.env as Record; + expect(redactedEnv.EXISTING_VALUE).toEqual({ type: "plain", value: "***REDACTED***" }); + + const patchRes = await requestApp(app, (baseUrl) => + request(baseUrl).patch(`/api/agents/${agentId}`).send({ + title: "Renamed while redacted env round-trips", + adapterConfig: redactedResponse.body.adapterConfig, + }), + ); + + expect(patchRes.status).toBe(200); + const updateCallArgs = mockAgentService.update.mock.calls[0]?.[1] as + | { adapterConfig?: Record } + | undefined; + expect(updateCallArgs?.adapterConfig?.env).toEqual({ + EXISTING_VALUE: { type: "plain", value: plaintextValue }, + }); + expect(JSON.stringify(updateCallArgs?.adapterConfig ?? {})).not.toContain("***REDACTED***"); + }, 20_000); + it("redacts company agent list for authenticated company members without agent admin permission", async () => { mockAccessService.canUser.mockResolvedValue(false); mockAccessService.decide.mockImplementation(async (input: { action?: string }) => ({ @@ -628,6 +820,43 @@ describe.sequential("agent permission routes", () => { expect(mockAgentService.rollbackConfigRevision).not.toHaveBeenCalled(); }); + it("redacts plaintext env values in configuration rollback responses", async () => { + const revisionId = "33333333-3333-4333-8333-333333333333"; + const plaintextValue = "rollback-value-must-not-leak"; + mockAgentService.getConfigRevision.mockResolvedValue({ + id: revisionId, + afterConfig: { + adapterType: "process", + adapterConfig: {}, + runtimeConfig: {}, + }, + }); + mockAgentService.rollbackConfigRevision.mockResolvedValue({ + ...baseAgent, + adapterConfig: { + env: { GENERIC_NAME: { type: "plain", value: plaintextValue } }, + }, + }); + + const app = await createApp({ + type: "board", + userId: "instance-admin-user", + source: "session", + isInstanceAdmin: true, + companyIds: [companyId], + }); + + const res = await requestApp(app, (baseUrl) => + request(baseUrl).post(`/api/agents/${agentId}/config-revisions/${revisionId}/rollback`), + ); + + expect(res.status).toBe(200); + expect(res.body.adapterConfig.env).toEqual({ + GENERIC_NAME: { type: "plain", value: "***REDACTED***" }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }); + it("blocks api key creation for authenticated company members without agent admin permission", async () => { mockAccessService.canUser.mockResolvedValue(false); @@ -1649,6 +1878,13 @@ describe.sequential("agent permission routes", () => { // the read-only permission loosening introduced by this PR. mockAccessService.canUser.mockResolvedValue(false); mockAccessService.hasPermission.mockResolvedValue(false); + const plaintextValue = "configuration-value-must-not-leak"; + mockAgentService.getById.mockResolvedValue({ + ...baseAgent, + adapterConfig: { + env: { GENERIC_NAME: { type: "plain", value: plaintextValue } }, + }, + }); const app = await createApp({ type: "board", @@ -1661,6 +1897,78 @@ describe.sequential("agent permission routes", () => { const res = await request(app).get(`/api/agents/${agentId}/configuration`); expect(res.status).toBe(200); + expect(res.body.adapterConfig.env).toEqual({ + GENERIC_NAME: { type: "plain", value: "***REDACTED***" }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }); + + it("redacts plaintext env values in company configuration-list responses", async () => { + const plaintextValue = "configuration-list-value-must-not-leak"; + mockAgentService.list.mockResolvedValue([ + { + ...baseAgent, + adapterConfig: { + env: { GENERIC_NAME: { type: "plain", value: plaintextValue } }, + }, + }, + ]); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app).get(`/api/companies/${companyId}/agent-configurations`); + + expect(res.status).toBe(200); + expect(res.body[0].adapterConfig.env).toEqual({ + GENERIC_NAME: { type: "plain", value: "***REDACTED***" }, + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); + }); + + it("redacts plaintext env values in configuration revisions", async () => { + const plaintextValue = "revision-value-must-not-leak"; + mockAgentService.listConfigRevisions.mockResolvedValue([ + { + id: "33333333-3333-4333-8333-333333333333", + beforeConfig: { + adapterConfig: { + env: { GENERIC_NAME: { type: "plain", value: plaintextValue } }, + }, + }, + afterConfig: { + adapterConfig: { + env: { GENERIC_NAME: plaintextValue }, + }, + }, + }, + ]); + + const app = await createApp({ + type: "board", + userId: "board-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app).get(`/api/agents/${agentId}/config-revisions`); + + expect(res.status).toBe(200); + expect(res.body[0].beforeConfig.adapterConfig.env.GENERIC_NAME).toEqual({ + type: "plain", + value: "***REDACTED***", + }); + expect(res.body[0].afterConfig.adapterConfig.env.GENERIC_NAME).toEqual({ + type: "plain", + value: "***REDACTED***", + }); + expect(JSON.stringify(res.body)).not.toContain(plaintextValue); }); it("denies an agent actor without configure or suggest grants when reading peer config", async () => { diff --git a/server/src/__tests__/assets.test.ts b/server/src/__tests__/assets.test.ts index 61a5ac042d..9c8308fb92 100644 --- a/server/src/__tests__/assets.test.ts +++ b/server/src/__tests__/assets.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import express from "express"; +import { Readable } from "node:stream"; import request from "supertest"; import { MAX_ATTACHMENT_BYTES } from "../attachment-types.js"; import type { StorageService } from "../storage/types.js"; @@ -407,3 +408,89 @@ describe("POST /api/companies/:companyId/logo", () => { expect(createAssetMock).not.toHaveBeenCalled(); }); }); + +describe("GET /api/assets/:assetId/content", () => { + beforeEach(() => { + vi.resetModules(); + vi.doUnmock("../services/index.js"); + vi.doUnmock("../routes/assets.js"); + vi.doUnmock("../routes/authz.js"); + vi.doUnmock("../middleware/index.js"); + registerModuleMocks(); + vi.clearAllMocks(); + getAssetByIdMock.mockReset(); + }); + + it("downloads script-capable HTML with nosniff and a sandbox CSP", async () => { + const html = Buffer.from(""); + const storage = createStorageService("text/html"); + getAssetByIdMock.mockResolvedValue({ + ...createAsset(), + contentType: "text/html", + byteSize: html.byteLength, + originalFilename: "proof.html", + }); + vi.mocked(storage.getObject).mockResolvedValue({ + stream: Readable.from(html), + contentType: "text/html", + contentLength: html.byteLength, + }); + + const res = await requestApp(await createApp(storage), (baseUrl) => + request(baseUrl).get("/api/assets/asset-1/content"), + ); + + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toBe('attachment; filename="proof.html"'); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + expect(res.headers["content-security-policy"]).toBe("sandbox; default-src 'none'"); + }); + + it("downloads SVG instead of rendering it on the application origin", async () => { + const svg = Buffer.from(""); + const storage = createStorageService("image/svg+xml; charset=utf-8"); + getAssetByIdMock.mockResolvedValue({ + ...createAsset(), + contentType: "image/svg+xml; charset=utf-8", + byteSize: svg.byteLength, + originalFilename: "logo.svg", + }); + vi.mocked(storage.getObject).mockResolvedValue({ + stream: Readable.from(svg), + contentType: "image/svg+xml; charset=utf-8", + contentLength: svg.byteLength, + }); + + const res = await requestApp(await createApp(storage), (baseUrl) => + request(baseUrl).get("/api/assets/asset-1/content"), + ); + + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toBe('attachment; filename="logo.svg"'); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + expect(res.headers["content-security-policy"]).toBe("sandbox; default-src 'none'"); + }); + + it("keeps curated image types inline", async () => { + const image = Buffer.from("png-bytes"); + const storage = createStorageService("image/png"); + getAssetByIdMock.mockResolvedValue({ + ...createAsset(), + byteSize: image.byteLength, + }); + vi.mocked(storage.getObject).mockResolvedValue({ + stream: Readable.from(image), + contentType: "image/png", + contentLength: image.byteLength, + }); + + const res = await requestApp(await createApp(storage), (baseUrl) => + request(baseUrl).get("/api/assets/asset-1/content"), + ); + + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toBe('inline; filename="logo.png"'); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + expect(res.headers).not.toHaveProperty("content-security-policy"); + }); +}); diff --git a/server/src/__tests__/cloud-tenant-transient-db-retry.test.ts b/server/src/__tests__/cloud-tenant-transient-db-retry.test.ts new file mode 100644 index 0000000000..70ada7397d --- /dev/null +++ b/server/src/__tests__/cloud-tenant-transient-db-retry.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; + +import { + isTransientDbConnectionError, + retryOnTransientDbConnectionError, +} from "../middleware/auth.ts"; + +/** The shape drizzle produces: a wrapper whose `cause` is the driver error. */ +function driverClosedError(code: string): Error { + const driver = Object.assign(new Error(`write ${code} db.example.internal:5432`), { code }); + return new Error("Failed query: insert into \"companies\" (…)", { cause: driver }); +} + +describe("isTransientDbConnectionError", () => { + it("detects a closed-connection code anywhere on the cause chain", () => { + expect(isTransientDbConnectionError(driverClosedError("CONNECTION_CLOSED"))).toBe(true); + expect(isTransientDbConnectionError(driverClosedError("CONNECTION_ENDED"))).toBe(true); + expect(isTransientDbConnectionError(driverClosedError("CONNECTION_DESTROYED"))).toBe(true); + const bare = Object.assign(new Error("write CONNECTION_CLOSED host:5432"), { + code: "CONNECTION_CLOSED", + }); + expect(isTransientDbConnectionError(bare)).toBe(true); + }); + + it("rejects everything else", () => { + expect(isTransientDbConnectionError(new Error("boom"))).toBe(false); + const unique = Object.assign(new Error("duplicate key"), { code: "23505" }); + expect(isTransientDbConnectionError(unique)).toBe(false); + expect(isTransientDbConnectionError(new Error("outer", { cause: unique }))).toBe(false); + expect(isTransientDbConnectionError("CONNECTION_CLOSED")).toBe(false); + expect(isTransientDbConnectionError(undefined)).toBe(false); + }); +}); + +describe("retryOnTransientDbConnectionError", () => { + it("retries exactly once after a transient closed connection", async () => { + let calls = 0; + const result = await retryOnTransientDbConnectionError(async () => { + calls += 1; + if (calls === 1) throw driverClosedError("CONNECTION_CLOSED"); + return "ok"; + }); + expect(result).toBe("ok"); + expect(calls).toBe(2); + }); + + it("propagates a non-transient failure without retrying", async () => { + let calls = 0; + await expect( + retryOnTransientDbConnectionError(async () => { + calls += 1; + throw new Error("constraint violation"); + }), + ).rejects.toThrow("constraint violation"); + expect(calls).toBe(1); + }); + + it("propagates the second failure when the retry also dies", async () => { + let calls = 0; + await expect( + retryOnTransientDbConnectionError(async () => { + calls += 1; + throw driverClosedError("CONNECTION_CLOSED"); + }), + ).rejects.toThrow("Failed query"); + expect(calls).toBe(2); + }); +}); diff --git a/server/src/__tests__/companies-route-cross-company-authz.test.ts b/server/src/__tests__/companies-route-cross-company-authz.test.ts index 520095fe74..4a92461f9c 100644 --- a/server/src/__tests__/companies-route-cross-company-authz.test.ts +++ b/server/src/__tests__/companies-route-cross-company-authz.test.ts @@ -18,6 +18,7 @@ const mockCompanyService = vi.hoisted(() => ({ const mockAgentService = vi.hoisted(() => ({ getById: vi.fn(), + list: vi.fn(), })); const mockAccessService = vi.hoisted(() => ({ @@ -169,6 +170,7 @@ function resetMockDefaults() { 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 }); diff --git a/server/src/__tests__/company-import-cloud-floor.test.ts b/server/src/__tests__/company-import-cloud-floor.test.ts index 71482ac227..cc17301bd5 100644 --- a/server/src/__tests__/company-import-cloud-floor.test.ts +++ b/server/src/__tests__/company-import-cloud-floor.test.ts @@ -23,7 +23,7 @@ const mockLogActivity = vi.hoisted(() => vi.fn()); vi.mock("../services/index.js", () => ({ accessService: () => ({}), - agentService: () => ({}), + agentService: () => ({ list: vi.fn().mockResolvedValue([]) }), budgetService: () => ({}), companyArtifactsService: () => ({}), companyPortabilityService: () => mockPortabilityService, diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index 94c57559ee..9872cd2f6b 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -14,6 +14,7 @@ const mockCompanyService = vi.hoisted(() => ({ const mockAgentService = vi.hoisted(() => ({ getById: vi.fn(), + list: vi.fn(), })); const mockAccessService = vi.hoisted(() => ({ @@ -341,6 +342,7 @@ describe.sequential("company portability routes", () => { companyId, role: id === ceoAgentId ? "ceo" : "engineer", })); + mockAgentService.list.mockResolvedValue([]); mockCompanyPortabilityService.exportBundle.mockResolvedValue(createExportResult()); mockCompanyPortabilityService.previewExport.mockResolvedValue({ rootPath: "paperclip", @@ -456,8 +458,18 @@ describe.sequential("company portability routes", () => { expect(res.body.rootPath).toBe("paperclip"); } expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledTimes(2); - expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith(1, companyId, exportRequest); - expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith(2, companyId, exportRequest); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith( + 1, + companyId, + exportRequest, + { allowExternalInstructions: false }, + ); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenNthCalledWith( + 2, + companyId, + exportRequest, + { allowExternalInstructions: false }, + ); }); it.sequential("allows board users to export through legacy and CEO-safe bundle routes", async () => { @@ -479,6 +491,166 @@ describe.sequential("company portability routes", () => { expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledTimes(2); }); + it.sequential("requires instance-admin access when a company export includes external instructions", async () => { + mockAgentService.list.mockResolvedValue([{ + id: "external-agent", + companyId, + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + }, + }]); + const nonAdminActors = [ + { + type: "board", + userId: "company-admin", + companyIds: [companyId], + memberships: [{ companyId, status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }, + { + type: "agent", + agentId: ceoAgentId, + companyId, + source: "agent_key", + runId: "run-1", + }, + ]; + + for (const actor of nonAdminActors) { + const app = await createApp(actor); + for (const path of [ + `/api/companies/${companyId}/export`, + `/api/companies/${companyId}/exports`, + `/api/companies/${companyId}/exports/preview`, + ]) { + const res = await request(app).post(path).send(exportRequest); + expect(res.status, `${path}: ${JSON.stringify(res.body)}`).toBe(403); + expect(res.body.error).toMatch(/Instance admin|Board access/); + } + } + + expect(mockCompanyPortabilityService.exportBundle).not.toHaveBeenCalled(); + expect(mockCompanyPortabilityService.previewExport).not.toHaveBeenCalled(); + }); + + it.sequential("allows an instance admin to export companies with external instructions", async () => { + mockAgentService.list.mockResolvedValue([{ + id: "external-agent", + companyId, + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + }, + }]); + const app = await createApp({ + type: "board", + userId: "instance-admin", + companyIds: [companyId], + memberships: [{ companyId, status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: true, + }); + + const res = await request(app).post(`/api/companies/${companyId}/exports`).send(exportRequest); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledWith( + companyId, + exportRequest, + { allowExternalInstructions: true }, + ); + }); + + it.sequential("uses the export selector resolver before checking external instructions", async () => { + mockAgentService.list.mockResolvedValue([ + { + id: "external-agent", + name: "Managed-Agent", + companyId, + status: "active", + metadata: null, + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + }, + }, + { + id: "managed-agent", + name: "Managed Agent", + companyId, + status: "active", + metadata: null, + adapterConfig: { instructionsBundleMode: "managed" }, + }, + ]); + mockCompanyPortabilityService.exportBundle.mockResolvedValue(createExportResult()); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: [companyId], + memberships: [{ companyId, status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + const selectedAgentRequest = { + ...exportRequest, + agents: ["managed-agent"], + }; + + const res = await request(app) + .post(`/api/companies/${companyId}/exports`) + .send(selectedAgentRequest); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.list).toHaveBeenCalledWith(companyId, { includeTerminated: true }); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledWith( + companyId, + selectedAgentRequest, + { allowExternalInstructions: false }, + ); + }); + + it.sequential("keeps non-agent exports open when external instructions are excluded", async () => { + mockAgentService.list.mockResolvedValue([{ + id: "external-agent", + companyId, + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/srv/paperclip/external-agent", + }, + }]); + const app = await createApp({ + type: "board", + userId: "company-admin", + companyIds: [companyId], + memberships: [{ companyId, status: "active", membershipRole: "admin" }], + source: "session", + isInstanceAdmin: false, + }); + const companyOnlyRequest = { + include: { + company: true, + agents: false, + projects: false, + issues: false, + skills: false, + }, + }; + + const res = await request(app) + .post(`/api/companies/${companyId}/exports`) + .send(companyOnlyRequest); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockCompanyPortabilityService.exportBundle).toHaveBeenCalledWith( + companyId, + companyOnlyRequest, + { allowExternalInstructions: false }, + ); + }); + it.sequential("rejects CEO agents from exporting another company before services run", async () => { const app = await createApp({ type: "agent", diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index 3ea7f705ee..354e084ca5 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -164,6 +164,10 @@ vi.mock("../services/secrets.js", () => ({ vi.mock("../services/agent-instructions.js", () => ({ agentInstructionsService: () => agentInstructionsSvc, + agentInstructionsBundleMode: (agent: { adapterConfig?: unknown }) => { + const config = agent.adapterConfig as Record | undefined; + return config?.instructionsBundleMode === "external" ? "external" : "managed"; + }, })); vi.mock("../services/instance-settings.js", () => ({ @@ -610,6 +614,51 @@ describe("company portability", () => { expect(asTextFile(exported.files["agents/claudecoder/AGENTS.md"])).toContain(`- "${paperclipKey}"`); }); + it("refuses to read external instruction roots without an instance-admin export grant", async () => { + agentSvc.list.mockResolvedValue([{ + id: "external-agent", + companyId: "company-1", + name: "ExternalAgent", + status: "idle", + role: "engineer", + title: null, + icon: null, + reportsTo: null, + capabilities: null, + adapterType: "codex_local", + adapterConfig: { + instructionsBundleMode: "external", + instructionsRootPath: "/private/host/instructions", + }, + runtimeConfig: {}, + budgetMonthlyCents: 0, + permissions: { canCreateAgents: false }, + metadata: null, + }]); + + await expect(companyPortabilityService({} as any).exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + })).rejects.toMatchObject({ status: 403 }); + expect(agentInstructionsSvc.exportFiles).not.toHaveBeenCalled(); + + await expect(companyPortabilityService({} as any).exportBundle("company-1", { + include: { + company: true, + agents: true, + projects: false, + issues: false, + }, + }, { allowExternalInstructions: true })).resolves.toMatchObject({ + manifest: { agents: [expect.objectContaining({ slug: "externalagent" })] }, + }); + expect(agentInstructionsSvc.exportFiles).toHaveBeenCalledTimes(1); + }); + it("exports agent permission grants through the Paperclip extension and manifest", async () => { const db = { select: vi.fn((selection: Record) => ({ @@ -700,6 +749,14 @@ describe("company portability", () => { adapterConfig: { env: { OPENAI_API_KEY: "sk-inline-secret-value", + OPENAI_KEY: { + type: "plain", + value: "sk-short-key-secret-value", + }, + MONKEY: { + type: "plain", + value: "banana", + }, NODE_ENV: { type: "plain", value: "development", @@ -726,6 +783,7 @@ describe("company portability", () => { const serialized = JSON.stringify(exported); expect(serialized).not.toContain("sk-inline-secret-value"); + expect(serialized).not.toContain("sk-short-key-secret-value"); expect(exported.manifest.envInputs).toContainEqual({ key: "OPENAI_API_KEY", description: "Optional default for OPENAI_API_KEY on agent inlinesecretagent", @@ -736,6 +794,26 @@ describe("company portability", () => { defaultValue: "", portability: "portable", }); + expect(exported.manifest.envInputs).toContainEqual({ + key: "OPENAI_KEY", + description: "Optional default for OPENAI_KEY on agent inlinesecretagent", + agentSlug: "inlinesecretagent", + projectSlug: null, + kind: "secret", + requirement: "optional", + defaultValue: "", + portability: "portable", + }); + expect(exported.manifest.envInputs).toContainEqual({ + key: "MONKEY", + description: "Optional default for MONKEY on agent inlinesecretagent", + agentSlug: "inlinesecretagent", + projectSlug: null, + kind: "plain", + requirement: "optional", + defaultValue: "banana", + portability: "portable", + }); expect(exported.manifest.envInputs).toContainEqual({ key: "NODE_ENV", description: "Optional default for NODE_ENV on agent inlinesecretagent", diff --git a/server/src/__tests__/feedback-service.test.ts b/server/src/__tests__/feedback-service.test.ts index bde8659b03..9770c94170 100644 --- a/server/src/__tests__/feedback-service.test.ts +++ b/server/src/__tests__/feedback-service.test.ts @@ -618,7 +618,7 @@ describeEmbeddedPostgres("feedbackService.saveIssueVote", () => { }); }); - it("builds a detailed sanitized shared bundle with issue and agent context", async () => { + it("builds a sanitized shared bundle without reading external instruction roots", async () => { const { companyId, issueId, targetCommentId, runId } = await seedIssueWithRichAgentComment(); await svc.saveIssueVote({ @@ -662,8 +662,12 @@ describeEmbeddedPostgres("feedbackService.saveIssueVote", () => { expect(sourceRun?.id).toBe(runId); expect(JSON.stringify(sourceRun)).toContain("gpt-5.4"); expect(skillItems?.[1]?.sourceLocator).toBe("https://github.com/octo/research/tree/main/skills/public-skill"); - expect(String(instructions?.entryBody)).toContain("[REDACTED]"); - expect(String(instructions?.entryBody)).not.toContain("secret-value"); + expect(instructions).toBeNull(); + expect(runtime?.configuredInstructionsBundleMode).toBe("external"); + expect(runtime?.configuredInstructionsFilePath).toBeNull(); + expect(runtime?.configuredInstructionsRootPath).toBeNull(); + expect(JSON.stringify(bundle)).not.toContain("secret-value"); + expect(JSON.stringify(bundle)).not.toContain("private-workspace"); }); it("keeps earlier local votes local when a later vote enables sharing", async () => { diff --git a/server/src/__tests__/http-adapter-remote-fetch.test.ts b/server/src/__tests__/http-adapter-remote-fetch.test.ts new file mode 100644 index 0000000000..7801e383dd --- /dev/null +++ b/server/src/__tests__/http-adapter-remote-fetch.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it, vi } from "vitest"; +import { + guardedHttpAdapterFetch, + httpAdapterPrivateEndpointAllowlist, +} from "../adapters/http/remote-fetch.js"; + +describe("HTTP adapter guarded fetch", () => { + it("parses only comma-separated exact HTTP(S) origins", () => { + const allowlist = httpAdapterPrivateEndpointAllowlist([ + "http://127.0.0.1:3100", + "HTTPS://INTERNAL.EXAMPLE:8443/", + "https://internal.example/path", + "https://user:pass@internal.example", + "file:///tmp/socket", + "not-a-url", + ].join(",")); + + expect([...allowlist]).toEqual([ + "http://127.0.0.1:3100", + "https://internal.example:8443", + ]); + }); + + it("allows public HTTP(S) endpoints by default and forces manual redirects", async () => { + const unpinnedFetch = vi.fn(async () => new Response(null, { + status: 302, + headers: { location: "http://127.0.0.1/admin" }, + })); + + const response = await guardedHttpAdapterFetch("https://93.184.216.34/hook", { + method: "POST", + }, { unpinnedFetch }); + + expect(response.status).toBe(302); + expect(unpinnedFetch).toHaveBeenCalledWith( + "https://93.184.216.34/hook", + expect.objectContaining({ method: "POST", redirect: "manual" }), + ); + }); + + it.each([ + "http://127.0.0.1:3100/hook", + "http://10.0.0.8/hook", + "http://172.16.0.8/hook", + "http://192.168.1.8/hook", + ])("blocks private endpoint %s unless its exact origin is allowlisted", async (url) => { + const unpinnedFetch = vi.fn(); + + await expect(guardedHttpAdapterFetch(url, {}, { unpinnedFetch })) + .rejects.toMatchObject({ code: "remote_http_private_endpoint" }); + expect(unpinnedFetch).not.toHaveBeenCalled(); + }); + + it("allows an exact private origin without allowing a sibling port", async () => { + const unpinnedFetch = vi.fn(async () => new Response("ok", { status: 200 })); + const privateEndpointAllowlist = new Set(["http://127.0.0.1:3100"]); + + const response = await guardedHttpAdapterFetch("http://127.0.0.1:3100/hook", {}, { + privateEndpointAllowlist, + unpinnedFetch, + }); + expect(response.status).toBe(200); + + await expect(guardedHttpAdapterFetch("http://127.0.0.1:3101/hook", {}, { + privateEndpointAllowlist, + unpinnedFetch, + })).rejects.toMatchObject({ code: "remote_http_private_endpoint" }); + }); + + it("rejects metadata link-local addresses even when their origin is allowlisted", async () => { + const unpinnedFetch = vi.fn(); + + await expect(guardedHttpAdapterFetch("http://169.254.169.254/latest/meta-data/", {}, { + privateEndpointAllowlist: new Set(["http://169.254.169.254"]), + unpinnedFetch, + })).rejects.toMatchObject({ code: "remote_http_private_endpoint" }); + expect(unpinnedFetch).not.toHaveBeenCalled(); + }); + + it("rejects private DNS results before opening a socket", async () => { + const socketFactory = vi.fn(() => { + throw new Error("must not dial"); + }); + + await expect(guardedHttpAdapterFetch("http://internal.example/hook", {}, { + lookup: async () => [{ address: "10.0.0.8", family: 4 }], + socketFactory, + })).rejects.toMatchObject({ code: "remote_http_private_endpoint" }); + expect(socketFactory).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/http-log-redaction.test.ts b/server/src/__tests__/http-log-redaction.test.ts index e6ac40ea5e..9028ba3746 100644 --- a/server/src/__tests__/http-log-redaction.test.ts +++ b/server/src/__tests__/http-log-redaction.test.ts @@ -127,4 +127,42 @@ describe("HTTP logger redaction", () => { expect(log.req.query).toBeUndefined(); expect(log.reqQuery).toBeUndefined(); }); + + it("redacts failed secret payload values from structured request logs", async () => { + const chunks: string[] = []; + const stream = new Writable({ + write(chunk, _encoding, callback) { + chunks.push(chunk.toString()); + callback(); + }, + }); + const testLogger = pino({ redact: [...HTTP_LOG_REDACT_PATHS] }, stream); + const app = express(); + app.use(express.json()); + app.use(createHttpLogger(testLogger)); + app.post("/api/companies/:companyId/secrets", (_req, res) => { + res.status(422).json({ error: "validation failed" }); + }); + + const response = await request(app) + .post("/api/companies/company-1/secrets") + .send({ + name: "OpenAI", + value: "value-canary-4c845d", + metadata: { token: "token-canary-902ffc" }, + }); + + expect(response.status).toBe(422); + const output = chunks.join(""); + expect(output).not.toMatch(/value-canary-4c845d|token-canary-902ffc/); + + const log = JSON.parse(output.trim()) as { + reqBody: Record; + }; + expect(log.reqBody).toEqual({ + name: "OpenAI", + value: "[REDACTED]", + metadata: { token: "[REDACTED]" }, + }); + }); }); diff --git a/server/src/__tests__/low-trust-red-team-routes.test.ts b/server/src/__tests__/low-trust-red-team-routes.test.ts index 8288f55744..b4478c7d1d 100644 --- a/server/src/__tests__/low-trust-red-team-routes.test.ts +++ b/server/src/__tests__/low-trust-red-team-routes.test.ts @@ -1081,6 +1081,31 @@ describeEmbeddedPostgres("low-trust red-team HTTP route regression suite", () => expect(issueScopedLowTrustRes.body).not.toHaveProperty("runtimeConfig"); expectNoCanary(issueScopedLowTrustRes.body, fixture.canaries.agentConfig); + for (const restrictedActor of [ + skillTestActor(fixture), + { + ...standardActor, + source: "agent_key" as const, + keyScope: { + kind: "task_bridge" as const, + parentIssueId: fixture.issues.assignedReview.id, + }, + }, + ]) { + const restrictedRes = await request(createApp(db, restrictedActor)).get("/api/agents/me"); + expect(restrictedRes.status, JSON.stringify(restrictedRes.body)).toBe(200); + expect(restrictedRes.body).toMatchObject({ + id: fixture.agents.standard.id, + companyId: fixture.company.id, + keyScope: restrictedActor.keyScope, + }); + expect(restrictedRes.body).not.toHaveProperty("adapterConfig"); + expect(restrictedRes.body).not.toHaveProperty("runtimeConfig"); + expect(restrictedRes.body).not.toHaveProperty("permissions"); + expect(restrictedRes.body).not.toHaveProperty("access"); + expectNoCanary(restrictedRes.body, fixture.canaries.agentConfig); + } + await db.update(issues).set({ executionPolicy: null }).where(eq(issues.id, fixture.issues.assignedReview.id)); await db.update(projects).set({ diff --git a/server/src/__tests__/project-goal-validation.test.ts b/server/src/__tests__/project-goal-validation.test.ts new file mode 100644 index 0000000000..117a792be2 --- /dev/null +++ b/server/src/__tests__/project-goal-validation.test.ts @@ -0,0 +1,162 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import { + companies, + createDb, + goals, + projectGoals, + projects as projectsTable, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { HttpError } from "../errors.js"; +import { projectService } from "../services/projects.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres project goal validation tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +// A project's goals must exist and belong to the same company. Before this +// validation, a nonexistent goal id died at the projects.goal_id foreign key +// as an opaque 500 (observed live 2026-09-03, retried four times by the +// caller), and a goal from another company linked silently — the foreign key +// proves existence, not ownership. +describeEmbeddedPostgres("project goal validation", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + let prefixCounter = 0; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-project-goal-validation-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(projectGoals); + await db.delete(projectsTable); + await db.delete(goals); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedCompany(name: string): Promise { + prefixCounter += 1; + const [company] = await db + .insert(companies) + .values({ name, issuePrefix: `GV${prefixCounter}` }) + .returning(); + return company.id; + } + + async function seedGoal(companyId: string, title: string): Promise { + const [goal] = await db + .insert(goals) + .values({ companyId, title, level: "task", status: "active" }) + .returning(); + return goal.id; + } + + function expectUnprocessable(error: unknown, unknownGoalId: string) { + expect(error).toBeInstanceOf(HttpError); + expect((error as HttpError).status).toBe(422); + expect((error as Error).message).toContain(unknownGoalId); + } + + it("creates and links a project to a goal of the same company", async () => { + const companyId = await seedCompany("Valid Co"); + const goalId = await seedGoal(companyId, "Ship it"); + const projects = projectService(db); + + const created = await projects.create(companyId, { name: "Rocket", goalIds: [goalId] }); + expect(created.goalIds).toEqual([goalId]); + }); + + it("rejects a create whose goal id does not exist, before any insert", async () => { + const companyId = await seedCompany("Missing Goal Co"); + const projects = projectService(db); + const ghost = "db7da378-99a6-43ea-964f-000000000000"; + + const failure = await projects + .create(companyId, { name: "Rocket", goalIds: [ghost] }) + .then(() => null, (error: unknown) => error); + expectUnprocessable(failure, ghost); + + const rows = await db.select({ id: projectsTable.id }).from(projectsTable); + expect(rows).toHaveLength(0); + }); + + it("rejects the legacy single goalId field the same way", async () => { + const companyId = await seedCompany("Legacy Field Co"); + const projects = projectService(db); + const ghost = "db7da378-99a6-43ea-964f-111111111111"; + + const failure = await projects + .create(companyId, { name: "Rocket", goalId: ghost }) + .then(() => null, (error: unknown) => error); + expectUnprocessable(failure, ghost); + }); + + it("rejects another company's goal on create — the FK only proves existence", async () => { + const companyId = await seedCompany("Home Co"); + const otherCompanyId = await seedCompany("Other Co"); + const foreignGoalId = await seedGoal(otherCompanyId, "Not yours"); + const projects = projectService(db); + + const failure = await projects + .create(companyId, { name: "Rocket", goalIds: [foreignGoalId] }) + .then(() => null, (error: unknown) => error); + expectUnprocessable(failure, foreignGoalId); + }); + + it("ignores the legacy goalId when an explicit empty goalIds list wins resolution", async () => { + // goalIds and goalId may arrive together; the resolved set (goalIds + // first) is canonical for persistence too. Before this rule, an empty + // list skipped validation while the raw legacy id was still written — + // unvalidated, and unchecked for ownership. + const companyId = await seedCompany("Conflicting Fields Co"); + const otherCompanyId = await seedCompany("Conflicting Other Co"); + const foreignGoalId = await seedGoal(otherCompanyId, "Should not link"); + const projects = projectService(db); + + const created = await projects.create(companyId, { + name: "Rocket", + goalIds: [], + goalId: foreignGoalId, + }); + expect(created.goalIds).toEqual([]); + + const [row] = await db + .select({ goalId: projectsTable.goalId }) + .from(projectsTable) + .where(eq(projectsTable.id, created.id)); + expect(row.goalId).toBeNull(); + }); + + it("rejects an update to an unknown or foreign goal and leaves links unchanged", async () => { + const companyId = await seedCompany("Update Co"); + const otherCompanyId = await seedCompany("Update Other Co"); + const goodGoalId = await seedGoal(companyId, "Good goal"); + const foreignGoalId = await seedGoal(otherCompanyId, "Foreign goal"); + const projects = projectService(db); + + const created = await projects.create(companyId, { name: "Rocket", goalIds: [goodGoalId] }); + + const failure = await projects + .update(created.id, { goalIds: [foreignGoalId] }) + .then(() => null, (error: unknown) => error); + expectUnprocessable(failure, foreignGoalId); + + const fetched = await projects.getById(created.id); + expect(fetched?.goalIds).toEqual([goodGoalId]); + }); +}); diff --git a/server/src/__tests__/redact-sensitive.test.ts b/server/src/__tests__/redact-sensitive.test.ts index 78761a51f6..05034febb7 100644 --- a/server/src/__tests__/redact-sensitive.test.ts +++ b/server/src/__tests__/redact-sensitive.test.ts @@ -50,11 +50,19 @@ describe("redactSensitive", () => { expect(JSON.stringify(out)).not.toContain("\\u001b"); }); - it("does not redact a bare `token` field — pagination cursors and CSRF tokens are not credentials", () => { - const out = redactSensitive({ token: "next-page-cursor", limit: 20 }) as Record; + it("redacts bare value and token fields recursively", () => { + const out = redactSensitive({ + token: "secret-token", + nested: { value: "secret-value" }, + entries: [{ value: "array-secret" }], + limit: 20, + }) as Record; - expect(out.token).toBe("next-page-cursor"); + expect(out.token).toBe("[REDACTED]"); + expect((out.nested as Record).value).toBe("[REDACTED]"); + expect((out.entries as Array>)[0].value).toBe("[REDACTED]"); expect(out.limit).toBe(20); + expect(JSON.stringify(out)).not.toMatch(/secret-token|secret-value|array-secret/); }); it("strips secret-bearing query and fragment values from source URLs", () => { diff --git a/server/src/__tests__/redaction.test.ts b/server/src/__tests__/redaction.test.ts index 2de97caf96..b42699be7c 100644 --- a/server/src/__tests__/redaction.test.ts +++ b/server/src/__tests__/redaction.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { PRP_V1_EVENT_TYPES, REDACTED_EVENT_VALUE, + redactAgentAdapterConfig, redactEventPayload, redactSensitiveText, sanitizeRecord, @@ -626,4 +627,73 @@ second-line\" status=401`, expect(result?.args).toEqual(["--api-key", "not-a-command-secret"]); expect(result?.argv).toEqual(["--api-key", REDACTED_EVENT_VALUE]); }); + + it("redacts every plaintext agent env binding while preserving secret references", () => { + const plaintextValue = "adapter-env-value-must-not-leak"; + + const result = redactAgentAdapterConfig({ + command: "pnpm agent:run", + env: { + EXISTING_VALUE: plaintextValue, + NEW_VALUE: { type: "plain", value: plaintextValue }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "55555555-5555-4555-8555-555555555555", + version: "latest", + }, + USER_SECRET_REFERENCE: { + type: "user_secret_ref", + key: "GITHUB_TOKEN", + }, + }, + }); + + expect(result).toEqual({ + command: "pnpm agent:run", + env: { + EXISTING_VALUE: { type: "plain", value: REDACTED_EVENT_VALUE }, + NEW_VALUE: { type: "plain", value: REDACTED_EVENT_VALUE }, + SECRET_REFERENCE: { + type: "secret_ref", + secretId: "55555555-5555-4555-8555-555555555555", + version: "latest", + }, + USER_SECRET_REFERENCE: { + type: "user_secret_ref", + key: "GITHUB_TOKEN", + }, + }, + }); + expect(JSON.stringify(result)).not.toContain(plaintextValue); + }); + + it("redacts non-env adapter keys while leaving env binding shapes intact", () => { + const result = redactAgentAdapterConfig({ + command: "pnpm agent:run", + apiKey: "adapter-level-secret", + env: { + API_KEY: "env-level-secret", + AUTH_TOKEN: { type: "plain", value: "another-env-secret" }, + }, + }); + + // Non-env keys still go through the shared payload sanitizer. + expect(result.apiKey).toBe(REDACTED_EVENT_VALUE); + expect(result.command).toBe("pnpm agent:run"); + + // Env bindings keep their binding shape rather than collapsing to a bare + // sentinel string, which is what a second sanitizer pass would produce for + // these sensitive-looking key names. + expect(result.env).toEqual({ + API_KEY: { type: "plain", value: REDACTED_EVENT_VALUE }, + AUTH_TOKEN: { type: "plain", value: REDACTED_EVENT_VALUE }, + }); + }); + + it("redacts adapter configs that have no env block", () => { + expect(redactAgentAdapterConfig({ command: "pnpm agent:run", apiKey: "secret" })).toEqual({ + command: "pnpm agent:run", + apiKey: REDACTED_EVENT_VALUE, + }); + }); }); diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index 91cb611f7e..ffd4c3893f 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -42,12 +42,20 @@ const mockSecretService = vi.hoisted(() => ({ resolveSecretValueForAgentAccess: vi.fn(), })); const mockLogActivity = vi.hoisted(() => vi.fn()); +const mockAccessService = vi.hoisted(() => ({ + decide: vi.fn(), +})); vi.mock("../services/index.js", () => ({ + accessService: () => mockAccessService, secretService: () => mockSecretService, logActivity: mockLogActivity, })); +vi.mock("../services/access.js", () => ({ + accessService: () => mockAccessService, +})); + function createApp(actor: Record = { type: "board", userId: "user-1", @@ -72,6 +80,12 @@ describe("secret routes", () => { mock.mockReset(); } mockLogActivity.mockReset(); + mockAccessService.decide.mockReset(); + mockAccessService.decide.mockResolvedValue({ + allowed: true, + reason: "allow_standard_agent", + explanation: "Allowed by test policy", + }); }); it("returns an opaque secretRef in agent secret metadata without internal binding details", async () => { @@ -1078,6 +1092,32 @@ describe("secret routes", () => { expect(res.body[0]).toMatchObject({ id: expect.any(String), name: "MY_API_KEY", key: "my_api_key", status: "active" }); }); + it.each(["skill_test", "task_bridge"])( + "rejects %s keys from the company secret catalog", + async (kind) => { + mockAccessService.decide.mockResolvedValue({ + allowed: false, + reason: "deny_key_scope", + explanation: "Restricted keys cannot read the company secret catalog.", + }); + + const res = await request(createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + keyScope: { kind }, + })).get("/api/companies/company-1/secrets/catalog"); + + expect(res.status).toBe(403); + expect(mockSecretService.list).not.toHaveBeenCalled(); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "secrets:read", + resource: { type: "company", companyId: "company-1" }, + })); + }, + ); + it("rejects unauthenticated requests", async () => { const res = await request(createApp({ type: "none" })) .get("/api/companies/company-1/secrets/catalog"); diff --git a/server/src/__tests__/workspace-command-authz.test.ts b/server/src/__tests__/workspace-command-authz.test.ts new file mode 100644 index 0000000000..d98c234183 --- /dev/null +++ b/server/src/__tests__/workspace-command-authz.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import { + collectExecutionWorkspaceCommandPaths, + collectIssueWorkspaceCommandPaths, + collectProjectExecutionWorkspaceCommandPaths, + collectProjectWorkspaceCommandPaths, +} from "../routes/workspace-command-authz.js"; + +describe("workspace host-command mutation detection", () => { + it.each([ + { + name: "project execution policy commands", + actual: () => collectProjectExecutionWorkspaceCommandPaths({ + workspaceRuntime: { commands: [{ name: "seed", command: "pnpm seed" }] }, + }), + expected: "executionWorkspacePolicy.workspaceRuntime.commands[0].command", + }, + { + name: "project execution policy services", + actual: () => collectProjectExecutionWorkspaceCommandPaths({ + workspaceRuntime: { services: [{ name: "web", command: "pnpm dev" }] }, + }), + expected: "executionWorkspacePolicy.workspaceRuntime.services[0].command", + }, + { + name: "project workspace jobs", + actual: () => collectProjectWorkspaceCommandPaths({ + runtimeConfig: { workspaceRuntime: { jobs: [{ name: "build", command: "pnpm build" }] } }, + }), + expected: "runtimeConfig.workspaceRuntime.jobs[0].command", + }, + { + name: "issue execution workspace services", + actual: () => collectIssueWorkspaceCommandPaths({ + executionWorkspaceSettings: { + workspaceRuntime: { services: [{ name: "web", command: "pnpm dev" }] }, + }, + }), + expected: "executionWorkspaceSettings.workspaceRuntime.services[0].command", + }, + { + name: "execution workspace config commands", + actual: () => collectExecutionWorkspaceCommandPaths({ + config: { workspaceRuntime: { commands: [{ name: "seed", command: "pnpm seed" }] } }, + }), + expected: "config.workspaceRuntime.commands[0].command", + }, + { + name: "execution workspace metadata jobs", + actual: () => collectExecutionWorkspaceCommandPaths({ + metadata: { + config: { workspaceRuntime: { jobs: [{ name: "build", command: "pnpm build" }] } }, + }, + }), + expected: "metadata.config.workspaceRuntime.jobs[0].command", + }, + ])("detects $name", ({ actual, expected }) => { + expect(actual()).toContain(expected); + }); + + it("ignores descriptive runtime entries without a command field", () => { + expect(collectProjectExecutionWorkspaceCommandPaths({ + workspaceRuntime: { + commands: [{ name: "seed" }], + services: [{ name: "web", port: 3100 }], + jobs: [null, "build"], + }, + })).toEqual([]); + }); +}); diff --git a/server/src/__tests__/workspace-runtime-routes-authz.test.ts b/server/src/__tests__/workspace-runtime-routes-authz.test.ts index 64bfce7f9c..cd7efe6ea4 100644 --- a/server/src/__tests__/workspace-runtime-routes-authz.test.ts +++ b/server/src/__tests__/workspace-runtime-routes-authz.test.ts @@ -343,6 +343,66 @@ describe.sequential("workspace runtime service route authorization", () => { expect(mockAssertCanManageProjectWorkspaceRuntimeServices).toHaveBeenCalled(); }, 15000); + it.each(["skill_test", "task_bridge"])( + "rejects %s keys at the central runtime-manage gate for project and execution workspaces", + async (kind) => { + mockAccessService.decide.mockImplementation(async (input: { action?: string }) => ({ + allowed: input.action !== "runtime:manage", + action: input.action, + reason: input.action === "runtime:manage" ? "deny_key_scope" : "allow_test", + explanation: input.action === "runtime:manage" + ? "Restricted keys cannot manage workspace runtimes." + : "Allowed by test mock.", + })); + mockProjectService.getById.mockResolvedValue(buildProject({ + id: projectId, + workspaces: [{ + id: workspaceId, + companyId: "company-1", + projectId, + runtimeConfig: { + workspaceRuntime: { services: [{ name: "web", command: "pnpm dev" }] }, + }, + }], + })); + mockExecutionWorkspaceService.getById.mockResolvedValue(buildExecutionWorkspace({ + id: executionWorkspaceId, + config: { + workspaceRuntime: { services: [{ name: "web", command: "pnpm dev" }] }, + }, + })); + const actor = { + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + keyScope: kind === "skill_test" + ? { kind, issueId: "issue-1" } + : { kind, parentIssueId: "issue-1" }, + }; + + const projectRes = await request(await createProjectApp(actor)) + .post(`/api/projects/${projectId}/workspaces/${workspaceId}/runtime-services/start`) + .send({}); + expect(projectRes.status, JSON.stringify(projectRes.body)).toBe(403); + expect(projectRes.body.error).toContain("authorization boundary"); + + const executionRes = await request(await createExecutionWorkspaceApp(actor)) + .post(`/api/execution-workspaces/${executionWorkspaceId}/runtime-services/start`) + .send({}); + expect(executionRes.status, JSON.stringify(executionRes.body)).toBe(403); + expect(executionRes.body.error).toContain("authorization boundary"); + + expect(mockAssertCanManageProjectWorkspaceRuntimeServices).not.toHaveBeenCalled(); + expect(mockAssertCanManageExecutionWorkspaceRuntimeServices).not.toHaveBeenCalled(); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "runtime:manage", + resource: { type: "company", companyId: "company-1" }, + })); + }, + ); + it("blocks shared-project stop/restart requests from agents", async () => { mockProjectService.getById.mockResolvedValue(buildProject({ id: projectId, @@ -420,6 +480,45 @@ describe.sequential("workspace runtime service route authorization", () => { expect(mockProjectService.create).not.toHaveBeenCalled(); }); + it("rejects agent callers that persist workspace-runtime service 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 projectRes = await request(app) + .post("/api/companies/company-1/projects") + .send({ + name: "Exploit", + executionWorkspacePolicy: { + enabled: true, + workspaceRuntime: { + services: [{ name: "web", command: "touch /tmp/paperclip-rce" }], + }, + }, + }); + expect(projectRes.status).toBe(403); + expect(projectRes.body.error).toContain("executionWorkspacePolicy.workspaceRuntime.services[0].command"); + + const workspaceRes = await request(app) + .patch(`/api/projects/${projectId}/workspaces/${workspaceId}`) + .send({ + runtimeConfig: { + workspaceRuntime: { + jobs: [{ name: "build", command: "touch /tmp/paperclip-rce" }], + }, + }, + }); + expect(workspaceRes.status).toBe(403); + expect(workspaceRes.body.error).toContain("runtimeConfig.workspaceRuntime.jobs[0].command"); + expect(mockProjectService.create).not.toHaveBeenCalled(); + expect(mockProjectService.updateWorkspace).not.toHaveBeenCalled(); + }); + it("rejects agent callers that update project workspace cleanup commands", async () => { mockProjectService.getById.mockResolvedValue(buildProject()); const app = await createProjectApp({ diff --git a/server/src/adapters/http/execute.test.ts b/server/src/adapters/http/execute.test.ts index 5dbbb12bd0..7d6cd79874 100644 --- a/server/src/adapters/http/execute.test.ts +++ b/server/src/adapters/http/execute.test.ts @@ -2,14 +2,20 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { CONNECTION_INTENT_AGENT_GUIDANCE } from "@paperclipai/shared"; import { execute } from "./execute.js"; +const guardedFetchMock = vi.hoisted(() => vi.fn()); + +vi.mock("./remote-fetch.js", () => ({ + guardedHttpAdapterFetch: guardedFetchMock, +})); + afterEach(() => { - vi.unstubAllGlobals(); + guardedFetchMock.mockReset(); }); describe("http adapter execute", () => { it("delivers the complete runtime connection descriptor and shared guidance", async () => { const onDispatch = vi.fn(); - const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + guardedFetchMock.mockImplementation(async (_url: string, init?: RequestInit) => { expect(onDispatch).toHaveBeenCalledOnce(); const body = JSON.parse(String(init?.body)) as Record; expect(body.paperclipRuntimeTools).toEqual({ @@ -26,7 +32,6 @@ describe("http adapter execute", () => { }); return new Response(null, { status: 204 }); }); - vi.stubGlobal("fetch", fetchMock); await execute({ runId: "run-1", @@ -61,18 +66,17 @@ describe("http adapter execute", () => { onDispatch, }); - expect(fetchMock).toHaveBeenCalledOnce(); + expect(guardedFetchMock).toHaveBeenCalledOnce(); expect(onDispatch).toHaveBeenCalledOnce(); }); it("reports configured request timeout as timed_out", async () => { - vi.stubGlobal( - "fetch", - vi.fn((_url: string, init?: RequestInit) => new Promise((_resolve, reject) => { + guardedFetchMock.mockImplementation( + (_url: string, init?: RequestInit) => new Promise((_resolve, reject) => { init?.signal?.addEventListener("abort", () => { reject(new DOMException("Aborted", "AbortError")); }); - })), + }), ); const result = await execute({ diff --git a/server/src/adapters/http/execute.ts b/server/src/adapters/http/execute.ts index 6bfcc6183f..272cd61e9e 100644 --- a/server/src/adapters/http/execute.ts +++ b/server/src/adapters/http/execute.ts @@ -1,5 +1,6 @@ import type { AdapterExecutionContext, AdapterExecutionResult } from "../types.js"; import { asString, asNumber, parseObject } from "../utils.js"; +import { guardedHttpAdapterFetch } from "./remote-fetch.js"; export async function execute(ctx: AdapterExecutionContext): Promise { const { config, runId, agent, context } = ctx; @@ -26,7 +27,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise { + return new Set( + raw + .split(",") + .map((entry) => normalizeAllowlistedOrigin(entry.trim())) + .filter((entry): entry is string => entry !== null), + ); +} + +type HttpAdapterFetchOptions = Omit< + GuardedRemoteHttpFetchOptions, + "allowPrivateNetwork" | "error" +> & { + privateEndpointAllowlist?: ReadonlySet; +}; + +/** + * Guard every HTTP-adapter request at the actual socket boundary. Public + * endpoints are allowed by default. An exact operator-configured origin can + * opt into private networking, while the shared guard continues to reject + * link-local metadata targets and pins DNS results to prevent rebinding. + */ +export async function guardedHttpAdapterFetch( + url: string | URL, + init: RequestInit, + options: HttpAdapterFetchOptions = {}, +): Promise { + const endpoint = parseRemoteHttpEndpoint(url.toString(), endpointError); + const allowlist = options.privateEndpointAllowlist ?? httpAdapterPrivateEndpointAllowlist(); + return guardedRemoteHttpFetch(endpoint, init, { + ...options, + allowPrivateNetwork: allowlist.has(endpoint.origin.toLowerCase()), + error: endpointError, + }); +} diff --git a/server/src/adapters/http/test.ts b/server/src/adapters/http/test.ts index a1a8fd3440..f9ef5fc078 100644 --- a/server/src/adapters/http/test.ts +++ b/server/src/adapters/http/test.ts @@ -4,6 +4,7 @@ import type { AdapterEnvironmentTestResult, } from "../types.js"; import { asString, parseObject } from "../utils.js"; +import { guardedHttpAdapterFetch } from "./remote-fetch.js"; function summarizeStatus(checks: AdapterEnvironmentCheck[]): AdapterEnvironmentTestResult["status"] { if (checks.some((check) => check.level === "error")) return "fail"; @@ -77,7 +78,7 @@ export async function testEnvironment( const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 3000); try { - const response = await fetch(url, { + const response = await guardedHttpAdapterFetch(url, { method: "HEAD", signal: controller.signal, }); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index e2e4b84d59..1125cbf4f7 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -512,9 +512,62 @@ export function cloudActorHeaderSourceFromHeaders( }; } +/** + * postgres.js codes for a connection the server side closed out from under + * an in-flight query — a pooled Postgres endpoint recycling or suspending + * (observed 2026-09-03 with a managed pooler closing the socket mid-INSERT). + * The driver reconnects transparently on the next query; only the statement + * that was on the wire is lost. + */ +const transientDbConnectionCodes = new Set([ + "CONNECTION_CLOSED", + "CONNECTION_ENDED", + "CONNECTION_DESTROYED", +]); + +/** + * True when the error chain (drizzle wraps the driver error as `cause`) + * carries a postgres.js closed-connection code. Exported for tests. + */ +export function isTransientDbConnectionError(error: unknown): boolean { + for (let current: unknown = error; current instanceof Error; current = current.cause) { + const code = (current as { code?: unknown }).code; + if (typeof code === "string" && transientDbConnectionCodes.has(code)) return true; + } + return false; +} + +/** + * Runs `run` and retries it exactly once when it fails on a transient + * closed-connection error. Callers must pass an idempotent operation. + * Exported for tests. + */ +export async function retryOnTransientDbConnectionError(run: () => Promise): Promise { + try { + return await run(); + } catch (error) { + if (!isTransientDbConnectionError(error)) throw error; + return run(); + } +} + +/** + * Trusted-header actor resolution with a single transient-connection retry. + * The tenant sync inside is idempotent end to end — every write is an + * upsert/on-conflict/delete and the write debounce records only after the + * whole sync succeeds — so replaying it after a dropped connection is safe, + * and turns a golden-path authentication 500 into a served request. + */ export async function resolveCloudTenantActor( db: Db, req: CloudActorHeaderSource, +): Promise { + return retryOnTransientDbConnectionError(() => resolveCloudTenantActorOnce(db, req)); +} + +async function resolveCloudTenantActorOnce( + db: Db, + req: CloudActorHeaderSource, ): Promise { const expectedToken = process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN?.trim(); if (!expectedToken) return null; diff --git a/server/src/middleware/redact-sensitive.ts b/server/src/middleware/redact-sensitive.ts index 84ed453ac7..3af3d4ee9d 100644 --- a/server/src/middleware/redact-sensitive.ts +++ b/server/src/middleware/redact-sensitive.ts @@ -21,6 +21,11 @@ const SENSITIVE_KEYS = new Set([ "password_confirm", "confirmpassword", "confirm_password", + // Secret creation/update bodies use a generic `value` field. Failure logs + // must prefer losing that diagnostic value over persisting credential + // material. `token` is likewise ambiguous but frequently credential-bearing. + "value", + "token", "secret", "client_secret", "clientsecret", diff --git a/server/src/redaction.ts b/server/src/redaction.ts index fe381887ea..e9a29c5531 100644 --- a/server/src/redaction.ts +++ b/server/src/redaction.ts @@ -941,6 +941,35 @@ export function redactEventPayload( return sanitized; } +function redactAgentEnvBinding(value: unknown): unknown { + if (isSecretRefBinding(value) || isUserSecretRefBinding(value)) { + return sanitizeValue(value); + } + if (typeof value === "string" || isPlainBinding(value)) { + return { type: "plain", value: REDACTED_EVENT_VALUE }; + } + if (value === null || value === undefined) return value; + return REDACTED_EVENT_VALUE; +} + +export function redactAgentAdapterConfig( + adapterConfig: Record, +): Record { + if (!isPlainObject(adapterConfig)) return adapterConfig; + if (!isPlainObject(adapterConfig.env)) return redactEventPayload(adapterConfig) ?? {}; + + // Redact `env` here and sanitize the remaining keys separately, so bindings + // are never processed twice. `redactAgentEnvBinding` is authoritative for + // `env`; keeping it out of `sanitizeRecord` means a future change there + // cannot alter entries this function has already redacted. + const { env, ...rest } = adapterConfig; + const redactedEnv = Object.fromEntries( + Object.entries(env).map(([key, value]) => [key, redactAgentEnvBinding(value)]), + ); + + return { ...(redactEventPayload(rest) ?? {}), env: redactedEnv }; +} + export function redactSensitiveText(input: string): string { if (!maybeContainsSecretText(input)) return input; return redactCommandText( diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index d13477d0a9..5ee35842b9 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -47,6 +47,7 @@ import { } from "@paperclipai/adapter-utils/server-utils"; import { trackAgentCreated } from "@paperclipai/shared/telemetry"; import { validate } from "../middleware/validate.js"; +import { agentInstructionsBundleMode } from "../services/agent-instructions.js"; import { agentService, agentInstructionsService, @@ -105,7 +106,11 @@ import { refreshAdapterModels, requireServerAdapter, } from "../adapters/index.js"; -import { redactEventPayload } from "../redaction.js"; +import { + REDACTED_EVENT_VALUE, + redactAgentAdapterConfig, + redactEventPayload, +} from "../redaction.js"; import { redactCurrentUserValue } from "../log-redaction.js"; import { HarnessRuntimeRequestResolutionError, @@ -1008,6 +1013,17 @@ export function agentRoutes( return false; } + async function assertRunTelemetryReadAllowed(req: Request, res: Response, companyId: string) { + const decision = await access.decide({ + actor: req.actor, + action: "company_scope:read", + resource: { type: "company", companyId }, + }); + if (decision.allowed) return true; + res.status(403).json({ error: "Run telemetry is outside this actor's authorization boundary" }); + return false; + } + async function filterAgentsForActor>( req: Request, rows: T[], @@ -1483,8 +1499,12 @@ export function agentRoutes( buildAgentAccessState(agent), ]); + const baseAgent = redactAgentRowForResponse( + options?.restricted ? redactForRestrictedAgentView(agent) : agent, + ); + return { - ...(options?.restricted ? redactForRestrictedAgentView(agent) : agent), + ...baseAgent, chainOfCommand, access: accessState, }; @@ -2548,6 +2568,15 @@ export function agentRoutes( ); } + function assertExternalInstructionsAdmin( + req: Request, + agent: Parameters[0], + ) { + if (agentInstructionsBundleMode(agent) === "external") { + assertInstanceAdmin(req); + } + } + function adapterConfigTouchesInstructionsConfig(adapterConfig: Record) { return KNOWN_INSTRUCTIONS_BUNDLE_KEYS.some((key) => adapterConfig[key] !== undefined); } @@ -2773,6 +2802,21 @@ export function agentRoutes( }; } + // Single presenter for every response that emits a raw agent row. Restricted + // views blank the config wholesale for authorization reasons; this runs for + // config-reading (board) callers too, so plaintext `adapterConfig.env` values + // never leave the API regardless of actor scope. + function redactAgentRowForResponse( + agent: T, + ): T { + if (!agent || typeof agent !== "object") return agent; + if (!agent.adapterConfig || typeof agent.adapterConfig !== "object") return agent; + return { + ...agent, + adapterConfig: redactAgentAdapterConfig(agent.adapterConfig as Record), + }; + } + function redactAgentConfiguration(agent: Awaited>) { if (!agent) return null; return { @@ -2784,19 +2828,41 @@ export function agentRoutes( status: agent.status, reportsTo: agent.reportsTo, adapterType: agent.adapterType, - adapterConfig: redactEventPayload(agent.adapterConfig), + adapterConfig: redactAgentAdapterConfig(agent.adapterConfig), runtimeConfig: redactEventPayload(agent.runtimeConfig), permissions: agent.permissions, updatedAt: agent.updatedAt, }; } + function restoreRedactedAgentEnv( + requestedConfig: Record, + existingConfig: Record, + ): Record { + const requestedEnv = asRecord(requestedConfig.env); + const existingEnv = asRecord(existingConfig.env); + if (!requestedEnv || !existingEnv) return requestedConfig; + + const restoredEnv = { ...requestedEnv }; + for (const [key, value] of Object.entries(requestedEnv)) { + const binding = asRecord(value); + if ( + binding?.type === "plain" + && binding.value === REDACTED_EVENT_VALUE + && Object.prototype.hasOwnProperty.call(existingEnv, key) + ) { + restoredEnv[key] = existingEnv[key]; + } + } + return { ...requestedConfig, env: restoredEnv }; + } + function redactRevisionSnapshot(snapshot: unknown): Record { if (!snapshot || typeof snapshot !== "object" || Array.isArray(snapshot)) return {}; const record = snapshot as Record; return { ...record, - adapterConfig: redactEventPayload( + adapterConfig: redactAgentAdapterConfig( typeof record.adapterConfig === "object" && record.adapterConfig !== null ? (record.adapterConfig as Record) : {}, @@ -3466,7 +3532,7 @@ export function agentRoutes( const result = await filterAgentsForActor(req, await svc.list(companyId)); const canReadConfigs = await actorCanReadConfigurationsForCompany(req, companyId); if (canReadConfigs) { - res.json(result); + res.json(result.map((agent) => redactAgentRowForResponse(agent))); return; } res.json(result.map((agent) => redactForRestrictedAgentView(agent))); @@ -3584,16 +3650,10 @@ export function agentRoutes( res.status(404).json({ error: "Agent not found" }); return; } - const trustPreset = await resolveAgentSelfTrustPreset(req, agent); - if (trustPreset.kind === "denied") { - res.status(403).json({ error: trustPreset.detail }); - return; - } - if (trustPreset.kind === "low_trust_review") { - res.json(buildLowTrustSelfView(agent)); - return; - } - if (req.actor.keyScope?.kind === "task_bridge") { + if ( + req.actor.keyScope?.kind === "task_bridge" + || req.actor.keyScope?.kind === "skill_test" + ) { res.json({ id: agent.id, companyId: agent.companyId, @@ -3605,6 +3665,15 @@ export function agentRoutes( }); return; } + const trustPreset = await resolveAgentSelfTrustPreset(req, agent); + if (trustPreset.kind === "denied") { + res.status(403).json({ error: trustPreset.detail }); + return; + } + if (trustPreset.kind === "low_trust_review") { + res.json(buildLowTrustSelfView(agent)); + return; + } res.json(await buildAgentDetail(agent)); }); @@ -3772,6 +3841,11 @@ export function agentRoutes( await assertSelectableAdapterType(rollbackAdapterType); } const rollbackAdapterConfig = asRecord(rollbackConfig.adapterConfig) ?? {}; + assertExternalInstructionsAdmin(req, existing); + assertExternalInstructionsAdmin(req, { + ...existing, + adapterConfig: rollbackAdapterConfig, + }); if ( rollbackAdapterType !== existing.adapterType || rollbackAdapterType === "paperclip_runner" @@ -3806,7 +3880,7 @@ export function agentRoutes( details: { revisionId }, }); - res.json(updated); + res.json(redactAgentRowForResponse(updated)); }); router.get("/agents/:id/runtime-state", async (req, res) => { @@ -3903,6 +3977,12 @@ export function agentRoutes( rawHireAdapterConfig, ), ); + assertExternalInstructionsAdmin(req, { + id: hiredAgentId, + companyId, + name: hireInput.name, + adapterConfig: requestedAdapterConfig, + }); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, hireInput.adapterType, @@ -4122,6 +4202,12 @@ export function agentRoutes( rawCreateAdapterConfig, ), ); + assertExternalInstructionsAdmin(req, { + id: agentId, + companyId, + name: createInput.name, + adapterConfig: requestedAdapterConfig, + }); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, createInput.adapterType, @@ -4210,7 +4296,7 @@ export function agentRoutes( ); } - res.status(201).json(agent); + res.status(201).json(redactAgentRowForResponse(agent)); }); router.patch("/agents/:id/permissions", validate(updateAgentPermissionsSchema), async (req, res) => { @@ -4282,6 +4368,7 @@ export function agentRoutes( if (!existing) return; await assertCanManageInstructionsPath(req, existing); + assertExternalInstructionsAdmin(req, existing); const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {}; const explicitKey = asNonEmptyString(req.body.adapterConfigKey); @@ -4302,6 +4389,7 @@ export function agentRoutes( } const syncedAdapterConfig = syncInstructionsBundleConfigFromFilePath(existing, nextAdapterConfig); + assertExternalInstructionsAdmin(req, { ...existing, adapterConfig: syncedAdapterConfig }); const normalizedAdapterConfig = await secretsSvc.normalizeAdapterConfigForPersistence( existing.companyId, syncedAdapterConfig, @@ -4357,6 +4445,7 @@ export function agentRoutes( const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; await assertCanReadAgent(req, existing); + assertExternalInstructionsAdmin(req, existing); res.json(await instructions.getBundle(existing)); }); @@ -4365,6 +4454,8 @@ export function agentRoutes( const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; await assertCanManageInstructionsPath(req, existing); + assertExternalInstructionsAdmin(req, existing); + if (req.body.mode === "external") assertInstanceAdmin(req); const actor = getActorInfo(req); const { bundle, adapterConfig } = await instructions.updateBundle(existing, req.body); @@ -4411,6 +4502,7 @@ export function agentRoutes( const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; await assertCanReadAgent(req, existing); + assertExternalInstructionsAdmin(req, existing); const relativePath = typeof req.query.path === "string" ? req.query.path : ""; if (!relativePath.trim()) { @@ -4426,6 +4518,7 @@ export function agentRoutes( const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; await assertCanManageInstructionsPath(req, existing); + assertExternalInstructionsAdmin(req, existing); const actor = getActorInfo(req); const result = await instructions.writeFile(existing, req.body.path, req.body.content, { @@ -4473,6 +4566,7 @@ export function agentRoutes( const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; await assertCanManageInstructionsPath(req, existing); + assertExternalInstructionsAdmin(req, existing); const relativePath = typeof req.query.path === "string" ? req.query.path : ""; if (!relativePath.trim()) { @@ -4560,6 +4654,7 @@ export function agentRoutes( hasOwn(patchData, "adapterType") || hasOwn(patchData, "adapterConfig"); if (touchesAdapterConfiguration) { + assertExternalInstructionsAdmin(req, existing); const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {}; const changingAdapterType = typeof patchData.adapterType === "string" && patchData.adapterType !== existing.adapterType; @@ -4575,9 +4670,11 @@ export function agentRoutes( ) { await assertCanManageInstructionsPath(req, existing); } - let rawEffectiveAdapterConfig = requestedAdapterConfig ?? existingAdapterConfig; + let rawEffectiveAdapterConfig = requestedAdapterConfig + ? restoreRedactedAgentEnv(requestedAdapterConfig, existingAdapterConfig) + : existingAdapterConfig; if (requestedAdapterConfig && !changingAdapterType && !replaceAdapterConfig) { - rawEffectiveAdapterConfig = { ...existingAdapterConfig, ...requestedAdapterConfig }; + rawEffectiveAdapterConfig = { ...existingAdapterConfig, ...rawEffectiveAdapterConfig }; } if (changingAdapterType) { // Preserve adapter-agnostic keys (env, cwd, etc.) from the existing config @@ -4631,6 +4728,10 @@ export function agentRoutes( adapterConfig: effectiveAdapterConfig, }); patchData.adapterConfig = syncInstructionsBundleConfigFromFilePath(existing, normalizedEffectiveAdapterConfig); + assertExternalInstructionsAdmin(req, { + ...existing, + adapterConfig: patchData.adapterConfig, + }); } if (requestedRuntimeConfig) patchData.runtimeConfig = requestedRuntimeConfig; if (touchesAdapterConfiguration || Object.prototype.hasOwnProperty.call(patchData, "defaultEnvironmentId")) { @@ -4688,7 +4789,7 @@ export function agentRoutes( details: summarizeAgentUpdateDetails(patchData), }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.post("/agents/:id/pause", async (req, res) => { @@ -4714,7 +4815,7 @@ export function agentRoutes( entityId: agent.id, }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.post("/agents/:id/resume", async (req, res) => { @@ -4749,7 +4850,7 @@ export function agentRoutes( entityId: agent.id, }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.post("/agents/:id/clear-error", async (req, res) => { @@ -4781,7 +4882,7 @@ export function agentRoutes( entityId: agent.id, }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.post("/agents/:id/approve", async (req, res) => { @@ -4835,7 +4936,7 @@ export function agentRoutes( details: { source: "agent_detail", approvalId: openApproval?.id ?? null }, }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.post("/agents/:id/terminate", async (req, res) => { @@ -4905,7 +5006,7 @@ export function agentRoutes( }, }); - res.json(agent); + res.json(redactAgentRowForResponse(agent)); }); router.delete("/agents/:id", async (req, res) => { @@ -5708,6 +5809,7 @@ export function agentRoutes( router.get("/companies/:companyId/heartbeat-runs", async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + if (!(await assertRunTelemetryReadAllowed(req, res, companyId))) return; const agentId = req.query.agentId as string | undefined; const limitParam = req.query.limit as string | undefined; const limit = limitParam ? Math.max(1, Math.min(1000, parseInt(limitParam, 10) || 200)) : undefined; @@ -5748,6 +5850,7 @@ export function agentRoutes( router.get("/companies/:companyId/live-runs", async (req, res) => { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + if (!(await assertRunTelemetryReadAllowed(req, res, companyId))) return; // `minCount` is a padding floor for callers that want a minimum number of // recent runs to render (e.g. dashboard cards). It must default to 0 so @@ -5835,6 +5938,7 @@ export function agentRoutes( const runId = req.params.runId as string; const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); if (!run) return; + if (!(await assertRunTelemetryReadAllowed(req, res, run.companyId))) return; const retryExhaustedReason = await heartbeat.getRetryExhaustedReason(runId); const decoratedRun = heartbeat.decorateActiveRunStatus(run); res.json(await runRedactions.redactForRun( @@ -6308,6 +6412,7 @@ export function agentRoutes( const runId = req.params.runId as string; const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); if (!run) return; + if (!(await assertRunTelemetryReadAllowed(req, res, run.companyId))) return; const afterSeq = Number(req.query.afterSeq ?? 0); const limit = Number(req.query.limit ?? 200); @@ -6326,6 +6431,7 @@ export function agentRoutes( const runId = req.params.runId as string; const run = await getAccessibleResource(req, res, heartbeat.getRunLogAccess(runId), "Heartbeat run not found"); if (!run) return; + if (!(await assertRunTelemetryReadAllowed(req, res, run.companyId))) return; const offset = Number(req.query.offset ?? 0); const limitBytes = readRunLogLimitBytes(req.query.limitBytes); @@ -6342,6 +6448,7 @@ export function agentRoutes( const runId = req.params.runId as string; const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); if (!run) return; + if (!(await assertRunTelemetryReadAllowed(req, res, run.companyId))) return; const context = asRecord(run.contextSnapshot); const executionWorkspaceId = asNonEmptyString(context?.executionWorkspaceId); @@ -6353,6 +6460,7 @@ export function agentRoutes( const operationId = req.params.operationId as string; const operation = await getAccessibleResource(req, res, workspaceOperations.getById(operationId), "Workspace operation not found"); if (!operation) return; + if (!(await assertRunTelemetryReadAllowed(req, res, operation.companyId))) return; const offset = Number(req.query.offset ?? 0); const limitBytes = readRunLogLimitBytes(req.query.limitBytes); diff --git a/server/src/routes/assets.ts b/server/src/routes/assets.ts index 8419161a4d..3f9d4ee0c3 100644 --- a/server/src/routes/assets.ts +++ b/server/src/routes/assets.ts @@ -9,6 +9,7 @@ import { assetService, logActivity } from "../services/index.js"; import { formatAttachmentSize, isAllowedContentType, + isInlineAttachmentContentType, MAX_ATTACHMENT_BYTES, } from "../attachment-types.js"; import { assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; @@ -329,15 +330,21 @@ export function assetRoutes(db: Db, storage: StorageService) { const object = await storage.getObject(asset.companyId, asset.objectKey); const responseContentType = asset.contentType || object.contentType || "application/octet-stream"; + const mediaType = responseContentType.split(";", 1)[0]?.trim().toLowerCase() ?? ""; + const inlineSafe = mediaType !== SVG_CONTENT_TYPE + && isInlineAttachmentContentType(mediaType); res.setHeader("Content-Type", responseContentType); res.setHeader("Content-Length", String(asset.byteSize || object.contentLength || 0)); res.setHeader("Cache-Control", "private, max-age=60"); res.setHeader("X-Content-Type-Options", "nosniff"); - if (responseContentType === SVG_CONTENT_TYPE) { - res.setHeader("Content-Security-Policy", "sandbox; default-src 'none'; img-src 'self' data:; style-src 'unsafe-inline'"); + if (!inlineSafe) { + res.setHeader("Content-Security-Policy", "sandbox; default-src 'none'"); } const filename = asset.originalFilename ?? "asset"; - res.setHeader("Content-Disposition", `inline; filename=\"${filename.replaceAll("\"", "")}\"`); + const disposition = inlineSafe + ? "inline" + : "attachment"; + res.setHeader("Content-Disposition", `${disposition}; filename=\"${filename.replaceAll("\"", "")}\"`); object.stream.on("error", (err) => { next(err); diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index a6152c303a..0c7306a15c 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -49,6 +49,8 @@ import { writeImportTransferPart, } from "../services/company-import-transfers.js"; import { companyTransferRunService } from "../services/company-transfer-runs.js"; +import { agentInstructionsBundleMode } from "../services/agent-instructions.js"; +import { resolvePortableExportAgentSelection } from "../services/company-portability-agent-selection.js"; import { accessService, agentService, @@ -315,6 +317,25 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan return Math.floor(parsed); } + async function assertExternalInstructionExportAllowed( + req: Request, + companyId: string, + input: { include?: { agents?: boolean }; agents?: string[] }, + ) { + const instanceAdmin = req.actor.type === "board" + && (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin === true); + const includesAgents = input.agents && input.agents.length > 0 + ? true + : input.include?.agents ?? true; + if (!includesAgents) return instanceAdmin; + const companyAgents = await agents.list(companyId, { includeTerminated: true }); + const selection = resolvePortableExportAgentSelection(companyAgents, input.agents, includesAgents); + if (selection.agents.some((agent) => agentInstructionsBundleMode(agent) === "external")) { + assertInstanceAdmin(req); + } + return instanceAdmin; + } + const timelineQuerySchema = z.object({ from: z.string().optional(), to: z.string().optional(), @@ -495,7 +516,8 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan const companyId = req.params.companyId as string; await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); const body = companyPortabilityExportSchema.parse(req.body); - const result = await portability.exportBundle(companyId, body); + const allowExternalInstructions = await assertExternalInstructionExportAllowed(req, companyId, body); + const result = await portability.exportBundle(companyId, body, { allowExternalInstructions }); res.json(result); }); @@ -1089,7 +1111,8 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan const companyId = req.params.companyId as string; await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); const body = companyPortabilityExportSchema.parse(req.body); - const preview = await portability.previewExport(companyId, body); + const allowExternalInstructions = await assertExternalInstructionExportAllowed(req, companyId, body); + const preview = await portability.previewExport(companyId, body, { allowExternalInstructions }); res.json(preview); }); @@ -1097,7 +1120,8 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan const companyId = req.params.companyId as string; await assertSameCompanyCeoAgentOrBoard(req, companyId, "company exports"); const body = companyPortabilityExportSchema.parse(req.body); - const result = await portability.exportBundle(companyId, body); + const allowExternalInstructions = await assertExternalInstructionExportAllowed(req, companyId, body); + const result = await portability.exportBundle(companyId, body, { allowExternalInstructions }); res.json(result); }); diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index 6f58cad51c..034386c549 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -131,6 +131,17 @@ export function projectRoutes(db: Db) { return false; } + async function assertRuntimeManageAllowed(req: Request, res: Response, companyId: string) { + const decision = await access.decide({ + actor: req.actor, + action: "runtime:manage", + resource: { type: "company", companyId }, + }); + if (decision.allowed) return true; + res.status(403).json({ error: "Runtime service control is outside this actor's authorization boundary" }); + return false; + } + async function filterProjectsForActor(req: Request, rows: T[]) { const decisions = await Promise.all(rows.map((project) => access.decide({ @@ -403,6 +414,7 @@ export function projectRoutes(db: Db) { res.status(404).json({ error: "Project workspace not found" }); return; } + if (!(await assertRuntimeManageAllowed(req, res, project.companyId))) return; const isSharedWorkspace = Boolean(workspace.sharedWorkspaceKey); if ( diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index e933b135e8..3d3ca863f2 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -116,6 +116,16 @@ export function secretRoutes(db: Db, deps: SecretRoutesDeps = {}) { const runRedactions = createRunSecretRedactionRegistry(db); const defaultProvider = getConfiguredSecretProvider(); + async function assertSecretCatalogReadAllowed(req: Parameters[0], companyId: string) { + const decision = await access.decide({ + actor: req.actor, + action: "secrets:read", + resource: { type: "company", companyId }, + }); + if (decision.allowed) return; + throw forbidden(decision.explanation, authorizationDeniedDetails(decision)); + } + function agentSecretContext(req: Parameters[0]) { if (req.actor.type !== "agent" || !req.actor.agentId || !req.actor.companyId || !req.actor.runId) { throw forbidden("Run-bound agent authentication required"); @@ -576,6 +586,7 @@ export function secretRoutes(db: Db, deps: SecretRoutesDeps = {}) { assertBoardOrAgent(req); const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); + await assertSecretCatalogReadAllowed(req, companyId); const secrets = await svc.list(companyId); res.json(secrets.map(({ id, name, key, status }) => ({ id, name, key, status }))); }); diff --git a/server/src/routes/workspace-command-authz.ts b/server/src/routes/workspace-command-authz.ts index 56e6b07ed7..7ca79a1c01 100644 --- a/server/src/routes/workspace-command-authz.ts +++ b/server/src/routes/workspace-command-authz.ts @@ -28,6 +28,21 @@ function collectWorkspaceStrategyCommandPaths(raw: unknown, prefix: string): str return paths; } +function collectWorkspaceRuntimeCommandPaths(raw: unknown, prefix: string): string[] { + if (!isRecord(raw)) return []; + const paths: string[] = []; + for (const collectionKey of ["commands", "services", "jobs"] as const) { + const entries = raw[collectionKey]; + if (!Array.isArray(entries)) continue; + entries.forEach((entry, index) => { + if (isRecord(entry) && hasOwn(entry, "command")) { + paths.push(`${prefixPath(prefix, collectionKey)}[${index}].command`); + } + }); + } + return paths; +} + function collectExecutionWorkspaceConfigCommandPaths(raw: unknown, prefix: string): string[] { if (!isRecord(raw)) return []; const paths: string[] = []; @@ -43,6 +58,12 @@ function collectExecutionWorkspaceConfigCommandPaths(raw: unknown, prefix: strin if (hasOwn(raw, "cleanupCommand")) { paths.push(prefixPath(prefix, "cleanupCommand")); } + paths.push( + ...collectWorkspaceRuntimeCommandPaths( + raw.workspaceRuntime, + prefixPath(prefix, "workspaceRuntime"), + ), + ); return paths; } @@ -66,10 +87,16 @@ export function collectAgentAdapterWorkspaceCommandPaths( export function collectProjectExecutionWorkspaceCommandPaths(policy: unknown): string[] { if (!isRecord(policy)) return []; - return collectWorkspaceStrategyCommandPaths( - policy.workspaceStrategy, - "executionWorkspacePolicy.workspaceStrategy", - ); + return [ + ...collectWorkspaceStrategyCommandPaths( + policy.workspaceStrategy, + "executionWorkspacePolicy.workspaceStrategy", + ), + ...collectWorkspaceRuntimeCommandPaths( + policy.workspaceRuntime, + "executionWorkspacePolicy.workspaceRuntime", + ), + ]; } export function collectProjectWorkspaceCommandPaths( @@ -77,9 +104,18 @@ export function collectProjectWorkspaceCommandPaths( prefix = "", ): string[] { if (!isRecord(workspacePatch)) return []; - return hasOwn(workspacePatch, "cleanupCommand") + const paths = hasOwn(workspacePatch, "cleanupCommand") ? [prefixPath(prefix, "cleanupCommand")] : []; + if (isRecord(workspacePatch.runtimeConfig)) { + paths.push( + ...collectWorkspaceRuntimeCommandPaths( + workspacePatch.runtimeConfig.workspaceRuntime, + prefixPath(prefix, "runtimeConfig.workspaceRuntime"), + ), + ); + } + return paths; } export function collectIssueWorkspaceCommandPaths(input: { @@ -94,6 +130,12 @@ export function collectIssueWorkspaceCommandPaths(input: { "executionWorkspaceSettings.workspaceStrategy", ), ); + paths.push( + ...collectWorkspaceRuntimeCommandPaths( + input.executionWorkspaceSettings.workspaceRuntime, + "executionWorkspaceSettings.workspaceRuntime", + ), + ); } if (isRecord(input.assigneeAdapterOverrides)) { const adapterConfig = input.assigneeAdapterOverrides.adapterConfig; diff --git a/server/src/services/agent-instructions.ts b/server/src/services/agent-instructions.ts index 3246671d35..5045143eb7 100644 --- a/server/src/services/agent-instructions.ts +++ b/server/src/services/agent-instructions.ts @@ -282,6 +282,19 @@ function deriveBundleState(agent: AgentLike): BundleState { }; } +/** Classify the configured bundle without touching the filesystem. */ +export function agentInstructionsBundleMode(agent: AgentLike): BundleMode | null { + const state = deriveBundleState(agent); + if (state.mode === "external") return "external"; + if ( + state.rootPath + && path.resolve(state.rootPath) !== resolveManagedInstructionsRoot(agent) + ) { + return "external"; + } + return state.mode; +} + async function recoverManagedBundleState(agent: AgentLike, state: BundleState): Promise { const managedRootPath = resolveManagedInstructionsRoot(agent); const stat = await statIfExists(managedRootPath); diff --git a/server/src/services/company-portability-agent-selection.ts b/server/src/services/company-portability-agent-selection.ts new file mode 100644 index 0000000000..e258eb81e7 --- /dev/null +++ b/server/src/services/company-portability-agent-selection.ts @@ -0,0 +1,67 @@ +import { normalizeAgentUrlKey } from "@paperclipai/shared"; +import { readBuiltInAgentMarker } from "./built-in-agent-metadata.js"; + +interface ExportAgentCandidate { + id: string; + name: string; + status: string; + metadata: unknown; +} + +export function resolvePortableExportAgentSelection( + allAgentRows: T[], + selectors: string[] | undefined, + includeAgents: boolean, +): { agents: T[]; warnings: string[] } { + const warnings: string[] = []; + const liveAgentRows = allAgentRows.filter((agent) => agent.status !== "terminated"); + const builtInAgentRows = liveAgentRows.filter((agent) => readBuiltInAgentMarker(agent.metadata)); + const portableAgentRows = liveAgentRows.filter((agent) => !readBuiltInAgentMarker(agent.metadata)); + + if (includeAgents) { + const skipped = allAgentRows.length - liveAgentRows.length; + if (skipped > 0) { + warnings.push(`Skipped ${skipped} terminated agent${skipped === 1 ? "" : "s"} from export.`); + } + if (builtInAgentRows.length > 0) { + warnings.push(`Skipped ${builtInAgentRows.length} built-in managed agent${builtInAgentRows.length === 1 ? "" : "s"} from export.`); + } + } + + const agentByReference = new Map(); + const builtInAgentByReference = new Map(); + const addAgentReferences = (map: Map, agent: T) => { + map.set(agent.id, agent); + map.set(agent.name, agent); + const normalizedName = normalizeAgentUrlKey(agent.name); + if (normalizedName) map.set(normalizedName, agent); + }; + for (const agent of portableAgentRows) addAgentReferences(agentByReference, agent); + for (const agent of builtInAgentRows) addAgentReferences(builtInAgentByReference, agent); + + const selectedAgents = new Map(); + for (const selector of selectors ?? []) { + const trimmed = selector.trim(); + if (!trimmed) continue; + const normalized = normalizeAgentUrlKey(trimmed) ?? trimmed; + const match = agentByReference.get(trimmed) ?? agentByReference.get(normalized); + if (!match) { + const builtInMatch = builtInAgentByReference.get(trimmed) ?? builtInAgentByReference.get(normalized); + if (builtInMatch) { + warnings.push(`Agent selector "${selector}" is a built-in managed agent and was skipped.`); + } else { + warnings.push(`Agent selector "${selector}" was not found and was skipped.`); + } + continue; + } + selectedAgents.set(match.id, match); + } + + // Preserve the established compatibility behavior: no effective explicit + // selection falls back to every portable agent when agent export is enabled. + if (includeAgents && selectedAgents.size === 0) { + for (const agent of portableAgentRows) selectedAgents.set(agent.id, agent); + } + + return { agents: Array.from(selectedAgents.values()), warnings }; +} diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index 8007f9e3a0..23ec71f0fe 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -76,7 +76,7 @@ import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js"; import type { StorageService } from "../storage/types.js"; import { accessService } from "./access.js"; import { agentService } from "./agents.js"; -import { agentInstructionsService } from "./agent-instructions.js"; +import { agentInstructionsBundleMode, agentInstructionsService } from "./agent-instructions.js"; import { assetService } from "./assets.js"; import { generateReadme } from "./company-export-readme.js"; import { renderOrgChartPng, type OrgNode } from "../routes/org-chart-svg.js"; @@ -96,7 +96,7 @@ import { readCatalogStringList, readPortableCatalogProvenance, } from "./catalog-provenance.js"; -import { readBuiltInAgentMarker } from "./built-in-agent-metadata.js"; +import { resolvePortableExportAgentSelection } from "./company-portability-agent-selection.js"; import { normalizePortablePath } from "./portable-path.js"; import type { ImportIssueRow, @@ -555,6 +555,9 @@ function buildSkillExportDirMap(skills: CompanySkill[], companyIssuePrefix: stri function isSensitiveEnvKey(key: string) { const normalized = key.trim().toLowerCase(); return ( + normalized === "key" || + normalized.endsWith("_key") || + normalized.endsWith("-key") || normalized === "token" || normalized.endsWith("_token") || normalized.endsWith("-token") || @@ -3834,7 +3837,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { async function exportBundle( companyId: string, input: CompanyPortabilityExport, - options: { preview?: boolean } = {}, + options: { preview?: boolean; allowExternalInstructions?: boolean } = {}, ): Promise { const include = normalizeInclude({ ...input.include, @@ -3877,67 +3880,16 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { ); const allAgentRows = include.agents ? await agents.list(companyId, { includeTerminated: true }) : []; - const liveAgentRows = allAgentRows.filter((agent) => agent.status !== "terminated"); - const builtInAgentRows = liveAgentRows.filter((agent) => readBuiltInAgentMarker(agent.metadata)); - const portableAgentRows = liveAgentRows.filter((agent) => !readBuiltInAgentMarker(agent.metadata)); + const agentSelection = resolvePortableExportAgentSelection(allAgentRows, input.agents, include.agents); const companySkillRowsRaw = include.skills ? await companySkills.listFull(companyId) : []; const managedSkillRows = companySkillRowsRaw.filter((skill) => managedSkillIds.has(skill.id)); const companySkillRows = companySkillRowsRaw.filter((skill) => !managedSkillIds.has(skill.id)); - if (include.agents) { - const skipped = allAgentRows.length - liveAgentRows.length; - if (skipped > 0) { - warnings.push(`Skipped ${skipped} terminated agent${skipped === 1 ? "" : "s"} from export.`); - } - if (builtInAgentRows.length > 0) { - warnings.push(`Skipped ${builtInAgentRows.length} built-in managed agent${builtInAgentRows.length === 1 ? "" : "s"} from export.`); - } - } + warnings.push(...agentSelection.warnings); if (include.skills && managedSkillRows.length > 0) { warnings.push(`Skipped ${managedSkillRows.length} built-in managed skill${managedSkillRows.length === 1 ? "" : "s"} from export.`); } - const agentByReference = new Map(); - const builtInAgentByReference = new Map(); - const addAgentReferences = (map: Map, agent: typeof liveAgentRows[number]) => { - map.set(agent.id, agent); - map.set(agent.name, agent); - const normalizedName = normalizeAgentUrlKey(agent.name); - if (normalizedName) { - map.set(normalizedName, agent); - } - }; - for (const agent of portableAgentRows) { - addAgentReferences(agentByReference, agent); - } - for (const agent of builtInAgentRows) { - addAgentReferences(builtInAgentByReference, agent); - } - - const selectedAgents = new Map(); - for (const selector of input.agents ?? []) { - const trimmed = selector.trim(); - if (!trimmed) continue; - const normalized = normalizeAgentUrlKey(trimmed) ?? trimmed; - const match = agentByReference.get(trimmed) ?? agentByReference.get(normalized); - if (!match) { - const builtInMatch = builtInAgentByReference.get(trimmed) ?? builtInAgentByReference.get(normalized); - if (builtInMatch) { - warnings.push(`Agent selector "${selector}" is a built-in managed agent and was skipped.`); - continue; - } - warnings.push(`Agent selector "${selector}" was not found and was skipped.`); - continue; - } - selectedAgents.set(match.id, match); - } - - if (include.agents && selectedAgents.size === 0) { - for (const agent of portableAgentRows) { - selectedAgents.set(agent.id, agent); - } - } - - const agentRows = Array.from(selectedAgents.values()) + const agentRows = agentSelection.agents .sort((left, right) => left.name.localeCompare(right.name)); const usedSlugs = new Set(); @@ -4118,7 +4070,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { projectSlugById.set(project.id, uniqueSlug(baseSlug, usedProjectSlugs)); } const sidebarOrder = requestedSidebarOrder ?? stripEmptyValues({ - agents: sortAgentsBySidebarOrder(Array.from(selectedAgents.values())) + agents: sortAgentsBySidebarOrder(agentSelection.agents) .map((agent) => idToSlug.get(agent.id)) .filter((slug): slug is string => Boolean(slug)), projects: selectedProjectRows @@ -4227,6 +4179,12 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { } if (include.agents) { + if ( + !options.allowExternalInstructions + && agentRows.some((agent) => agentInstructionsBundleMode(agent) === "external") + ) { + throw forbidden("Instance admin access is required to export external instruction bundles"); + } const agentInstructionsById = new Map( await mapWithConcurrency(agentRows, EXPORT_READ_CONCURRENCY, async (agent) => ( [agent.id, await instructions.exportFiles(agent)] as const @@ -4817,6 +4775,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { async function previewExport( companyId: string, input: CompanyPortabilityExport, + options: { allowExternalInstructions?: boolean } = {}, ): Promise { const previewInput: CompanyPortabilityExport = { ...input, @@ -4831,7 +4790,10 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { if (previewInput.include && previewInput.include.issues === undefined) { previewInput.include.issues = false; } - const exported = await exportBundle(companyId, previewInput, { preview: true }); + const exported = await exportBundle(companyId, previewInput, { + preview: true, + allowExternalInstructions: options.allowExternalInstructions, + }); return { ...exported, fileInventory: Object.keys(exported.files) diff --git a/server/src/services/feedback.ts b/server/src/services/feedback.ts index 7698520eb0..018a82546f 100644 --- a/server/src/services/feedback.ts +++ b/server/src/services/feedback.ts @@ -38,7 +38,7 @@ import { } from "@paperclipai/shared"; import { resolveHomeAwarePath, resolvePaperclipInstanceRoot } from "../home-paths.js"; import { notFound, unprocessable } from "../errors.js"; -import { agentInstructionsService } from "./agent-instructions.js"; +import { agentInstructionsBundleMode, agentInstructionsService } from "./agent-instructions.js"; import { createFeedbackRedactionState, finalizeFeedbackRedactionSummary, @@ -1168,12 +1168,23 @@ async function buildAgentContext( : []; const usage = asRecord(run?.usageJson) ?? {}; + const externalInstructions = agentInstructionsBundleMode({ + id: agent.id, + companyId: agent.companyId, + name: agent.name, + adapterConfig: agent.adapterConfig, + }) === "external"; + if (externalInstructions) { + state.omittedFields.add("bundle.agentContext.runtime.configuredInstructionsFilePath"); + state.omittedFields.add("bundle.agentContext.runtime.configuredInstructionsRootPath"); + state.omittedFields.add("bundle.agentContext.instructions"); + } const runtime = { configuredModel: asString(adapterConfig.model), configuredInstructionsBundleMode: asString(adapterConfig.instructionsBundleMode), configuredInstructionsEntryFile: asString(adapterConfig.instructionsEntryFile), - configuredInstructionsFilePath: asString(adapterConfig.instructionsFilePath), - configuredInstructionsRootPath: asString(adapterConfig.instructionsRootPath), + configuredInstructionsFilePath: externalInstructions ? null : asString(adapterConfig.instructionsFilePath), + configuredInstructionsRootPath: externalInstructions ? null : asString(adapterConfig.instructionsRootPath), heartbeatPolicy: sanitizeFeedbackValue(runtimeConfig.heartbeat ?? null, state, "bundle.agentContext.runtime.heartbeatPolicy", 400), provenanceMode: run ? "source_run" : "vote_time_snapshot", sourceRun: run @@ -1218,12 +1229,14 @@ async function buildAgentContext( : null, }; - const instructionsBundle = await instructionsSvc.getBundle({ - id: agent.id, - companyId: agent.companyId, - name: agent.name, - adapterConfig: agent.adapterConfig, - }).catch(() => null); + const instructionsBundle = externalInstructions + ? null + : await instructionsSvc.getBundle({ + id: agent.id, + companyId: agent.companyId, + name: agent.name, + adapterConfig: agent.adapterConfig, + }).catch(() => null); let entryDigest: string | null = null; let entryBody: string | null = null; diff --git a/server/src/services/plugin-managed-agents.ts b/server/src/services/plugin-managed-agents.ts index feb725c0b3..13473ec8c0 100644 --- a/server/src/services/plugin-managed-agents.ts +++ b/server/src/services/plugin-managed-agents.ts @@ -16,7 +16,7 @@ import { notFound } from "../errors.js"; import { agentService } from "./agents.js"; import { approvalService } from "./approvals.js"; import { logActivity } from "./activity-log.js"; -import { agentInstructionsService } from "./agent-instructions.js"; +import { agentInstructionsBundleMode, agentInstructionsService } from "./agent-instructions.js"; const MANAGED_AGENT_ENTITY_TYPE = "managed_agent"; const DEFAULT_MANAGED_AGENT_ADAPTER_TYPE = "process"; @@ -364,6 +364,9 @@ export function pluginManagedAgentService( const variables = await optionsForInstructionVariables(companyId); const declared = declaredInstructionFiles(declaration, variables); if (!declared) return null; + if (agentInstructionsBundleMode(agent) === "external") { + return { entryFile: declared.entryFile, changedFiles: [declared.entryFile] }; + } let exported: Awaited>; try { diff --git a/server/src/services/projects.ts b/server/src/services/projects.ts index 5ac1b50e4c..0f193a98a7 100644 --- a/server/src/services/projects.ts +++ b/server/src/services/projects.ts @@ -28,6 +28,7 @@ import { type PluginManagedProjectDeclaration, type PluginManagedProjectResolution, } from "@paperclipai/shared"; +import { unprocessable } from "../errors.js"; import { listCurrentRuntimeServicesForProjectWorkspaces } from "./workspace-runtime-read-model.js"; import { parseProjectExecutionWorkspacePolicy } from "./execution-workspace-policy.js"; import { mergeProjectWorkspaceRuntimeConfig, readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js"; @@ -401,6 +402,31 @@ async function attachListMetrics( } /** Sync the project_goals join table for a single project. */ +/** + * Every goal a project links to must exist and belong to the same company. + * Without this check a nonexistent id only dies at the projects.goal_id + * foreign key — an opaque 500 the caller retries (observed live + * 2026-09-03: four identical retries of one bad id) — and a goal from + * another company would link silently, because the foreign key proves + * existence, not ownership. + */ +async function assertGoalsBelongToCompany(db: Db, companyId: string, goalIds: string[]): Promise { + if (goalIds.length === 0) return; + const unique = [...new Set(goalIds)]; + const found = await db + .select({ id: goals.id }) + .from(goals) + .where(and(eq(goals.companyId, companyId), inArray(goals.id, unique))); + const foundIds = new Set(found.map((row) => row.id)); + const unknown = unique.filter((goalId) => !foundIds.has(goalId)); + if (unknown.length > 0) { + throw unprocessable( + `Unknown goal id(s) for this company: ${unknown.join(", ")}`, + { unknownGoalIds: unknown }, + ); + } +} + async function syncGoalLinks(db: Db, projectId: string, companyId: string, goalIds: string[]) { // Delete existing links await db.delete(projectGoals).where(eq(projectGoals.projectId, projectId)); @@ -547,6 +573,7 @@ export function projectService(db: Db) { ): Promise => { const { goalIds: inputGoalIds, ...projectData } = data; const ids = resolveGoalIds({ goalIds: inputGoalIds, goalId: projectData.goalId }); + if (ids && ids.length > 0) await assertGoalsBelongToCompany(db, companyId, ids); // Note: color is intentionally NOT auto-assigned. New projects default to // `color = null` (neutral gray) unless an explicit color is supplied. See PAP-68. @@ -558,7 +585,11 @@ export function projectService(db: Db) { projectData.name = resolveProjectNameForUniqueShortname(projectData.name, existingProjects); // Also write goalId to the legacy column (first goal or null) - const legacyGoalId = ids && ids.length > 0 ? ids[0] : projectData.goalId ?? null; + // The resolved set is canonical for persistence as well as validation: + // falling back to the raw legacy field here would write an id that + // skipped validation whenever `goalIds: []` and `goalId` arrive + // together (goalIds wins resolution, mirroring the update path). + const legacyGoalId = ids?.[0] ?? null; const row = await db .insert(projects) @@ -794,6 +825,9 @@ export function projectService(db: Db) { .where(eq(projects.id, id)) .then((rows) => rows[0] ?? null); if (!existingProject) return null; + if (ids && ids.length > 0) { + await assertGoalsBelongToCompany(db, existingProject.companyId, ids); + } if (projectData.name !== undefined) { const existingShortname = normalizeProjectUrlKey(existingProject.name);