diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index d914d700..f15c2440 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -196,7 +196,8 @@ Every local install keeps runtime state directly under the selected instance roo secrets/master.key # local_encrypted master key workspaces// # default agent workspaces projects/ # project execution workspaces - companies//codex-home/ # per-company codex_local home + companies//agents//codex-home/ + # per-agent codex_local home ``` `PAPERCLIP_HOME` and `PAPERCLIP_INSTANCE_ID` override the home root and instance id respectively. `paperclipai onboard` echoes the resolved values in its banner (`Local home: | instance: | config: `) so you can confirm where state will land before continuing. @@ -266,9 +267,11 @@ When a local agent run has no resolved project/session workspace, Paperclip fall This path honors `PAPERCLIP_HOME` and `PAPERCLIP_INSTANCE_ID` in non-default setups. -For `codex_local`, Paperclip also manages a per-company Codex home under the instance root and seeds it from the shared Codex login/config home (`$CODEX_HOME` or `~/.codex`): +For `codex_local`, Paperclip assigns new and updated agents an isolated Codex home under the instance root and blocks shared host/company Codex homes: -- `~/.paperclip/instances/default/companies//codex-home` +- `~/.paperclip/instances/default/companies//agents//codex-home` + +Paperclip also persists an empty `OPENAI_API_KEY` override for those agents so a host-level `OPENAI_API_KEY` cannot leak into Codex runs through process inheritance. If an operator explicitly configures `adapterConfig.env.CODEX_HOME`, it must not point at the shared company `codex-home`, `$CODEX_HOME`, or `~/.codex`. If the `codex` CLI is not installed or not on `PATH`, `codex_local` agent runs fail at execution time with a clear adapter error. Quota polling uses a short-lived `codex app-server` subprocess: when `codex` cannot be spawned, that provider reports `ok: false` in aggregated quota results and the API server keeps running (it must not exit on a missing binary). diff --git a/packages/adapters/codex-local/src/index.ts b/packages/adapters/codex-local/src/index.ts index 4c4a2de0..944d0369 100644 --- a/packages/adapters/codex-local/src/index.ts +++ b/packages/adapters/codex-local/src/index.ts @@ -92,8 +92,8 @@ Notes: - Prompts are piped via stdin (Codex receives "-" prompt argument). - If instructionsFilePath is configured, Paperclip prepends that file's contents to the stdin prompt on every run. - Codex exec automatically applies repo-scoped AGENTS.md instructions from the active workspace. Paperclip cannot suppress that discovery in exec mode, so repo AGENTS.md files may still apply even when you only configured an explicit instructionsFilePath. -- Paperclip injects desired local skills into the effective CODEX_HOME/skills/ directory at execution time so Codex can discover "$paperclip" and related skills without polluting the project working directory. In managed-home mode (the default) this is ~/.paperclip/instances//companies//codex-home/skills/; when CODEX_HOME is explicitly overridden in adapter config, that override is used instead. -- Unless explicitly overridden in adapter config, Paperclip runs Codex with a per-company managed CODEX_HOME under the active Paperclip instance and seeds auth/config from the shared Codex home (the CODEX_HOME env var, when set, or ~/.codex). +- Paperclip injects desired local skills into the effective CODEX_HOME/skills/ directory at execution time so Codex can discover "$paperclip" and related skills without polluting the project working directory. For new and updated agents, Paperclip assigns an isolated managed home at ~/.paperclip/instances//companies//agents//codex-home/skills/; when CODEX_HOME is explicitly overridden in adapter config, that override is used instead. +- New and updated codex_local agents persist an empty OPENAI_API_KEY override by default so a host-level OPENAI_API_KEY cannot leak into Codex runs through process inheritance. Explicit CODEX_HOME overrides must not point at the shared company codex-home, $CODEX_HOME, or ~/.codex. - Some model/tool combinations reject certain effort levels (for example minimal with web search enabled). - Fast mode is supported on GPT-5.5, GPT-5.4 and manual model IDs. When enabled for those models, Paperclip applies \`service_tier="fast"\` and \`features.fast_mode=true\`. - When Paperclip realizes a workspace/runtime for a run, it injects PAPERCLIP_WORKSPACE_* and PAPERCLIP_RUNTIME_* env vars for agent-side tooling. diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 198efd9c..a62b8655 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -1,4 +1,6 @@ import express from "express"; +import os from "node:os"; +import path from "node:path"; import request from "supertest"; import { beforeEach, afterEach, describe, expect, it, vi } from "vitest"; import type { ServerAdapterModule } from "../adapters/index.js"; @@ -6,6 +8,7 @@ import type { ServerAdapterModule } from "../adapters/index.js"; const mockAgentService = vi.hoisted(() => ({ create: vi.fn(), getById: vi.fn(), + update: vi.fn(), })); const mockAccessService = vi.hoisted(() => ({ @@ -24,6 +27,7 @@ const mockCompanySkillService = vi.hoisted(() => ({ const mockSecretService = vi.hoisted(() => ({ normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record) => config), resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record) => ({ config })), + syncEnvBindingsForTarget: vi.fn(), })); const mockAgentInstructionsService = vi.hoisted(() => ({ @@ -80,6 +84,10 @@ vi.mock("../services/instance-settings.js", () => ({ instanceSettingsService: () => mockInstanceSettingsService, })); +vi.mock("../services/secrets.js", () => ({ + secretService: () => mockSecretService, +})); + function registerModuleMocks() { vi.doMock("../services/index.js", () => ({ agentService: () => mockAgentService, @@ -100,6 +108,10 @@ function registerModuleMocks() { vi.doMock("../services/instance-settings.js", () => ({ instanceSettingsService: () => mockInstanceSettingsService, })); + + vi.doMock("../services/secrets.js", () => ({ + secretService: () => mockSecretService, + })); } const externalAdapter: ServerAdapterModule = { @@ -202,8 +214,12 @@ describe("agent routes adapter validation", () => { mockAccessService.ensureMembership.mockResolvedValue(undefined); mockAccessService.setPrincipalPermission.mockResolvedValue(undefined); mockLogActivity.mockResolvedValue(undefined); + mockSecretService.syncEnvBindingsForTarget.mockResolvedValue(undefined); + mockAgentInstructionsService.materializeManagedBundle.mockImplementation(async (agent: { adapterConfig: unknown }) => ({ + adapterConfig: agent.adapterConfig, + })); mockAgentService.create.mockImplementation(async (_companyId: string, input: Record) => ({ - id: "11111111-1111-4111-8111-111111111111", + id: String(input.id ?? "11111111-1111-4111-8111-111111111111"), companyId: "company-1", name: String(input.name ?? "Agent"), urlKey: "agent", @@ -226,6 +242,34 @@ describe("agent routes adapter validation", () => { createdAt: new Date(), updatedAt: new Date(), })); + mockAgentService.getById.mockResolvedValue({ + id: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + name: "Codex", + urlKey: "codex", + role: "engineer", + title: null, + icon: null, + status: "idle", + reportsTo: null, + capabilities: null, + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + budgetMonthlyCents: 0, + spentMonthlyCents: 0, + pauseReason: null, + pausedAt: null, + permissions: { canCreateAgents: false }, + lastHeartbeatAt: null, + metadata: null, + createdAt: new Date(), + updatedAt: new Date(), + }); + mockAgentService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...(await mockAgentService.getById()), + ...patch, + })); await unregisterTestAdapter("external_test"); await unregisterTestAdapter(missingAdapterType); }); @@ -253,6 +297,92 @@ describe("agent routes adapter validation", () => { expect(res.body.adapterType).toBe("external_test"); }); + it("adds isolated CODEX_HOME and empty OPENAI_API_KEY override when creating codex_local agents", async () => { + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/agents") + .send({ + name: "Codex Agent", + adapterType: "codex_local", + adapterConfig: {}, + }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + const createInput = mockAgentService.create.mock.calls.at(-1)?.[1] as Record; + const agentId = String(createInput.id); + expect(agentId).toMatch(/^[0-9a-f-]{36}$/i); + const adapterConfig = createInput.adapterConfig as Record; + const env = adapterConfig.env as Record; + expect(env.OPENAI_API_KEY).toBe(""); + expect(env.CODEX_HOME).toContain(`/companies/company-1/agents/${agentId}/codex-home`); + expect(String(env.CODEX_HOME)).not.toContain("/companies/company-1/codex-home"); + }); + + it("adds isolated CODEX_HOME and empty OPENAI_API_KEY override when updating codex_local agents", async () => { + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .patch("/api/agents/11111111-1111-4111-8111-111111111111") + .send({ + adapterConfig: { model: "gpt-5.4" }, + }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + const patch = mockAgentService.update.mock.calls.at(-1)?.[1] as Record; + const adapterConfig = patch.adapterConfig as Record; + const env = adapterConfig.env as Record; + expect(env.OPENAI_API_KEY).toBe(""); + expect(env.CODEX_HOME).toContain( + "/companies/company-1/agents/11111111-1111-4111-8111-111111111111/codex-home", + ); + expect(String(env.CODEX_HOME)).not.toContain("/companies/company-1/codex-home"); + }); + + it("rejects codex_local agents configured with the shared host Codex home", async () => { + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/agents") + .send({ + name: "Shared Codex", + adapterType: "codex_local", + adapterConfig: { + env: { + CODEX_HOME: path.join(os.homedir(), ".codex"), + }, + }, + }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + expect(String(res.body.error)).toContain("codex_local agents must use an isolated adapterConfig.env.CODEX_HOME"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + + it("rejects codex_local agents configured with the ~/.codex alias", async () => { + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post("/api/companies/company-1/agents") + .send({ + name: "Shared Codex Alias", + adapterType: "codex_local", + adapterConfig: { + env: { + CODEX_HOME: "~/.codex", + }, + }, + }), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + expect(String(res.body.error)).toContain("codex_local agents must use an isolated adapterConfig.env.CODEX_HOME"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + }); + it("rejects unknown adapter types even when schema accepts arbitrary strings", async () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index 1ba32715..c2f8266f 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -63,6 +63,13 @@ const mockAdapter = vi.hoisted(() => ({ syncSkills: vi.fn(), })); +function expectResponseId(value: unknown): string { + expect(value).toEqual(expect.any(String)); + expect(value).not.toBe(""); + expect(value).not.toBe("undefined"); + return String(value); +} + vi.mock("@paperclipai/shared/telemetry", () => ({ trackAgentCreated: mockTrackAgentCreated, trackErrorHandlerCrash: vi.fn(), @@ -546,6 +553,7 @@ describe.sequential("agent skill routes", () => { })); expect([200, 201], JSON.stringify(res.body)).toContain(res.status); + const createdAgentId = expectResponseId(res.body.id); expect(mockAgentService.create).toHaveBeenCalledWith( "company-1", expect.objectContaining({ @@ -559,7 +567,7 @@ describe.sequential("agent skill routes", () => { expect(mockTrackAgentCreated).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - agentId: "11111111-1111-4111-8111-111111111111", + agentId: createdAgentId, agentRole: "engineer", }), ); @@ -576,6 +584,7 @@ describe.sequential("agent skill routes", () => { })); expect([200, 201], JSON.stringify(res.body)).toContain(res.status); + const createdAgentId = expectResponseId(res.body.id); expect(res.body).toMatchObject({ role: "security", }); @@ -588,7 +597,7 @@ describe.sequential("agent skill routes", () => { expect(mockTrackAgentCreated).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ - agentId: "11111111-1111-4111-8111-111111111111", + agentId: createdAgentId, agentRole: "security", }), ); @@ -610,13 +619,15 @@ describe.sequential("agent skill routes", () => { })); expect([200, 201], JSON.stringify(res.body)).toContain(res.status); + const createdAgentId = expectResponseId(res.body.id); expect(mockAgentService.update).toHaveBeenCalledWith( - "11111111-1111-4111-8111-111111111111", + createdAgentId, expect.objectContaining({ adapterConfig: expect.objectContaining({ instructionsBundleMode: "managed", instructionsEntryFile: "AGENTS.md", - instructionsFilePath: "/tmp/11111111-1111-4111-8111-111111111111/instructions/AGENTS.md", + instructionsRootPath: `/tmp/${createdAgentId}/instructions`, + instructionsFilePath: `/tmp/${createdAgentId}/instructions/AGENTS.md`, }), }), ); @@ -658,9 +669,10 @@ describe.sequential("agent skill routes", () => { })); expect([200, 201], JSON.stringify(res.body)).toContain(res.status); + const createdAgentId = expectResponseId(res.body.id); expect(mockAgentInstructionsService.materializeManagedBundle).toHaveBeenCalledWith( expect.objectContaining({ - id: "11111111-1111-4111-8111-111111111111", + id: createdAgentId, role: "ceo", adapterType: "claude_local", }), @@ -685,10 +697,11 @@ describe.sequential("agent skill routes", () => { })); expect([200, 201], JSON.stringify(res.body)).toContain(res.status); + const createdAgentId = expectResponseId(res.body.id); await vi.waitFor(() => { expect(mockAgentInstructionsService.materializeManagedBundle).toHaveBeenCalledWith( expect.objectContaining({ - id: "11111111-1111-4111-8111-111111111111", + id: createdAgentId, role: "engineer", adapterType: "claude_local", }), @@ -811,6 +824,10 @@ describe.sequential("agent skill routes", () => { }); expect(res.status, JSON.stringify(res.body)).toBe(201); + const approvalInput = mockApprovalService.create.mock.calls.at(-1)?.[1] as + | { payload?: { agentId?: string; adapterConfig?: Record } } + | undefined; + const hiredAgentId = expectResponseId(approvalInput?.payload?.agentId); expect(mockApprovalService.create).toHaveBeenCalledWith( "company-1", expect.objectContaining({ @@ -818,14 +835,12 @@ describe.sequential("agent skill routes", () => { adapterConfig: expect.objectContaining({ instructionsBundleMode: "managed", instructionsEntryFile: "AGENTS.md", - instructionsFilePath: "/tmp/11111111-1111-4111-8111-111111111111/instructions/AGENTS.md", + instructionsRootPath: `/tmp/${hiredAgentId}/instructions`, + instructionsFilePath: `/tmp/${hiredAgentId}/instructions/AGENTS.md`, }), }), }), ); - const approvalInput = mockApprovalService.create.mock.calls.at(-1)?.[1] as - | { payload?: { adapterConfig?: Record } } - | undefined; expect(approvalInput?.payload?.adapterConfig?.promptTemplate).toBeUndefined(); }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 0a9688ac..4d22fa3d 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1,5 +1,6 @@ import { Router, type Request, type Response } from "express"; import { generateKeyPairSync, randomUUID } from "node:crypto"; +import os from "node:os"; import path from "node:path"; import type { Db } from "@paperclipai/db"; import { agents as agentsTable, companies, heartbeatRuns, issues as issuesTable, projects as projectsTable } from "@paperclipai/db"; @@ -29,6 +30,7 @@ import { LOW_TRUST_REVIEW_PRESET, } from "@paperclipai/shared"; import { + resolvePaperclipInstanceRootForAdapter, readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference, } from "@paperclipai/adapter-utils/server-utils"; @@ -957,6 +959,14 @@ export function agentRoutes( return trimmed.length > 0 ? trimmed : null; } + function asEnvBindingString(value: unknown): string | null { + const direct = asNonEmptyString(value); + if (direct) return direct; + const record = asRecord(value); + if (record?.type !== "plain") return null; + return asNonEmptyString(record.value); + } + function preserveInstructionsBundleConfig( existingAdapterConfig: Record, nextAdapterConfig: Record, @@ -1133,6 +1143,64 @@ export function agentRoutes( return { ...adapterConfig, devicePrivateKeyPem: generateEd25519PrivateKeyPem() }; } + function codexLocalAgentHome(companyId: string, agentId: string): string { + const instanceRoot = resolvePaperclipInstanceRootForAdapter({ + homeDir: asNonEmptyString(process.env.PAPERCLIP_HOME) ?? undefined, + instanceId: asNonEmptyString(process.env.PAPERCLIP_INSTANCE_ID) ?? undefined, + env: process.env, + }); + return path.resolve(instanceRoot, "companies", companyId, "agents", agentId, "codex-home"); + } + + function normalizeCodexLocalHomePath(rawHome: string): string { + if (rawHome === "~") return path.resolve(os.homedir()); + if (rawHome.startsWith("~/") || rawHome.startsWith("~\\")) { + return path.resolve(path.join(os.homedir(), rawHome.slice(2))); + } + return path.resolve(rawHome); + } + + function assertCodexLocalHomeIsNotShared(companyId: string, configuredHome: string) { + const instanceRoot = resolvePaperclipInstanceRootForAdapter({ + homeDir: asNonEmptyString(process.env.PAPERCLIP_HOME) ?? undefined, + instanceId: asNonEmptyString(process.env.PAPERCLIP_INSTANCE_ID) ?? undefined, + env: process.env, + }); + const normalizedHome = normalizeCodexLocalHomePath(configuredHome); + const sharedHomes = [ + path.resolve(instanceRoot, "companies", companyId, "codex-home"), + path.resolve(path.join(os.homedir(), ".codex")), + ]; + const hostCodexHome = asNonEmptyString(process.env.CODEX_HOME); + if (hostCodexHome) { + sharedHomes.push(normalizeCodexLocalHomePath(hostCodexHome)); + } + if (!sharedHomes.some((sharedHome) => sharedHome === normalizedHome)) return; + throw unprocessable( + "codex_local agents must use an isolated adapterConfig.env.CODEX_HOME; shared company codex-home or host Codex auth home is not allowed", + ); + } + + function applyCodexLocalIsolationGuard( + companyId: string, + agentId: string, + adapterType: string | null | undefined, + adapterConfig: Record, + ): Record { + if (adapterType !== "codex_local") return adapterConfig; + const env = asRecord(adapterConfig.env) ? { ...(adapterConfig.env as Record) } : {}; + const configuredHome = asEnvBindingString(env.CODEX_HOME); + if (configuredHome) { + assertCodexLocalHomeIsNotShared(companyId, configuredHome); + } else { + env.CODEX_HOME = codexLocalAgentHome(companyId, agentId); + } + if (!Object.prototype.hasOwnProperty.call(env, "OPENAI_API_KEY")) { + env.OPENAI_API_KEY = ""; + } + return { ...adapterConfig, env }; + } + function applyCreateDefaultsByAdapterType( adapterType: string | null | undefined, adapterConfig: Record, @@ -2158,9 +2226,15 @@ export function agentRoutes( ); assertNoAgentAdapterConfigMutation(req, rawHireAdapterConfig); assertNoAgentRuntimeConfigAdapterConfigMutation(req, hireInput.runtimeConfig); - const requestedAdapterConfig = applyCreateDefaultsByAdapterType( + const hiredAgentId = randomUUID(); + const requestedAdapterConfig = applyCodexLocalIsolationGuard( + companyId, + hiredAgentId, hireInput.adapterType, - rawHireAdapterConfig, + applyCreateDefaultsByAdapterType( + hireInput.adapterType, + rawHireAdapterConfig, + ), ); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, @@ -2198,6 +2272,7 @@ export function agentRoutes( const requiresApproval = company.requireBoardApprovalForNewAgents; const status = requiresApproval ? "pending_approval" : "idle"; const createdAgent = await svc.create(companyId, { + id: hiredAgentId, ...normalizedHireInput, status, spentMonthlyCents: 0, @@ -2344,9 +2419,15 @@ export function agentRoutes( ); assertNoAgentAdapterConfigMutation(req, rawCreateAdapterConfig); assertNoAgentRuntimeConfigAdapterConfigMutation(req, createInput.runtimeConfig); - const requestedAdapterConfig = applyCreateDefaultsByAdapterType( + const agentId = randomUUID(); + const requestedAdapterConfig = applyCodexLocalIsolationGuard( + companyId, + agentId, createInput.adapterType, - rawCreateAdapterConfig, + applyCreateDefaultsByAdapterType( + createInput.adapterType, + rawCreateAdapterConfig, + ), ); const desiredSkillAssignment = await resolveDesiredSkillAssignment( companyId, @@ -2372,6 +2453,7 @@ export function agentRoutes( }); const createdAgent = await svc.create(companyId, { + id: agentId, ...createInput, adapterConfig: normalizedAdapterConfig, runtimeConfig: normalizedRuntimeConfig, @@ -2820,9 +2902,14 @@ export function agentRoutes( rawEffectiveAdapterConfig, ); } - const effectiveAdapterConfig = applyCreateDefaultsByAdapterType( + const effectiveAdapterConfig = applyCodexLocalIsolationGuard( + existing.companyId, + existing.id, requestedAdapterType, - rawEffectiveAdapterConfig, + applyCreateDefaultsByAdapterType( + requestedAdapterType, + rawEffectiveAdapterConfig, + ), ); const normalizedEffectiveAdapterConfig = await normalizeMediatedAdapterConfigForPersistence({ companyId: existing.companyId,