diff --git a/server/src/__tests__/agents-claude-oauth-binding.test.ts b/server/src/__tests__/agents-claude-oauth-binding.test.ts index ae8b6e5982..2c906dc644 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) -------- @@ -801,7 +818,7 @@ describeEmbeddedPostgres("agent service Claude OAuth binding claim", () => { async function seedParentAgent( parentScope: Scope, - options: { adapterType?: string; holdsFixedBinding?: boolean } = {}, + options: { adapterType?: string; holdsFixedBinding?: boolean; version?: number } = {}, ) { const [row] = await db .insert(agents) @@ -812,7 +829,14 @@ describeEmbeddedPostgres("agent service Claude OAuth binding claim", () => { status: "idle", adapterType: options.adapterType ?? "claude_local", adapterConfig: { - env: options.holdsFixedBinding === false ? {} : { CLAUDE_CODE_OAUTH_TOKEN: { ...FIXED_BINDING } }, + 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: {}, }) @@ -841,6 +865,40 @@ describeEmbeddedPostgres("agent service Claude OAuth binding claim", () => { 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 }); diff --git a/server/src/services/agents.ts b/server/src/services/agents.ts index a3b4dfb60d..1a75fd9d71 100644 --- a/server/src/services/agents.ts +++ b/server/src/services/agents.ts @@ -40,9 +40,10 @@ import { normalizeAgentPermissions } from "./agent-permissions.js"; import { REDACTED_EVENT_VALUE, sanitizeRecord } from "../redaction.js"; import { assertClaudeOAuthBindingInvariant, + claudeOAuthBindingsMatchExactly, claudeOAuthClaimRejectedError, CLAUDE_LOCAL_ADAPTER_TYPE, - isFixedClaudeOAuthBinding, + readClaudeOAuthBinding, secretService, type ClaudeOAuthBindingInvariantDecision, } from "./secrets.js"; @@ -569,12 +570,15 @@ export function agentService(db: Db) { * 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 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 already holds the exact fixed binding. The route - * derives this 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. + * 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 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 @@ -589,6 +593,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; @@ -610,6 +621,10 @@ export function agentService(db: Db) { } 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({ @@ -620,15 +635,13 @@ export function agentService(db: Db) { .from(agents) .where(eq(agents.id, parentId)) .then((rows) => rows[0] ?? null); - const parentAdapterConfig = parent && isPlainRecord(parent.adapterConfig) ? parent.adapterConfig : null; - const parentEnv = - parentAdapterConfig && isPlainRecord(parentAdapterConfig.env) ? parentAdapterConfig.env : null; - const parentBinding = parentEnv ? parentEnv.CLAUDE_CODE_OAUTH_TOKEN : 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 || - !isFixedClaudeOAuthBinding(parentBinding) + !claudeOAuthBindingsMatchExactly(parentBinding, childBinding) ) { throw claudeOAuthClaimRejectedError(); } @@ -886,6 +899,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]);