fix(server): let codex_local agents inherit host Codex login by default (#8425)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The `codex_local` adapter spawns the Codex CLI for an agent, and `server/src/routes/agents.ts` normalizes each agent's `adapterConfig.env` on create/hire/update > - PR #8272 added an isolation guard that, on every codex_local agent, force-set a per-agent `CODEX_HOME` and injected `OPENAI_API_KEY = ""`, and rejected any "shared" home > - This broke the common case: operators who deleted `CODEX_HOME` / `OPENAI_API_KEY` in the UI saw them silently re-appear on save, and every agent was forced into an isolated home instead of sharing the device's existing Codex login (`~/.codex` / `$CODEX_HOME`) > - This pull request replaces the always-on isolation guard with key-scoped isolation: a keyless agent gets no env overrides and inherits the host Codex login at runtime; we only carve out an isolated per-agent `CODEX_HOME` when the agent sets its own `OPENAI_API_KEY` > - The benefit is that env-var deletion now persists, agents on one device share the host login by default, and per-account isolation is still available by setting a per-agent key ## Linked Issues or Issue Description No public GitHub issue exists. Bug report (following `bug_report.yml`): **What happened?** In a `codex_local` agent's configuration, removing the `CODEX_HOME` and `OPENAI_API_KEY` env vars via the UI (clicking the X) appears to work, but on save they instantly re-appear. The persisted config never loses the slots. **Root cause.** `applyCodexLocalIsolationGuard` in `server/src/routes/agents.ts` (added in #8272) re-injected `CODEX_HOME` (defaulting to a per-agent home) and `OPENAI_API_KEY = ""` on every create/hire/update, and rejected shared homes outright. So a PATCH that omitted those keys had them written back server-side. **Expected behavior.** Deleting these env vars should persist. A codex_local agent with no key should inherit whatever Codex login already exists on the device. **Steps to reproduce.** 1. Open a `codex_local` agent's configuration with `CODEX_HOME` and `OPENAI_API_KEY` set. 2. Remove both env vars and save. 3. Re-open the config — both slots are back. Related (different approach): Refs #8399 (keeps per-agent isolation, fixes only the empty `OPENAI_API_KEY` slot), Refs #8272 (introduced the guard), Refs #8403 (managed-auth seeding into isolated homes). ## What Changed - `server/src/routes/agents.ts` — replaced `applyCodexLocalIsolationGuard` (+ `assertCodexLocalHomeIsNotShared` / `normalizeCodexLocalHomePath`) with `applyCodexLocalKeyIsolation`. A codex_local agent now receives **no** env overrides unless it explicitly sets `OPENAI_API_KEY`; only then is an isolated per-agent `CODEX_HOME` injected (and only if the agent has not set its own `CODEX_HOME`). Keyless agents fall back to the host Codex login at runtime. Dropped the now-unused `node:os` import and the shared-home rejection. - `server/src/__tests__/agent-adapter-validation-routes.test.ts` — updated the validation-route tests to assert env-var deletion persists, keyless agents get no overrides, and key-bearing agents still get an isolated `CODEX_HOME`. ## Verification ```bash cd server && npx vitest run src/__tests__/agent-adapter-validation-routes.test.ts ``` All 6 tests pass locally. Manually verified in the live UI: removing `CODEX_HOME` and `OPENAI_API_KEY` from a codex_local agent and saving now persists the deletion (slots no longer re-appear on reload). ## Risks - **Host-key leak for keyless agents on a host with `OPENAI_API_KEY` set.** Low/intended: the product direction here is that codex_local agents inherit the host's Codex login by default; per-account isolation is opt-in via a per-agent `OPENAI_API_KEY`, which then gets its own `CODEX_HOME`. - **Divergence from #8399.** That PR retains the per-agent isolation default and only stops injecting the empty key, so it does not address the `CODEX_HOME` re-injection half of this bug. This PR intentionally changes the default to host-login inheritance. Reviewers should pick one direction. - **No migration.** Existing agents that already store these slots are not auto-cleaned, but operators can now delete them and the deletion sticks. ## Model Used - Provider: Anthropic (Claude) - Model ID: `claude-opus-4-8` - Capabilities: tool use, code execution, extended reasoning, agentic workflow ## 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 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 - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
@@ -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<string, unknown>;
|
||||
const agentId = String(createInput.id);
|
||||
expect(agentId).toMatch(/^[0-9a-f-]{36}$/i);
|
||||
const adapterConfig = createInput.adapterConfig as Record<string, unknown>;
|
||||
const env = adapterConfig.env as Record<string, unknown>;
|
||||
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<string, unknown> | 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<string, unknown>;
|
||||
const adapterConfig = patch.adapterConfig as Record<string, unknown>;
|
||||
const env = adapterConfig.env as Record<string, unknown>;
|
||||
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<string, unknown> | 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<string, unknown>;
|
||||
const adapterConfig = patch.adapterConfig as Record<string, unknown>;
|
||||
const env = adapterConfig.env as Record<string, unknown>;
|
||||
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<string, unknown>;
|
||||
const adapterConfig = createInput.adapterConfig as Record<string, unknown>;
|
||||
const env = adapterConfig.env as Record<string, unknown>;
|
||||
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<string, unknown>;
|
||||
const agentId = String(createInput.id);
|
||||
const adapterConfig = createInput.adapterConfig as Record<string, unknown>;
|
||||
const env = adapterConfig.env as Record<string, unknown>;
|
||||
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 () => {
|
||||
|
||||
+22
-43
@@ -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<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
if (adapterType !== "codex_local") return adapterConfig;
|
||||
const env = asRecord(adapterConfig.env) ? { ...(adapterConfig.env as Record<string, unknown>) } : {};
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user