diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index a62b8655..40a7266a 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -297,7 +297,7 @@ 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 () => { + it("does not inject CODEX_HOME or OPENAI_API_KEY when creating a keyless codex_local agent", async () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => request(baseUrl) @@ -311,16 +311,13 @@ describe("agent routes adapter validation", () => { 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"); + const env = (adapterConfig.env as Record | undefined) ?? {}; + expect(env.OPENAI_API_KEY).toBeUndefined(); + expect(env.CODEX_HOME).toBeUndefined(); }); - it("adds isolated CODEX_HOME and empty OPENAI_API_KEY override when updating codex_local agents", async () => { + it("does not re-inject CODEX_HOME or OPENAI_API_KEY when updating a keyless codex_local agent", async () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => request(baseUrl) @@ -333,16 +330,37 @@ describe("agent routes adapter validation", () => { 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"); + const env = (adapterConfig.env as Record | undefined) ?? {}; + expect(env.OPENAI_API_KEY).toBeUndefined(); + expect(env.CODEX_HOME).toBeUndefined(); }); - it("rejects codex_local agents configured with the shared host Codex home", async () => { + it("isolates CODEX_HOME when updating a codex_local agent to set its own OPENAI_API_KEY", async () => { + const agentId = "11111111-1111-4111-8111-111111111111"; const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .patch(`/api/agents/${agentId}`) + .send({ + adapterConfig: { + env: { + OPENAI_API_KEY: "sk-test-key", + }, + }, + }), + ); + + 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("sk-test-key"); + expect(String(env.CODEX_HOME)).toContain(`/companies/company-1/agents/${agentId}/codex-home`); + }); + + it("allows codex_local agents to share the host Codex home", async () => { + const app = await createApp(); + const sharedHome = path.join(os.homedir(), ".codex"); const res = await requestApp(app, (baseUrl) => request(baseUrl) .post("/api/companies/company-1/agents") @@ -351,36 +369,42 @@ describe("agent routes adapter validation", () => { adapterType: "codex_local", adapterConfig: { env: { - CODEX_HOME: path.join(os.homedir(), ".codex"), + CODEX_HOME: sharedHome, }, }, }), ); - 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(); + expect(res.status, JSON.stringify(res.body)).toBe(201); + const createInput = mockAgentService.create.mock.calls.at(-1)?.[1] as Record; + const adapterConfig = createInput.adapterConfig as Record; + const env = adapterConfig.env as Record; + expect(env.CODEX_HOME).toBe(sharedHome); }); - it("rejects codex_local agents configured with the ~/.codex alias", async () => { + it("isolates CODEX_HOME when a codex_local agent sets its own OPENAI_API_KEY", async () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => request(baseUrl) .post("/api/companies/company-1/agents") .send({ - name: "Shared Codex Alias", + name: "Keyed Codex", adapterType: "codex_local", adapterConfig: { env: { - CODEX_HOME: "~/.codex", + OPENAI_API_KEY: "sk-test-key", }, }, }), ); - 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(); + 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); + const adapterConfig = createInput.adapterConfig as Record; + const env = adapterConfig.env as Record; + expect(env.OPENAI_API_KEY).toBe("sk-test-key"); + expect(String(env.CODEX_HOME)).toContain(`/companies/company-1/agents/${agentId}/codex-home`); }); it("rejects unknown adapter types even when schema accepts arbitrary strings", async () => { diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index fa12da1d..d066bc86 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1,6 +1,5 @@ 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"; @@ -1152,53 +1151,33 @@ export function agentRoutes( 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 codexLocalEnvKeyConfigured(value: unknown): boolean { + if (asEnvBindingString(value)) return true; + const record = asRecord(value); + return record?.type === "secret_ref" && typeof record.secretId === "string"; } - 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( + // codex_local agents inherit whatever Codex login is already on the device + // (the host's ~/.codex or $CODEX_HOME) by default, so a fresh agent needs no + // env overrides at all. We only carve out an isolated per-agent CODEX_HOME + // when the agent sets its own OPENAI_API_KEY, so that key's api-key auth.json + // does not collide with the shared company home other agents use for the host + // login. Agents without a key share the host credentials. + function applyCodexLocalKeyIsolation( 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 }; + const existingEnv = asRecord(adapterConfig.env); + if (!existingEnv) return adapterConfig; + if (!codexLocalEnvKeyConfigured(existingEnv.OPENAI_API_KEY)) return adapterConfig; + if (codexLocalEnvKeyConfigured(existingEnv.CODEX_HOME)) return adapterConfig; + return { + ...adapterConfig, + env: { ...existingEnv, CODEX_HOME: codexLocalAgentHome(companyId, agentId) }, + }; } function applyCreateDefaultsByAdapterType( @@ -2216,7 +2195,7 @@ export function agentRoutes( assertNoAgentAdapterConfigMutation(req, rawHireAdapterConfig); assertNoAgentRuntimeConfigAdapterConfigMutation(req, hireInput.runtimeConfig); const hiredAgentId = randomUUID(); - const requestedAdapterConfig = applyCodexLocalIsolationGuard( + const requestedAdapterConfig = applyCodexLocalKeyIsolation( companyId, hiredAgentId, hireInput.adapterType, @@ -2409,7 +2388,7 @@ export function agentRoutes( assertNoAgentAdapterConfigMutation(req, rawCreateAdapterConfig); assertNoAgentRuntimeConfigAdapterConfigMutation(req, createInput.runtimeConfig); const agentId = randomUUID(); - const requestedAdapterConfig = applyCodexLocalIsolationGuard( + const requestedAdapterConfig = applyCodexLocalKeyIsolation( companyId, agentId, createInput.adapterType, @@ -2883,7 +2862,7 @@ export function agentRoutes( rawEffectiveAdapterConfig, ); } - const effectiveAdapterConfig = applyCodexLocalIsolationGuard( + const effectiveAdapterConfig = applyCodexLocalKeyIsolation( existing.companyId, existing.id, requestedAdapterType,