diff --git a/doc/execution-github-identity.md b/doc/execution-github-identity.md index e17eb63313..1832e9d0a1 100644 --- a/doc/execution-github-identity.md +++ b/doc/execution-github-identity.md @@ -28,6 +28,17 @@ An explicit dedicated-agent grant overrides personal selection. Revoked, disable Connection setup and permissions display: “This agent uses this GitHub account for everyone's work, instead of the person giving instructions.” +Multiple eligible connections for the same GitHub account are treated as one +identity, using GitHub's stable account ID rather than its login. The resolver +selects an available grant, preferring the newest authorization with a stable +ID tie-breaker. Duplicate eligibility includes an active credential record with +the correct owner, the OAuth access-token reference, and repository access +metadata. It keeps that grant's credential and connection policy together; +it does not combine repository access or bypass connection audiences. Distinct +accounts or unidentifiable duplicate grants remain ambiguous. Managed commands +print the redacted reason when GitHub access is unavailable, while unrelated +local operations can still proceed without credentials. + Run details show identity revisions and redacted GitHub results: responsible person, selected login when available, personal/dedicated source, and an unavailable reason. Tasks do not receive an additional identity indicator or takeover action. ## Deployment and verification diff --git a/packages/adapter-utils/src/github-launcher.test.ts b/packages/adapter-utils/src/github-launcher.test.ts index 0837c1ae72..00f3ed6815 100644 --- a/packages/adapter-utils/src/github-launcher.test.ts +++ b/packages/adapter-utils/src/github-launcher.test.ts @@ -11,6 +11,25 @@ const cleanups: Array<() => Promise> = []; afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); describe("managed GitHub launchers", () => { + it("explains unavailable access while allowing local work without credentials", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-diagnostic-")); + cleanups.push(() => rm(root, {recursive:true,force:true})); + const bin = path.join(root,"managed"), realBin = path.join(root,"real"); + await mkdir(bin); await mkdir(realBin); + await writeFile(path.join(bin,"gh"), githubLauncherSource(), {mode:0o700}); + await writeFile(path.join(realBin,"gh"), '#!/usr/bin/env node\nprocess.stdout.write(JSON.stringify({token:process.env.GH_TOKEN ?? null}));', {mode:0o700}); + const server = createServer((_req,res) => { + res.setHeader("content-type","application/json"); + res.end(JSON.stringify({status:"unavailable",reason:"More than one managed GitHub identity matches this run",env:{GH_TOKEN:"must-not-be-used"}})); + }); + await new Promise(resolve => server.listen(0,"127.0.0.1",resolve)); + cleanups.push(() => new Promise((resolve,reject) => server.close(error => error ? reject(error) : resolve()))); + const {port} = server.address() as {port:number}; + const result = await exec(path.join(bin,"gh"), [], {env:{...process.env,...githubBrokerEnvironment({GH_TOKEN:"host-token"},{url:`http://127.0.0.1:${port}`,token:"run-capability"}),PATH:`${bin}:${realBin}:${process.env.PATH}`}}); + expect(JSON.parse(result.stdout)).toEqual({token:null}); + expect(result.stderr).toContain("More than one managed GitHub identity matches this run"); + expect(result.stderr).not.toMatch(/host-token|must-not-be-used|run-capability/); + }); it("captures each command's identity and clears host credentials when the next person has none", async () => { const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-launcher-test-")); cleanups.push(() => rm(root, { recursive: true, force: true })); diff --git a/packages/adapter-utils/src/github-launcher.ts b/packages/adapter-utils/src/github-launcher.ts index 1e825bcc2b..b84234b861 100644 --- a/packages/adapter-utils/src/github-launcher.ts +++ b/packages/adapter-utils/src/github-launcher.ts @@ -56,6 +56,12 @@ async function main() { } if (!response.ok) throw new Error('GitHub credential context unavailable; retry this operation'); const result = await response.json(); + if (result.status === 'unavailable') { + const reason = typeof result.reason === 'string' + ? result.reason.replace(/[\x00-\x1f\x7f]/g, ' ').slice(0, 500) + : 'Check the GitHub connection in Paperclip'; + process.stderr.write('Paperclip: GitHub access unavailable: ' + reason + '. Continuing without GitHub credentials.\n'); + } if (result.status === 'available') { for (const [key, value] of Object.entries(result.env || {})) { if (/^(GH_TOKEN|GITHUB_TOKEN|PAPERCLIP_GIT_TOKEN|GIT_TERMINAL_PROMPT|GIT_AUTHOR_(NAME|EMAIL)|GIT_COMMITTER_(NAME|EMAIL)|GIT_CONFIG_COUNT|GIT_CONFIG_(KEY|VALUE)_\d+)$/.test(key) && typeof value === 'string') env[key] = value; diff --git a/server/src/__tests__/github-operation-credentials.test.ts b/server/src/__tests__/github-operation-credentials.test.ts index 5787ea59c3..44b3efd1fe 100644 --- a/server/src/__tests__/github-operation-credentials.test.ts +++ b/server/src/__tests__/github-operation-credentials.test.ts @@ -10,6 +10,7 @@ import { agents, companies, companyMemberships, companySecrets, connectionGrants import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { initializeRunIdentity, reserveSteeredIdentity, acceptSteeredIdentity } from "../services/run-identity.js"; import { resolveGitHubOperationCredentials } from "../services/github-operation-credentials.js"; +import { filterResolvedGitHubConnectionsForRun, resolveManagedGitHubIdentitySelection } from "../services/git-credentials.js"; const vault = vi.hoisted(() => ({ resolveUserSecretValue: vi.fn(async (_company: string, input: { responsibleUserId: string }) => ({ value: `test-token-${input.responsibleUserId}` })), @@ -37,9 +38,9 @@ const support = await getEmbeddedPostgresTestSupport(); await db.insert(toolConnections).values({id:connectionId,companyId:input.companyId,applicationId,name:connectionId,uid:connectionId,transport:"mcp_remote",status:"active",enabled:true,credentialPolicy:dedicated?"per_agent":"per_user",config:{sourceTemplateKey:"github"}}); await db.insert(toolConnectionInstalls).values({companyId:input.companyId,connectionId,targetType:"agent",targetId:input.agentId}); if (!dedicated) await db.insert(userSecretDefinitions).values({id:definitionId,companyId:input.companyId,key:definitionId,name:"Test GitHub"}); - await db.insert(companySecrets).values({id:secretId,companyId:input.companyId,key:secretId,name:"Test token",scope:dedicated?"company":"user",ownerUserId:dedicated?null:user,userSecretDefinitionId:dedicated?null:definitionId}); + await db.insert(companySecrets).values({id:secretId,companyId:input.companyId,key:secretId,name:`Test token ${secretId}`,scope:dedicated?"company":"user",ownerUserId:dedicated?null:user,userSecretDefinitionId:dedicated?null:definitionId}); await db.insert(connectionGrants).values({id,companyId:input.companyId,connectionId,kind:dedicated?"agent":"user",subjectUserId:dedicated?null:user,subjectAgentId:dedicated?input.agentId:null,status:"active",credentialSecretRefs:[{secretId,configPath:"oauth.access_token",versionSelector:"latest"}],providerTenant:{github:{userId:user,login:user,installationCount:1,repositoryCount:1,repositorySelection:"selected",installationIds:["1"],installationOwnerLogins:[user]}}}); - return {id,connectionId}; + return {id,connectionId,secretId,definitionId}; } async function switchTo(input: Awaited>, user:string) { const id=randomUUID(); @@ -67,10 +68,71 @@ const support = await getEmbeddedPostgresTestSupport(); await db.update(companyMemberships).set({status:"inactive"}).where(eq(companyMemberships.companyId,input.companyId)); expect((await resolveGitHubOperationCredentials(db,input)).status).toBe("unavailable"); await db.update(companyMemberships).set({status:"active"}).where(eq(companyMemberships.companyId,input.companyId)); - await grant(input,"A"); + const differentAccount = await grant(input,"A"); + await db.update(connectionGrants).set({providerTenant:{github:{userId:"other-github-id",login:"A",installationCount:1,repositoryCount:1,repositorySelection:"selected",installationIds:["1"],installationOwnerLogins:["A"]}}}).where(eq(connectionGrants.id,differentAccount.id)); expect((await resolveGitHubOperationCredentials(db,input)).reason).toMatch(/More than one/); await expect(resolveGitHubOperationCredentials(db,{...input,companyId:randomUUID()})).rejects.toThrow(); }); + it("uses one stable grant when the same person connects the same GitHub account twice", async () => { + const input = await seed(); + const first = await grant(input, "A"); + const second = await grant(input, "A"); + await db.update(connectionGrants).set({createdAt:new Date("2026-01-01"),updatedAt:new Date("2027-01-01")}).where(eq(connectionGrants.id,first.id)); + await db.update(connectionGrants).set({createdAt:new Date("2026-02-01")}).where(eq(connectionGrants.id,second.id)); + const context = {...input,responsibleUserId:"A"}; + expect((await resolveManagedGitHubIdentitySelection(db,input.companyId,context)).grant?.id).toBe(second.id); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"available",login:"A",source:"personal"}); + const connections = [first,second].map(row => ({id:row.connectionId,config:{sourceTemplateKey:"github"}})); + expect(await filterResolvedGitHubConnectionsForRun({db,...context,connections})).toEqual([connections[1]]); + // A newer webhook on the old connection must not change the selected policy. + await db.update(connectionGrants).set({updatedAt:new Date("2028-01-01")}).where(eq(connectionGrants.id,first.id)); + expect((await resolveManagedGitHubIdentitySelection(db,input.companyId,context)).grant?.id).toBe(second.id); + await db.update(connectionGrants).set({status:"revoked"}).where(eq(connectionGrants.id,second.id)); + expect((await resolveManagedGitHubIdentitySelection(db,input.companyId,context)).grant?.id).toBe(first.id); + await db.update(toolConnections).set({enabled:false}).where(eq(toolConnections.id,first.connectionId)); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"unavailable",env:{}}); + }); + it("does not conflate missing GitHub account IDs or another agent's connection audience", async () => { + const input = await seed(); + const first = await grant(input,"A"); + const duplicate = await grant(input,"A"); + await db.update(connectionGrants).set({providerTenant:null}).where(eq(connectionGrants.id,duplicate.id)); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"unavailable",env:{}}); + await db.update(toolConnectionInstalls).set({targetId:randomUUID()}).where(eq(toolConnectionInstalls.connectionId,duplicate.connectionId)); + expect((await resolveManagedGitHubIdentitySelection(db,input.companyId,{...input,responsibleUserId:"A"})).grant?.id).toBe(first.id); + await switchTo(input,"B"); + expect((await resolveGitHubOperationCredentials(db,input)).env).toEqual({}); + }); + it.each(["missing-ref", "disabled-secret", "missing-secret", "wrong-owner", "disabled-definition", "no-repositories"])( + "ignores an incomplete newer duplicate when the same account has an eligible grant (%s)", async (problem) => { + const input = await seed(); + const first = await grant(input,"A"); + const second = await grant(input,"A"); + await db.update(connectionGrants).set({createdAt:new Date("2026-01-01")}).where(eq(connectionGrants.id,first.id)); + await db.update(connectionGrants).set({createdAt:new Date("2026-02-01")}).where(eq(connectionGrants.id,second.id)); + if (problem === "missing-ref") await db.update(connectionGrants).set({credentialSecretRefs:[]}).where(eq(connectionGrants.id,second.id)); + if (problem === "disabled-secret") await db.update(companySecrets).set({status:"disabled"}).where(eq(companySecrets.id,second.secretId)); + if (problem === "disabled-definition") await db.update(userSecretDefinitions).set({status:"disabled"}).where(eq(userSecretDefinitions.id,second.definitionId)); + if (problem === "missing-secret") await db.delete(companySecrets).where(eq(companySecrets.id,second.secretId)); + if (problem === "wrong-owner") await db.update(companySecrets).set({ownerUserId:"B"}).where(eq(companySecrets.id,second.secretId)); + if (problem === "no-repositories") await db.update(connectionGrants).set({providerTenant:{github:{userId:"A",login:"A",installationCount:0,repositoryCount:0,repositorySelection:"none",installationIds:[],installationOwnerLogins:[]}}}).where(eq(connectionGrants.id,second.id)); + expect((await resolveManagedGitHubIdentitySelection(db,input.companyId,{...input,responsibleUserId:"A"})).grant?.id).toBe(first.id); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"available",login:"A"}); + const connections = [first,second].map(row => ({id:row.connectionId,config:{sourceTemplateKey:"github"}})); + expect(await filterResolvedGitHubConnectionsForRun({db,...input,responsibleUserId:"A",connections})).toEqual([connections[0]]); + }, + ); + it("retains dedicated override semantics when the dedicated account has duplicate grants", async () => { + const input = await seed(); + await grant(input,"A"); + const first = await grant(input,"robot",true); + const second = await grant(input,"robot",true); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"available",source:"dedicated",login:"robot"}); + await db.update(connectionGrants).set({status:"revoked"}).where(eq(connectionGrants.id,first.id)); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"available",source:"dedicated",login:"robot"}); + await db.update(connectionGrants).set({status:"revoked"}).where(eq(connectionGrants.id,second.id)); + expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"unavailable",source:"dedicated",env:{}}); + }); it("honors dedicated overrides and never substitutes personal credentials when revoked or disabled", async () => { const input=await seed(); await grant(input,"A"); const dedicated=await grant(input,"robot",true); expect(await resolveGitHubOperationCredentials(db,input)).toMatchObject({status:"available",source:"dedicated",login:"robot"}); diff --git a/server/src/services/git-credentials.ts b/server/src/services/git-credentials.ts index a16cc138ec..5b4bb6b8ab 100644 --- a/server/src/services/git-credentials.ts +++ b/server/src/services/git-credentials.ts @@ -6,6 +6,7 @@ import { connectionGrants, toolConnectionInstalls, toolConnections, + userSecretDefinitions, type Db, } from "@paperclipai/db"; import { and, eq, inArray, or } from "drizzle-orm"; @@ -354,7 +355,14 @@ export async function resolveManagedGitHubIdentitySelection( : []; const candidates = dedicated.length > 0 ? dedicated : personal.length > 0 ? personal : delegated; const identitySource = dedicated.length > 0 ? "dedicated" as const : "personal" as const; - if (candidates.length !== 1) { + // Reconnecting can create another connection/grant for the same GitHub + // account. Ambiguity is about provider identities, not the number of rows. + // Only trust GitHub's stable account ID; equal logins or missing metadata + // cannot establish that two grants belong to the same person. + const githubUserIds = candidates.map((candidate) => candidate.providerTenant?.github?.userId?.trim()); + if (candidates.length === 0 || (candidates.length > 1 && ( + githubUserIds.some((id) => !id) || new Set(githubUserIds).size !== 1 + ))) { return { configured: true, identitySource, error: candidates.length === 0 @@ -362,7 +370,44 @@ export async function resolveManagedGitHubIdentitySelection( : "More than one managed GitHub identity matches this run", }; } - const grant = candidates[0]!; + const credentialIds = candidates.flatMap((grant) => grant.credentialSecretRefs + .filter((ref) => ref.configPath === "oauth.access_token").map((ref) => ref.secretId)); + const credentialRecords = candidates.length > 1 && credentialIds.length > 0 + ? await db.select({ + id: companySecrets.id, status: companySecrets.status, deletedAt: companySecrets.deletedAt, + scope: companySecrets.scope, ownerUserId: companySecrets.ownerUserId, + definitionStatus: userSecretDefinitions.status, definitionDeletedAt: userSecretDefinitions.deletedAt, + }).from(companySecrets).leftJoin(userSecretDefinitions, and( + eq(userSecretDefinitions.id, companySecrets.userSecretDefinitionId), + eq(userSecretDefinitions.companyId, companyId), + )).where(and( + eq(companySecrets.companyId, companyId), inArray(companySecrets.id, credentialIds), + )) + : []; + const hasCredentialRecord = (grant: typeof connectionGrants.$inferSelect) => { + if (candidates.length === 1) return true; + const github = grant.providerTenant?.github; + const ref = grant.credentialSecretRefs.find((ref) => ref.configPath === "oauth.access_token"); + return Boolean(github && github.installationCount > 0 && github.repositoryCount > 0 && ref + && credentialRecords.some((secret) => secret.id === ref.secretId + && secret.status === "active" && !secret.deletedAt + && (grant.kind === "user" + ? secret.scope === "user" && secret.ownerUserId === grant.subjectUserId + && secret.definitionStatus === "active" && !secret.definitionDeletedAt + : secret.scope === "company"))); + }; + const isAvailable = (grant: typeof connectionGrants.$inferSelect) => + grant.status === "active" && hasCredentialRecord(grant) && githubConnections.some((connection) => + connection.id === grant.connectionId && connection.enabled && connection.status === "active", + ); + // Prefer an available authorization for this same account, then the newest + // connection grant. Do not rank by updatedAt: refreshes/webhooks change it. + // Select one grant, preserving its credential and connection policy intact. + const grant = [...candidates].sort((a, b) => + Number(isAvailable(b)) - Number(isAvailable(a)) + || b.createdAt.getTime() - a.createdAt.getTime() + || a.id.localeCompare(b.id), + )[0]!; const connection = githubConnections.find((candidate) => candidate.id === grant.connectionId); if (!connection?.enabled || connection.status !== "active") { return { configured: true, identitySource, error: "The managed GitHub connection is unavailable" };