From 9dd6526b47f3ca3a228e5eb7378de055e1622d8a Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:15:32 -0500 Subject: [PATCH] fix(security): harden privileged server boundaries (#12776) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server controls secrets, host files, outbound requests, and workspace commands > - A red-team review found cases where restricted callers could cross these trust boundaries > - These cases could expose credentials or let untrusted input reach privileged resources > - This pull request applies least-privilege checks at each affected server boundary > - The benefit is safer agent execution without changing the private-instance bootstrap contract ## Linked Issues or Issue Description **What happened?** Several server paths used authorization, redaction, or content-delivery rules that were too broad. Restricted agent keys could obtain company-level operational data. Some adapter and instruction paths could reach server-owned network or file resources without the required owner approval. **Expected behavior** Paperclip must redact credential values, enforce restricted-key scopes, guard outbound network access, prevent same-origin script execution, and reserve host-level file and command controls for authorized operators. **Steps to reproduce** 1. Configure an authenticated development instance at the parent commit. 2. Exercise the affected APIs with a restricted agent key or a non-instance-admin company user. 3. Observe that the parent commit returns privileged data or accepts a privileged operation. 4. Repeat on this branch and observe a redacted response, a safe download, or an HTTP 403 response. **Paperclip version or commit** The findings reproduce from commit `39898ab22` and are fixed by this pull request. **Deployment mode** Authenticated self-hosted server and local development modes. **Installation method** Built from source with pnpm. ## What Changed - Redact generic secret `value` and `token` fields recursively in structured logs. - Classify exact and separator-suffixed `KEY` environment names as secrets in company exports. - Limit restricted self-identity responses and protect company run, log, and secret catalog APIs. - Route HTTP adapter requests through DNS-pinned SSRF protection with exact private-origin allowlisting. - Download HTML, SVG, and other script-capable assets with `nosniff` and a sandbox CSP. - Require instance-admin access for external instruction roots and exports that read them. - Block agent-authenticated host command persistence across supported workspace runtime shapes. - Apply the central runtime-management decision before workspace command controls. - Keep the documented first-user instance-admin claim contract unchanged. - Add regression tests and server-owner configuration documentation. ## Verification - `pnpm -r typecheck` passes. - The Node 24 remediation suite passes with 365 tests. It skips 25 environment-gated tests. - `pnpm build` passes under Node 24. - `git diff --check` passes. - The full local runner reaches known macOS-only general-server harness failures before the serialized route lane. The Linux PR matrix is the authoritative full-suite gate. ## Risks - Restricted agent keys now receive HTTP 403 responses from company-wide run, log, and secret catalog endpoints. - Script-capable assets now download instead of rendering inline. - External instruction roots now require instance-admin access. - Private HTTP adapter endpoints now require an exact origin in `PAPERCLIP_HTTP_ADAPTER_PRIVATE_ENDPOINT_ALLOWLIST`. - Public HTTP adapter endpoints remain enabled. Redirects and metadata or link-local targets remain blocked. - No database migration is required. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5. The exact serving snapshot and context-window size are not exposed. The model used tool-enabled reasoning, repository access, code execution, and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .env.example | 4 + doc/DEPLOYMENT-MODES.md | 6 + doc/DEVELOPING.md | 18 ++ doc/SPEC-implementation.md | 5 + .../agent-instructions-routes.test.ts | 203 +++++++++++++++++- .../__tests__/agent-live-run-routes.test.ts | 82 ++++++- .../agent-permissions-routes.test.ts | 4 + server/src/__tests__/assets.test.ts | 87 ++++++++ ...ompanies-route-cross-company-authz.test.ts | 2 + .../company-import-cloud-floor.test.ts | 2 +- .../company-portability-routes.test.ts | 176 ++++++++++++++- .../src/__tests__/company-portability.test.ts | 78 +++++++ server/src/__tests__/feedback-service.test.ts | 10 +- .../http-adapter-remote-fetch.test.ts | 91 ++++++++ .../src/__tests__/http-log-redaction.test.ts | 38 ++++ .../low-trust-red-team-routes.test.ts | 25 +++ server/src/__tests__/redact-sensitive.test.ts | 14 +- server/src/__tests__/secrets-routes.test.ts | 40 ++++ .../__tests__/workspace-command-authz.test.ts | 70 ++++++ .../workspace-runtime-routes-authz.test.ts | 99 +++++++++ server/src/adapters/http/execute.test.ts | 20 +- server/src/adapters/http/execute.ts | 3 +- server/src/adapters/http/remote-fetch.ts | 77 +++++++ server/src/adapters/http/test.ts | 3 +- server/src/middleware/redact-sensitive.ts | 5 + server/src/routes/agents.ts | 81 ++++++- server/src/routes/assets.ts | 13 +- server/src/routes/companies.ts | 30 ++- server/src/routes/projects.ts | 12 ++ server/src/routes/secrets.ts | 11 + server/src/routes/workspace-command-authz.ts | 52 ++++- server/src/services/agent-instructions.ts | 13 ++ .../company-portability-agent-selection.ts | 67 ++++++ server/src/services/company-portability.ts | 80 ++----- server/src/services/feedback.ts | 31 ++- server/src/services/plugin-managed-agents.ts | 5 +- 36 files changed, 1435 insertions(+), 122 deletions(-) create mode 100644 server/src/__tests__/http-adapter-remote-fetch.test.ts create mode 100644 server/src/__tests__/workspace-command-authz.test.ts create mode 100644 server/src/adapters/http/remote-fetch.ts create mode 100644 server/src/services/company-portability-agent-selection.ts 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 cc6fe817e4..06aee69cf8 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -183,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, })); 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__/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__/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__/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/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/routes/agents.ts b/server/src/routes/agents.ts index 6e3873ddc7..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, @@ -1012,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[], @@ -2556,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); } @@ -3629,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, @@ -3650,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)); }); @@ -3817,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" @@ -3948,6 +3977,12 @@ export function agentRoutes( rawHireAdapterConfig, ), ); + assertExternalInstructionsAdmin(req, { + id: hiredAgentId, + companyId, + name: hireInput.name, + adapterConfig: requestedAdapterConfig, + }); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, hireInput.adapterType, @@ -4167,6 +4202,12 @@ export function agentRoutes( rawCreateAdapterConfig, ), ); + assertExternalInstructionsAdmin(req, { + id: agentId, + companyId, + name: createInput.name, + adapterConfig: requestedAdapterConfig, + }); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, createInput.adapterType, @@ -4327,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); @@ -4347,6 +4389,7 @@ export function agentRoutes( } const syncedAdapterConfig = syncInstructionsBundleConfigFromFilePath(existing, nextAdapterConfig); + assertExternalInstructionsAdmin(req, { ...existing, adapterConfig: syncedAdapterConfig }); const normalizedAdapterConfig = await secretsSvc.normalizeAdapterConfigForPersistence( existing.companyId, syncedAdapterConfig, @@ -4402,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)); }); @@ -4410,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); @@ -4456,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()) { @@ -4471,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, { @@ -4518,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()) { @@ -4605,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; @@ -4678,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")) { @@ -5755,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; @@ -5795,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 @@ -5882,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( @@ -6355,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); @@ -6373,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); @@ -6389,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); @@ -6400,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 {