diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index a91503b9c9..240ce43bfc 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -1619,6 +1619,35 @@ describe("shared ACPX engine runtime behavior", () => { }, ); + it.each(["OPENAI_API_KEY", "CODEX_API_KEY"] as const)( + "selects Codex ACP API-key authentication when only the host process provides %s", + async (apiKeyName) => { + const root = await makeTempRoot(); + const codexHome = path.join(root, "codex-home"); + await fs.mkdir(codexHome, { recursive: true }); + + // Simulate a local launch that inherits a provider key from the host + // process environment. No adapter config sets the key directly, so the + // launched env only receives it through host projection. + vi.stubEnv(apiKeyName, "sk-host-inherited-key"); + try { + const { sessionInputs } = await runExecutor({ + agent: "codex", + stateDir: path.join(root, "state"), + env: { CODEX_HOME: codexHome }, + paperclipRuntimeSkills: [], + paperclipSkillSync: { desiredSkills: [] }, + }); + + const env = (sessionInputs[0]!.sessionOptions as { env: Record }).env; + expect(env[apiKeyName]).toBe("sk-host-inherited-key"); + expect(env.DEFAULT_AUTH_REQUEST).toBe(JSON.stringify({ methodId: "api-key" })); + } finally { + vi.unstubAllEnvs(); + } + }, + ); + it("busts the session fingerprint when resolved adapter env changes but not across wakes", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index e61bf94ae2..d60e81de80 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -1968,17 +1968,6 @@ async function buildRuntime(input: { // are absent from tempKeysApplied and keep their compatibility protection. if (!scratchKeys.has(key) || value !== scratch.dir) resolvedAdapterEnv[key] = value; } - // codex-acp supports both key names, but ACP clients must select its - // api-key authentication method during session creation. Without this - // request, the server advertises authentication and rejects session/new even - // though the credential is present in the launched process environment. - if ( - acpxAgent === "codex" && - (env.OPENAI_API_KEY || env.CODEX_API_KEY) && - !env.DEFAULT_AUTH_REQUEST - ) { - env.DEFAULT_AUTH_REQUEST = JSON.stringify({ methodId: "api-key" }); - } if (authToken) env.PAPERCLIP_API_KEY = authToken; // For the claude agent, set model via ANTHROPIC_MODEL at startup rather than // via session/set_config_option — the ACP server's set_config_option handler @@ -2635,11 +2624,25 @@ function resolveRuntimeEnv( env, (options.platform ?? process.platform) === "win32", ); - return Object.fromEntries( + const finalEnv = Object.fromEntries( Object.entries(mergedEnv).filter( (entry): entry is [string, string] => typeof entry[1] === "string", ), ); + // codex-acp supports both key names, but ACP clients must select its + // api-key authentication method during session creation. Without this + // request, the server advertises authentication and rejects session/new even + // though the credential is present in the launched process environment. Check + // the final merged environment, not just the explicit run config, so a host + // key the local launch inherits still selects this default. + if ( + acpxAgent === "codex" && + (finalEnv.OPENAI_API_KEY || finalEnv.CODEX_API_KEY) && + !finalEnv.DEFAULT_AUTH_REQUEST + ) { + finalEnv.DEFAULT_AUTH_REQUEST = JSON.stringify({ methodId: "api-key" }); + } + return finalEnv; } function mergeRuntimeEnvironment( diff --git a/server/src/__tests__/agent-hire-auth-inheritance-routes.test.ts b/server/src/__tests__/agent-hire-auth-inheritance-routes.test.ts new file mode 100644 index 0000000000..ef03ac91eb --- /dev/null +++ b/server/src/__tests__/agent-hire-auth-inheritance-routes.test.ts @@ -0,0 +1,503 @@ +import { randomUUID } from "node:crypto"; +import { mkdirSync, rmSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import express from "express"; +import request from "supertest"; +import { and, eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + activityLog, + agents, + companies, + companyMemberships, + companySecretBindings, + companySecretProviderConfigs, + companySecretVersions, + companySecrets, + createDb, + principalPermissionGrants, + userSecretDeclarations, + userSecretDefinitions, +} from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { errorHandler } from "../middleware/index.js"; +import { agentRoutes } from "../routes/agents.js"; +import { secretService } from "../services/secrets.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping agent-hire credential inheritance route tests on this host: ${ + embeddedPostgresSupport.reason ?? "unsupported environment" + }`, + ); +} + +type Db = ReturnType; + +// The fixed Claude Code OAuth binding. It is a user-secret reference to the +// fixed key. Any other shape is a replacement or a weaker binding. +const FIXED_CLAUDE_OAUTH_BINDING = { type: "user_secret_ref", key: "CLAUDE_CODE_OAUTH_TOKEN" } as const; + +function agentActor(companyId: string, agentId: string): Express.Request["actor"] { + return { type: "agent", agentId, companyId, source: "agent_jwt" }; +} + +// The local implicit board actor always passes authorization, so it stands in +// for a user actor without needing a seeded company membership. +function userActor(): Express.Request["actor"] { + return { type: "board", userId: "local-board", source: "local_implicit" }; +} + +function createApp(db: Db, actor: Express.Request["actor"]) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + app.use("/api", agentRoutes(db)); + app.use(errorHandler); + return app; +} + +describeEmbeddedPostgres("hired agent provider credential inheritance", () => { + let db!: Db; + let tempDb: Awaited> | null = null; + const previousKeyFile = process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE; + const secretsTmpDir = path.join(os.tmpdir(), `paperclip-agent-hire-auth-inheritance-${randomUUID()}`); + + beforeAll(async () => { + mkdirSync(secretsTmpDir, { recursive: true }); + process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE = path.join(secretsTmpDir, "master.key"); + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-agent-hire-auth-inheritance-"); + db = createDb(tempDb.connectionString); + }, 60_000); + + afterEach(async () => { + await db.delete(activityLog); + await db.delete(userSecretDeclarations); + await db.delete(userSecretDefinitions); + await db.delete(companySecretBindings); + await db.delete(companySecretVersions); + await db.delete(companySecrets); + await db.delete(companySecretProviderConfigs); + await db.delete(principalPermissionGrants); + await db.delete(companyMemberships); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + if (previousKeyFile === undefined) { + delete process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE; + } else { + process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE = previousKeyFile; + } + rmSync(secretsTmpDir, { recursive: true, force: true }); + }); + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: `Auth Inheritance Co ${companyId.slice(0, 8)}`, + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + return companyId; + } + + async function seedParentAgent(companyId: string, adapterType: string, env: Record) { + const [row] = await db + .insert(agents) + .values({ + companyId, + name: `Parent ${randomUUID().slice(0, 8)}`, + role: "engineer", + adapterType, + adapterConfig: { env }, + runtimeConfig: {}, + permissions: { canCreateAgents: true }, + }) + .returning(); + return row!; + } + + async function createCompanySecret(companyId: string, value: string) { + return secretService(db).create(companyId, { + name: `cred-${randomUUID()}`, + provider: "local_encrypted", + value, + }); + } + + function secretRef(secretId: string, extra: Record = {}) { + return { type: "secret_ref" as const, secretId, ...extra }; + } + + function hire(actor: Express.Request["actor"], companyId: string, payload: Record) { + const app = createApp(db, actor); + return request(app).post(`/api/companies/${companyId}/agent-hires`).send(payload); + } + + function childEnvOf(res: request.Response): Record { + return ((res.body.agent?.adapterConfig as { env?: Record } | undefined)?.env) ?? {}; + } + + it("inherits the parent secret_ref for a codex_local hire with no env, and derives the child binding", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "sk-openai-parent"); + const parent = await seedParentAgent(companyId, "codex_local", { + OPENAI_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Codex Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).OPENAI_API_KEY).toMatchObject({ type: "secret_ref", secretId: secret.id }); + + const bindings = await db + .select() + .from(companySecretBindings) + .where( + and( + eq(companySecretBindings.companyId, companyId), + eq(companySecretBindings.targetType, "agent"), + eq(companySecretBindings.targetId, res.body.agent.id), + ), + ); + expect(bindings).toHaveLength(1); + expect(bindings[0]).toMatchObject({ + configPath: "env.OPENAI_API_KEY", + secretId: secret.id, + versionSelector: "latest", + }); + }); + + it("keeps a pinned version selector on the copied reference and its derived binding", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "sk-openai-pinned"); + const parent = await seedParentAgent(companyId, "codex_local", { + OPENAI_API_KEY: secretRef(secret.id, { version: 3 }), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Codex Pinned Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).OPENAI_API_KEY).toMatchObject({ type: "secret_ref", secretId: secret.id, version: 3 }); + + const bindings = await db + .select() + .from(companySecretBindings) + .where( + and( + eq(companySecretBindings.companyId, companyId), + eq(companySecretBindings.targetId, res.body.agent.id), + ), + ); + expect(bindings[0]?.versionSelector).toBe("3"); + }); + + it("retains version, required, and allowMissingOverride on a copied user_secret_ref", async () => { + const companyId = await seedCompany(); + await secretService(db).createUserSecretDefinition(companyId, { + key: "openai_shared_key", + name: "Shared OpenAI key", + provider: "local_encrypted", + }); + const parent = await seedParentAgent(companyId, "codex_local", { + OPENAI_API_KEY: { + type: "user_secret_ref", + key: "openai_shared_key", + version: 2, + required: false, + allowMissingOverride: true, + }, + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Codex User Secret Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).OPENAI_API_KEY).toMatchObject({ + type: "user_secret_ref", + key: "openai_shared_key", + version: 2, + required: false, + allowMissingOverride: true, + }); + }); + + it("inherits a grok_local parent's XAI_API_KEY reference", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "xai-parent-key"); + const parent = await seedParentAgent(companyId, "grok_local", { + XAI_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Grok Child", + role: "engineer", + adapterType: "grok_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).XAI_API_KEY).toMatchObject({ type: "secret_ref", secretId: secret.id }); + }); + + it("inherits a claude_local parent's ANTHROPIC_API_KEY reference", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "ant-parent-key"); + const parent = await seedParentAgent(companyId, "claude_local", { + ANTHROPIC_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Claude Child", + role: "engineer", + adapterType: "claude_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).ANTHROPIC_API_KEY).toMatchObject({ type: "secret_ref", secretId: secret.id }); + }); + + it("keeps the child-supplied ANTHROPIC_API_KEY and inherits no Claude credential at all", async () => { + const companyId = await seedCompany(); + const childSecret = await createCompanySecret(companyId, "ant-child-key"); + // The parent holds the fixed Claude OAuth binding, which is normally + // inheritable, but the child already supplies its own Claude credential. + const parent = await seedParentAgent(companyId, "claude_local", { + CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_CLAUDE_OAUTH_BINDING }, + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Claude Child With Own Key", + role: "engineer", + adapterType: "claude_local", + adapterConfig: { env: { ANTHROPIC_API_KEY: secretRef(childSecret.id) } }, + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + const childEnv = childEnvOf(res); + expect(childEnv.ANTHROPIC_API_KEY).toMatchObject({ type: "secret_ref", secretId: childSecret.id }); + expect(childEnv.CLAUDE_CODE_OAUTH_TOKEN).toBeUndefined(); + }); + + it("does not inherit a plain environment value", async () => { + const companyId = await seedCompany(); + const parent = await seedParentAgent(companyId, "grok_local", { + XAI_API_KEY: "xai-plain-value", + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Grok Plain Value Child", + role: "engineer", + adapterType: "grok_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).XAI_API_KEY).toBeUndefined(); + }); + + it("does not inherit across a mismatched adapter type", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "sk-openai-mismatch"); + const parent = await seedParentAgent(companyId, "codex_local", { + OPENAI_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Claude Child From Codex Parent", + role: "engineer", + adapterType: "claude_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(Object.keys(childEnvOf(res))).toHaveLength(0); + }); + + it("does not inherit a codex_local credential from a hiring agent in another company", async () => { + const parentCompanyId = await seedCompany(); + const targetCompanyId = await seedCompany(); + const secret = await createCompanySecret(parentCompanyId, "sk-openai-other-company"); + const parent = await seedParentAgent(parentCompanyId, "codex_local", { + OPENAI_API_KEY: secretRef(secret.id), + }); + + // The actor claims the target company, but the named hiring agent belongs + // to a different company. The route must reject the request and must + // never copy the other company's credential reference. + const res = await hire(agentActor(targetCompanyId, parent.id), targetCompanyId, { + name: "Codex Cross-Company Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status).toBe(403); + const children = await db.select().from(agents).where(eq(agents.companyId, targetCompanyId)); + expect(children).toHaveLength(0); + }); + + it("does not inherit a grok_local credential from a hiring agent in another company", async () => { + const parentCompanyId = await seedCompany(); + const targetCompanyId = await seedCompany(); + const secret = await createCompanySecret(parentCompanyId, "xai-other-company"); + const parent = await seedParentAgent(parentCompanyId, "grok_local", { + XAI_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(targetCompanyId, parent.id), targetCompanyId, { + name: "Grok Cross-Company Child", + role: "engineer", + adapterType: "grok_local", + }); + + expect(res.status).toBe(403); + const children = await db.select().from(agents).where(eq(agents.companyId, targetCompanyId)); + expect(children).toHaveLength(0); + }); + + it("does not inherit a claude_local credential, including the fixed OAuth binding, from a hiring agent in another company", async () => { + const parentCompanyId = await seedCompany(); + const targetCompanyId = await seedCompany(); + const parent = await seedParentAgent(parentCompanyId, "claude_local", { + ANTHROPIC_API_KEY: secretRef((await createCompanySecret(parentCompanyId, "ant-other-company")).id), + CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_CLAUDE_OAUTH_BINDING, version: 1 }, + }); + + const res = await hire(agentActor(targetCompanyId, parent.id), targetCompanyId, { + name: "Claude Cross-Company Child", + role: "engineer", + adapterType: "claude_local", + }); + + expect(res.status).toBe(403); + const children = await db.select().from(agents).where(eq(agents.companyId, targetCompanyId)); + expect(children).toHaveLength(0); + }); + + it("carves out a per-agent CODEX_HOME when the child inherits OPENAI_API_KEY", async () => { + const companyId = await seedCompany(); + const secret = await createCompanySecret(companyId, "sk-openai-isolated"); + const parent = await seedParentAgent(companyId, "codex_local", { + OPENAI_API_KEY: secretRef(secret.id), + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Codex Isolated Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + const childEnv = childEnvOf(res); + // A bare environment string persists as a `plain` binding. + const codexHome = childEnv.CODEX_HOME as { type: string; value: string } | undefined; + expect(codexHome?.type).toBe("plain"); + expect(codexHome?.value.length ?? 0).toBeGreaterThan(0); + }); + + it("keeps the shared company Codex home when the child inherits no key", async () => { + const companyId = await seedCompany(); + const parent = await seedParentAgent(companyId, "codex_local", {}); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Codex Shared Home Child", + role: "engineer", + adapterType: "codex_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(childEnvOf(res).CODEX_HOME).toBeUndefined(); + }); + + it("adds no home override for a grok_local child with no inherited key", async () => { + const companyId = await seedCompany(); + const parent = await seedParentAgent(companyId, "grok_local", {}); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Grok No Key Child", + role: "engineer", + adapterType: "grok_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + const childEnv = childEnvOf(res); + expect(childEnv.GROK_HOME).toBeUndefined(); + expect(childEnv.XAI_API_KEY).toBeUndefined(); + }); + + it("inherits nothing for a user actor hire", async () => { + const companyId = await seedCompany(); + // No hiring-agent context exists for a user actor. The merge must not run + // at all, so an otherwise-inheritable key is simply never considered. + const res = await hire(userActor(), companyId, { + name: "User Hired Child", + role: "engineer", + adapterType: "claude_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + expect(Object.keys(childEnvOf(res))).toHaveLength(0); + }); + + it("inherits the fixed Claude OAuth binding, keeps its version, and leaks no token value", async () => { + const companyId = await seedCompany(); + const parent = await seedParentAgent(companyId, "claude_local", { + CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_CLAUDE_OAUTH_BINDING, version: 3 }, + }); + + const res = await hire(agentActor(companyId, parent.id), companyId, { + name: "Claude OAuth Child", + role: "engineer", + adapterType: "claude_local", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(201); + const childAgentId = res.body.agent.id as string; + expect(childEnvOf(res).CLAUDE_CODE_OAUTH_TOKEN).toMatchObject({ + type: "user_secret_ref", + key: "CLAUDE_CODE_OAUTH_TOKEN", + version: 3, + }); + // The response body carries the reference only, never a token value. + expect(JSON.stringify(res.body)).not.toContain("sk-"); + + const declarations = await db + .select() + .from(userSecretDeclarations) + .where(eq(userSecretDeclarations.targetId, childAgentId)); + expect(declarations).toHaveLength(1); + expect(declarations[0]).toMatchObject({ + envKey: "CLAUDE_CODE_OAUTH_TOKEN", + configPath: "env.CLAUDE_CODE_OAUTH_TOKEN", + versionSelector: "3", + }); + + const hireActivity = await db + .select() + .from(activityLog) + .where(and(eq(activityLog.companyId, companyId), eq(activityLog.action, "agent.hire_created"))); + expect(hireActivity).toHaveLength(1); + const detailsText = JSON.stringify(hireActivity[0]!.details); + expect(detailsText).not.toContain("adapterConfig"); + expect(detailsText).not.toContain("CLAUDE_CODE_OAUTH_TOKEN"); + }); +}); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 06aee69cf8..fb76747a1b 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -1283,7 +1283,14 @@ describe.sequential("agent permission routes", () => { }, }, }), - { claudeLogin: { storedSessionId: null, ownerUserId: "board-user", applyExistingWithoutClaim: false } }, + { + claudeLogin: { + storedSessionId: null, + ownerUserId: "board-user", + applyExistingWithoutClaim: false, + inheritedFromAgentId: null, + }, + }, ); }); diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index 27bed85b00..cbf77aa7ad 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -1464,7 +1464,14 @@ describe.sequential("agent skill routes", () => { }), }), }), - { claudeLogin: { storedSessionId: null, ownerUserId: "local-board", applyExistingWithoutClaim: false } }, + { + claudeLogin: { + storedSessionId: null, + ownerUserId: "local-board", + applyExistingWithoutClaim: false, + inheritedFromAgentId: null, + }, + }, ); expect(mockApprovalService.create).toHaveBeenCalledWith( "company-1", diff --git a/server/src/__tests__/agents-claude-oauth-binding.test.ts b/server/src/__tests__/agents-claude-oauth-binding.test.ts index 90ae922e3c..19d496b945 100644 --- a/server/src/__tests__/agents-claude-oauth-binding.test.ts +++ b/server/src/__tests__/agents-claude-oauth-binding.test.ts @@ -27,6 +27,7 @@ import { assertClaudeOAuthBindingInvariant, CLAUDE_OAUTH_CLAIM_REJECTED, CLAUDE_OAUTH_CREDENTIAL_CONFLICT, + claudeOAuthBindingsMatchExactly, claudeOAuthClaimRejectedError, isFixedClaudeOAuthBinding, secretService, @@ -188,6 +189,22 @@ describe("assertClaudeOAuthBindingInvariant", () => { expect(error.status).toBe(409); expect(error.message).toBe(CLAUDE_OAUTH_CLAIM_REJECTED); }); + + it("matches two fixed bindings only when their version selectors are exactly equal", () => { + expect(claudeOAuthBindingsMatchExactly(FIXED_BINDING, FIXED_BINDING)).toBe(true); + expect( + claudeOAuthBindingsMatchExactly({ ...FIXED_BINDING, version: 5 }, { ...FIXED_BINDING, version: 5 }), + ).toBe(true); + expect( + claudeOAuthBindingsMatchExactly({ ...FIXED_BINDING, version: 5 }, { ...FIXED_BINDING, version: 2 }), + ).toBe(false); + expect( + claudeOAuthBindingsMatchExactly({ ...FIXED_BINDING, version: 5 }, { ...FIXED_BINDING, version: "latest" }), + ).toBe(false); + // Neither side needs the exact fixed shape only; both sides do. + expect(claudeOAuthBindingsMatchExactly(FIXED_BINDING, { type: "plain", value: "x" })).toBe(false); + expect(claudeOAuthBindingsMatchExactly(null, FIXED_BINDING)).toBe(false); + }); }); // --- The stored-session claim on the create and hire paths (Postgres) -------- @@ -797,6 +814,200 @@ describeEmbeddedPostgres("agent service Claude OAuth binding claim", () => { expect(JSON.stringify(definitions)).not.toContain("sk-secret-resolve"); }); + // --- The hire-inheritance path (no login round trip, no stored owner value) - + + async function seedParentAgent( + parentScope: Scope, + options: { adapterType?: string; holdsFixedBinding?: boolean; version?: number } = {}, + ) { + const [row] = await db + .insert(agents) + .values({ + companyId: parentScope.companyId, + name: `Parent ${randomUUID().slice(0, 8)}`, + role: "engineer", + status: "idle", + adapterType: options.adapterType ?? "claude_local", + adapterConfig: { + env: + options.holdsFixedBinding === false + ? {} + // A binding written through the normal persistence path always + // carries a resolved version, "latest" by default. Match that + // shape here, so only `options.version` simulates a pinned + // version, or a version change since the child copied it. + : { CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_BINDING, version: options.version ?? "latest" } }, + }, + runtimeConfig: {}, + }) + .returning(); + return row!; + } + + async function countDeclarationsForCompany(companyId: string): Promise { + const rows = await db + .select() + .from(userSecretDeclarations) + .where(eq(userSecretDeclarations.companyId, companyId)); + return rows.length; + } + + it("binds the inherited fixed reference from a named claude_local parent with no claim and no stored owner value", async () => { + const scope = await seedScope(); + const parent = await seedParentAgent(scope); + + const created = await agentService(db).create(scope.companyId, createInput(scope), { + claudeLogin: { inheritedFromAgentId: parent.id }, + }); + + const persisted = created.adapterConfig as { env: Record }; + expect(persisted.env.CLAUDE_CODE_OAUTH_TOKEN).toMatchObject(FIXED_BINDING); + expect(await countDeclarationsForAgent(created.id)).toBe(1); + }); + + it("binds the inherited reference when the child's copied version still matches the parent's current version", async () => { + const scope = await seedScope(); + const parent = await seedParentAgent(scope, { version: 5 }); + + const created = await agentService(db).create( + scope.companyId, + createInput(scope, { CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_BINDING, version: 5 } }), + { claudeLogin: { inheritedFromAgentId: parent.id } }, + ); + + const persisted = created.adapterConfig as { env: Record }; + expect(persisted.env.CLAUDE_CODE_OAUTH_TOKEN).toMatchObject({ ...FIXED_BINDING, version: 5 }); + expect(await countDeclarationsForAgent(created.id)).toBe(1); + }); + + it("rejects an inherited claim when the parent's version moved after the route copied the child's reference", async () => { + const scope = await seedScope(); + // The parent now holds version 5. The child's reference, copied before this + // transaction, still names version 2 — a concurrent parent rotation moved + // the parent's version between the copy and this write. + const parent = await seedParentAgent(scope, { version: 5 }); + + await expect( + agentService(db).create( + scope.companyId, + createInput(scope, { CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_BINDING, version: 2 } }), + { claudeLogin: { inheritedFromAgentId: parent.id } }, + ), + ).rejects.toMatchObject({ message: CLAUDE_OAUTH_CLAIM_REJECTED }); + // Only the seeded parent exists; the rejected create inserted no child. + expect(await countAgents(scope.companyId)).toBe(1); + expect(await countDeclarationsForCompany(scope.companyId)).toBe(0); + }); + + it("rejects an inherited claim when the named parent holds no fixed binding", async () => { + const scope = await seedScope(); + const parent = await seedParentAgent(scope, { holdsFixedBinding: false }); + + await expect( + agentService(db).create(scope.companyId, createInput(scope), { + claudeLogin: { inheritedFromAgentId: parent.id }, + }), + ).rejects.toMatchObject({ message: CLAUDE_OAUTH_CLAIM_REJECTED }); + // Only the seeded parent exists; the rejected create inserted no child. + expect(await countAgents(scope.companyId)).toBe(1); + expect(await countDeclarationsForCompany(scope.companyId)).toBe(0); + }); + + it("rejects an inherited claim naming a parent in another company", async () => { + const scope = await seedScope(); + const foreignScope = await seedScope(); + const parent = await seedParentAgent(foreignScope); + + await expect( + agentService(db).create(scope.companyId, createInput(scope), { + claudeLogin: { inheritedFromAgentId: parent.id }, + }), + ).rejects.toMatchObject({ message: CLAUDE_OAUTH_CLAIM_REJECTED }); + expect(await countAgents(scope.companyId)).toBe(0); + expect(await countDeclarationsForCompany(scope.companyId)).toBe(0); + expect(await countUserSecretDefinitions(scope.companyId)).toBe(0); + }); + + it("rejects an inherited claim naming an unknown or deleted parent", async () => { + const scope = await seedScope(); + + await expect( + agentService(db).create(scope.companyId, createInput(scope), { + claudeLogin: { inheritedFromAgentId: randomUUID() }, + }), + ).rejects.toMatchObject({ message: CLAUDE_OAUTH_CLAIM_REJECTED }); + expect(await countAgents(scope.companyId)).toBe(0); + expect(await countDeclarationsForCompany(scope.companyId)).toBe(0); + }); + + it("rejects an inherited claim naming a non-claude_local parent that still holds the fixed binding shape", async () => { + const scope = await seedScope(); + // The parent's stored env happens to carry the exact fixed-binding shape + // under a non-claude_local adapter type. Complete mediation requires the + // gate to check the adapter type itself, not only the binding shape. + const parent = await seedParentAgent(scope, { adapterType: "codex_local" }); + + await expect( + agentService(db).create(scope.companyId, createInput(scope), { + claudeLogin: { inheritedFromAgentId: parent.id }, + }), + ).rejects.toMatchObject({ message: CLAUDE_OAUTH_CLAIM_REJECTED }); + // Only the seeded parent exists; the rejected create inserted no child. + expect(await countAgents(scope.companyId)).toBe(1); + expect(await countDeclarationsForCompany(scope.companyId)).toBe(0); + }); + + it("rejects an inherited claim when a concurrent parent rotation commits while this create waits on the parent row lock", async () => { + const scope = await seedScope(); + const parent = await seedParentAgent(scope, { version: 5 }); + + const lockDb = createDb(connectionString); + let signalLocked!: () => void; + const locked = new Promise((resolve) => { + signalLocked = resolve; + }); + let releaseGate!: () => void; + const gate = new Promise((resolve) => { + releaseGate = resolve; + }); + // Simulate a credential rotation on the parent: it takes the row lock + // first, moves the bound version to 6, then holds the open transaction + // until the gate releases. + const rotationHeld = lockDb.transaction(async (tx) => { + await tx.execute(sql`SELECT id FROM agents WHERE id = ${parent.id} FOR UPDATE`); + await tx.execute( + sql`UPDATE agents SET adapter_config = jsonb_set(adapter_config, '{env,CLAUDE_CODE_OAUTH_TOKEN,version}', '6') WHERE id = ${parent.id}`, + ); + signalLocked(); + await gate; + }); + + await locked; + // The child's copied reference still names version 5, the version the + // route read before the rotation started. The create call must wait for + // the parent row lock, so it can only proceed once the rotation commits. + const createPromise = agentService(db) + .create( + scope.companyId, + createInput(scope, { CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_BINDING, version: 5 } }), + { claudeLogin: { inheritedFromAgentId: parent.id } }, + ) + .then(() => "created") + .catch((error: Error) => error.message); + await new Promise((resolve) => setTimeout(resolve, 300)); + releaseGate(); + await rotationHeld; + + // The create call reads the parent's committed post-rotation version, so + // it detects the mismatch against the child's stale copy and rejects the + // claim. Without the row lock, the create call could read the parent's + // pre-rotation version and bind the child to a reference the parent no + // longer holds. + expect(await createPromise).toBe(CLAUDE_OAUTH_CLAIM_REJECTED); + expect(await countAgents(scope.companyId)).toBe(1); + await lockDb.$client.end(); + }); + // --- The atomic credential-claim writer (item 2) --------------------------- function claimScope(scope: Scope): SetupTokenSessionScope { diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 1428fae7b8..3a3daa2bc8 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -102,7 +102,7 @@ import { evaluateCodexCredentialReadiness } from "@paperclipai/adapter-codex-loc import type { AdapterAuthSignal, AdapterAuthSignalResponse, CodexAccountBindingClaim } from "@paperclipai/shared"; import { getDisabledAdapterTypes } from "../services/adapter-plugin-store.js"; import { skillVersionSelectionMap } from "../services/runtime-skill-selections.js"; -import { secretService } from "../services/secrets.js"; +import { isFixedClaudeOAuthBinding, secretService } from "../services/secrets.js"; import { authorizationDeniedDetails } from "../services/authorization.js"; import { providerTraceStore } from "../services/provider-trace-store.js"; import { @@ -2493,6 +2493,80 @@ export function agentRoutes( }; } + // The provider credential environment keys a hired agent can inherit from the + // hiring agent, by adapter type. Each key holds a credential. A configuration + // key such as CODEX_HOME or GROK_HOME is a path, not a credential, and stays + // out of this list. + const INHERITABLE_AGENT_CREDENTIAL_ENV_KEYS: Record = { + claude_local: ["ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"], + codex_local: ["OPENAI_API_KEY", "CODEX_API_KEY"], + grok_local: ["XAI_API_KEY"], + }; + + function isInheritableCredentialReference(value: unknown): value is Record { + const record = asRecord(value); + return record !== null && (record.type === "secret_ref" || record.type === "user_secret_ref"); + } + + // A hired agent inherits the provider credential references the hiring + // agent already holds for the same adapter type, so a freshly hired agent + // can run without a separate credential setup step. The merge copies each + // reference object whole, so the child keeps the parent's pinned version + // and its other fields. A key the hire request already supplies always + // wins, and the merge never inherits a plain environment value. + // + // A claude_local hire request that already supplies any Claude credential + // key inherits no Claude credential key at all. That keeps child-wins + // precedence and rules out the forbidden pairing of the fixed OAuth binding + // with an ANTHROPIC_API_KEY. + // + // The hiring agent must belong to the target company. Without that check, an + // agent that can create agents in another company could copy its own + // company's credential reference into that other company. + async function applyHiringAgentAuthInheritance( + req: Request, + companyId: string, + adapterType: string | null | undefined, + adapterConfig: Record, + ): Promise<{ adapterConfig: Record; inheritedFixedClaudeOAuthBinding: boolean }> { + const noInheritance = { adapterConfig, inheritedFixedClaudeOAuthBinding: false }; + if (req.actor.type !== "agent" || !req.actor.agentId) return noInheritance; + const credentialKeys = adapterType ? INHERITABLE_AGENT_CREDENTIAL_ENV_KEYS[adapterType] : undefined; + if (!credentialKeys) return noInheritance; + + const parent = await svc.getById(req.actor.agentId); + if (!parent || parent.companyId !== companyId || parent.adapterType !== adapterType) return noInheritance; + const parentEnv = asRecord(asRecord(parent.adapterConfig)?.env); + if (!parentEnv) return noInheritance; + + const existingEnv = asRecord(adapterConfig.env); + const claudeCredentialKeys = INHERITABLE_AGENT_CREDENTIAL_ENV_KEYS.claude_local; + const childHasClaudeCredential = + adapterType === "claude_local" && + existingEnv !== null && + claudeCredentialKeys.some((key) => existingEnv[key] !== undefined); + if (childHasClaudeCredential) return noInheritance; + + const nextEnv: Record = { ...(existingEnv ?? {}) }; + let inheritedFixedClaudeOAuthBinding = false; + let changed = false; + for (const key of credentialKeys) { + if (existingEnv && existingEnv[key] !== undefined) continue; + const parentValue = parentEnv[key]; + if (!isInheritableCredentialReference(parentValue)) continue; + nextEnv[key] = { ...parentValue }; + changed = true; + if (key === "CLAUDE_CODE_OAUTH_TOKEN" && isFixedClaudeOAuthBinding(parentValue)) { + inheritedFixedClaudeOAuthBinding = true; + } + } + if (!changed) return noInheritance; + return { + adapterConfig: { ...adapterConfig, env: nextEnv }, + inheritedFixedClaudeOAuthBinding, + }; + } + function applyCreateDefaultsByAdapterType( adapterType: string | null | undefined, adapterConfig: Record, @@ -4288,15 +4362,21 @@ export function agentRoutes( ); assertNoAgentAdapterConfigMutation(req, rawHireAdapterConfig); const hiredAgentId = randomUUID(); - const requestedAdapterConfig = applyCodexLocalKeyIsolation( + const authInheritance = await applyHiringAgentAuthInheritance( + req, companyId, - hiredAgentId, hireInput.adapterType, applyCreateDefaultsByAdapterType( hireInput.adapterType, rawHireAdapterConfig, ), ); + const requestedAdapterConfig = applyCodexLocalKeyIsolation( + companyId, + hiredAgentId, + hireInput.adapterType, + authInheritance.adapterConfig, + ); assertExternalInstructionsAdmin(req, { id: hiredAgentId, companyId, @@ -4396,6 +4476,15 @@ export function agentRoutes( // from the actor, so an agent actor never reaches the no-claim bind. applyExistingWithoutClaim: req.actor.type !== "agent" && hireApplyStoredClaudeLogin === true, + // Set only when an agent actor hired this child and the merge above + // inherited the parent's fixed Claude OAuth reference. The service + // re-reads this named parent inside the write transaction before it + // permits the bind, so this identifier is a claim to verify, not a + // trusted value. + inheritedFromAgentId: + req.actor.type === "agent" && authInheritance.inheritedFixedClaudeOAuthBinding + ? req.actor.agentId + : null, }, }, ); diff --git a/server/src/services/agents.ts b/server/src/services/agents.ts index d380043fee..b0f8bd2ca3 100644 --- a/server/src/services/agents.ts +++ b/server/src/services/agents.ts @@ -41,8 +41,10 @@ import { normalizeAgentPermissions } from "./agent-permissions.js"; import { REDACTED_EVENT_VALUE, sanitizeRecord } from "../redaction.js"; import { assertClaudeOAuthBindingInvariant, + claudeOAuthBindingsMatchExactly, claudeOAuthClaimRejectedError, CLAUDE_LOCAL_ADAPTER_TYPE, + readClaudeOAuthBinding, secretService, type ClaudeOAuthBindingInvariantDecision, } from "./secrets.js"; @@ -98,12 +100,20 @@ interface RevisionMetadata { * from that actor. The path binds the fixed reference to the owner stored value * with no login round trip. It is distinct from `allowInternalBindingOverride`, * which does no ownership check. + * + * The `inheritedFromAgentId` field is the hire-inheritance path. The route + * sets it only for an authenticated agent actor whose hire request inherited + * the fixed reference from that named parent. The service re-reads the parent + * agent inside the write transaction and binds the fixed reference only when + * the parent exists, is in the same company, is a `claude_local` agent, and + * already holds the exact fixed binding. */ interface ClaudeLoginContext { storedSessionId?: string | null; ownerUserId?: string | null; allowInternalBindingOverride?: boolean; applyExistingWithoutClaim?: boolean; + inheritedFromAgentId?: string | null; } interface UpdateAgentOptions { @@ -559,6 +569,23 @@ export function agentService(db: Db) { * owner or a missing stored value raises the same fixed claim error, so the * caller cannot tell the reasons apart. * + * The hire-inheritance path (`inheritedFromAgentId`) binds the fixed + * reference with no login round trip and no stored owner value, because the + * owning user resolves per run, not from a value stored against this agent. + * The route copies the parent's reference onto the child before this + * transaction starts, so a concurrent version change on the parent can + * leave the child holding a stale version. The gate re-reads the named + * parent agent inside this transaction and permits the bind only when the + * parent exists, is in the same company, is a `claude_local` agent, and its + * current reference matches the child's copied reference exactly, including + * the version selector. The gate locks the parent row with `SELECT ... + * FOR UPDATE` before it reads the reference. The lock blocks a concurrent + * credential rotation on the same parent row until this transaction + * commits or rolls back, so the compare-and-bind check stays atomic with + * the parent's current state. The route derives the parent identifier + * from the authenticated agent actor, never from the request body, so the + * gate treats it as a claim to verify, not a trusted value. + * * A controlled internal override skips the claim for a migration or an * administrator repair. The function creates the fixed user-secret definition * before the caller runs the declaration synchronization, so the synchronized @@ -572,6 +599,13 @@ export function agentService(db: Db) { consume: boolean; environmentId: string | null; claudeLogin?: ClaudeLoginContext; + /** + * The adapter config the write is about to persist. The + * `inheritedFromAgentId` path reads the child's copied + * `CLAUDE_CODE_OAUTH_TOKEN` reference from it, to compare against the + * parent's current reference. + */ + childAdapterConfig?: unknown; }, ): Promise { const ownerUserId = input.claudeLogin?.ownerUserId ?? null; @@ -590,6 +624,34 @@ export function agentService(db: Db) { if (!stored) { throw claudeOAuthClaimRejectedError(); } + } else if (input.claudeLogin?.inheritedFromAgentId) { + // The hire-inheritance path. Re-read the named parent inside this + // transaction; a caller-supplied identifier never binds on its own. + // Compare the parent's current reference against the reference + // already copied onto the child, including the version selector, so + // a concurrent version change on the parent cannot leave the child + // bound to a stale version. + const parentId = input.claudeLogin.inheritedFromAgentId; + const parent = await txDb + .select({ + companyId: agents.companyId, + adapterType: agents.adapterType, + adapterConfig: agents.adapterConfig, + }) + .from(agents) + .where(eq(agents.id, parentId)) + .for("update") + .then((rows) => rows[0] ?? null); + const parentBinding = readClaudeOAuthBinding(parent?.adapterConfig ?? null); + const childBinding = readClaudeOAuthBinding(input.childAdapterConfig ?? null); + if ( + !parent || + parent.companyId !== input.companyId || + parent.adapterType !== CLAUDE_LOCAL_ADAPTER_TYPE || + !claudeOAuthBindingsMatchExactly(parentBinding, childBinding) + ) { + throw claudeOAuthClaimRejectedError(); + } } else if (!input.consume) { throw claudeOAuthClaimRejectedError(); } else { @@ -844,6 +906,7 @@ export function agentService(db: Db) { consume: true, environmentId: (data.defaultEnvironmentId as string | null | undefined) ?? null, claudeLogin: options?.claudeLogin, + childAdapterConfig: adapterConfig, }); const created = await tx .insert(agents) diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index dd6fa92712..c5facc4964 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -190,6 +190,31 @@ export function isFixedClaudeOAuthBinding(binding: unknown): boolean { return record.type === "user_secret_ref" && record.key === CLAUDE_CODE_OAUTH_TOKEN_KEY; } +/** + * Reads the `CLAUDE_CODE_OAUTH_TOKEN` binding from an adapter config, or + * `null` when the config carries no such key. + */ +export function readClaudeOAuthBinding(config: unknown): unknown { + const value = readAdapterEnvRecord(config)[CLAUDE_CODE_OAUTH_TOKEN_KEY]; + return value === undefined ? null : value; +} + +/** + * Returns true when both bindings are the exact fixed Claude Code OAuth + * reference and select the exact same secret version. The hire-inheritance + * gate compares the parent's current reference, re-read inside the write + * transaction, against the reference already copied onto the child before the + * transaction started. A concurrent version change on the parent must fail + * this check, so the child never keeps a stale version under a claim the gate + * treats as current. + */ +export function claudeOAuthBindingsMatchExactly(parentBinding: unknown, childBinding: unknown): boolean { + if (!isFixedClaudeOAuthBinding(parentBinding) || !isFixedClaudeOAuthBinding(childBinding)) return false; + const parentVersion = (parentBinding as Record).version; + const childVersion = (childBinding as Record).version; + return parentVersion === childVersion; +} + /** True when the config carries the exact fixed OAuth binding. */ function hasFixedClaudeOAuthBinding(config: unknown): boolean { return isFixedClaudeOAuthBinding(readAdapterEnvRecord(config)[CLAUDE_CODE_OAUTH_TOKEN_KEY]);