From d2d647c34b02a4391e64dbe29b176be101bf90a3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 5 Sep 2026 10:58:12 -0500 Subject: [PATCH] fix(connections): honor identity after reconnect (#12897) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app that people use to manage AI agents for work. > - Connections give agents access to external services with a selected credential identity. > - A removed connection keeps its database row so Paperclip can retain its history. > - A fresh GitHub setup can select a different identity from the removed connection. > - The retained row incorrectly kept its old credential policy after that new selection. > - The GitHub callback then could not save the new grant and returned the user to setup. > - This pull request applies the explicit identity selection when Paperclip revives an archived row. > - The benefit is a successful GitHub reconnect after the user changes from a dedicated agent account to a personal account. ## Linked Issues or Issue Description **What happened?** After a user removed a dedicated-agent GitHub connection, a fresh setup with “My GitHub account” returned to the setup page with `oauth=failed`. The Cloud claim succeeded, but the local connection still used the old `per_agent` policy. **Expected behavior** A fresh setup must apply the explicit identity choice. An interrupted draft or an explicit reconnect must keep its existing identity. **Steps to reproduce** 1. Connect GitHub with a dedicated agent identity. 2. Remove the connection. 3. Start a fresh GitHub connection with “My GitHub account.” 4. Complete GitHub OAuth. 5. Observe that Paperclip returns to the setup page instead of the permissions page. **Paperclip version or commit** `342c01fee` **Deployment mode** Local dev (`pnpm dev`) with embedded Postgres and the staging managed connector. **Additional context** This follows the GitHub access UI change in #12893. ## What Changed - Apply an explicit Access identity when a fresh gallery setup revives an archived connection row. - Preserve the identity for interrupted drafts and explicit reconnects. - Do not carry credential material across an identity change. - Apply the omitted organization default during a fresh archived-row recovery. - Restore the prior grants and credential policy transactionally if a revived setup rolls back. - Disable the connection and surface a specific failure if that restoration cannot complete. - Preserve newer concurrent grant changes with a row lock and optimistic version check. - Preserve newer concurrent connection identity/configuration changes with a locked state fingerprint. - Add regressions for dedicated-to-personal OAuth, organization-default recovery, rollback, rollback failure, and concurrent grant/connection changes. ## Verification - `pnpm exec vitest run server/src/__tests__/tool-access-service.test.ts` — 222 tests passed. - `pnpm --filter @paperclipai/server typecheck` — passed. - Browser proof on an isolated local instance: dedicated connection removed, personal setup selected, staging GitHub OAuth completed, permissions page opened, connection reported active and healthy, personal grant active, old agent grant revoked. ## Risks - Low migration risk. This change has no schema migration. - The behavior changes only when a fresh setup explicitly selects an identity for an archived connection row. - Existing draft resume and explicit reconnect behavior stays unchanged. - Connection-manager checks still protect changes to a retained credential identity. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5, with reasoning, browser control, tool use, and code execution. ## 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 (e.g. `docs/...`, `fix/...`) 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 - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../src/__tests__/tool-access-service.test.ts | 317 +++++++++++++++++- server/src/services/tool-access.ts | 277 ++++++++++++--- 2 files changed, 543 insertions(+), 51 deletions(-) diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 5963fc4105..a5709c0fd2 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -5231,6 +5231,113 @@ describeEmbeddedPostgres("tool access service", () => { } }); + it("replaces an archived dedicated GitHub identity with an explicitly selected personal identity", async () => { + const company = await createCompany(db); + const userId = `github-personal-revival-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const agent = await createAgent(db, company.id); + const connector = fakeGitHubConnector(company.id, `agent:${agent.id}`); + const originalClaim = connector.claim; + connector.claim = vi.fn(async (input) => ({ + ...await originalClaim(input), + subject: input.subject, + })); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const actor = { actorType: "user" as const, actorId: userId }; + const githubDefinition = getConnectableAppDefinition("github")!; + const previousOwnershipAvailability = githubDefinition.ownershipAvailability; + githubDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + if (href === "https://api.github.com/user") { + return mcpHttpResponse({ id: 42, login: "octocat" }); + } + if (href.includes("https://api.github.com/user/installations?")) { + return mcpHttpResponse({ installations: [{ + id: 101, + repository_selection: "selected", + html_url: "https://github.com/settings/installations/101", + account: { login: "paperclipai" }, + }] }); + } + if (href.includes("https://api.github.com/user/installations/101/repositories?")) { + return mcpHttpResponse({ total_count: 1, repositories: [] }); + } + if (href === GITHUB_CONNECTOR_PROFILES["github.code"].serverUrl) { + return mcpHttpResponse({ + jsonrpc: "2.0", + id: "paperclip-catalog-refresh", + result: { tools: [{ name: "get_pull_request", annotations: { readOnlyHint: true } }] }, + }); + } + throw new Error(`unexpected fetch ${href}`); + }); + + try { + const dedicated = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "managed", + grantKind: "agent", + subjectAgentId: agent.id, + name: "GitHub", + }, actor); + const dedicatedStart = await service.startOAuth(company.id, dedicated.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + subjectAgentId: agent.id, + }); + await service.completePaperclipCloudConnectorCallback({ + state: new URL(dedicatedStart.authorizationUrl).searchParams.get("state")!, + claimId: "github-dedicated-before-removal", + actor, + }); + await service.archiveConnection(dedicated.connectionId, company.id, actor); + + const personal = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "managed", + grantKind: "user", + name: "GitHub", + }, actor); + expect(personal.connectionId).toBe(dedicated.connectionId); + expect(personal.connection).toMatchObject({ + status: "draft", + credentialPolicy: "per_user", + }); + + const personalStart = await service.startOAuth(company.id, personal.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const completed = await service.completePaperclipCloudConnectorCallback({ + state: new URL(personalStart.authorizationUrl).searchParams.get("state")!, + claimId: "github-personal-after-removal", + actor, + }); + expect(completed.connection).toMatchObject({ + status: "active", + credentialPolicy: "per_user", + }); + + const grants = await service.listConnectionGrants(personal.connectionId, company.id); + expect(grants.grants).toEqual(expect.arrayContaining([ + expect.objectContaining({ + kind: "agent", + subjectAgentId: agent.id, + status: "revoked", + }), + expect.objectContaining({ + kind: "user", + subjectUserId: userId, + status: "active", + }), + ])); + expect(grants.grants.some((grant) => grant.kind === "organization")).toBe(false); + } finally { + githubDefinition.ownershipAvailability = previousOwnershipAvailability; + } + }); + it("routes a managed Drive callback into the personal vault, filtered catalog, and provider-specific activity", async () => { const company = await createCompany(db); const userId = `drive-member-${randomUUID()}`; @@ -9057,7 +9164,7 @@ describeEmbeddedPostgres("tool access service", () => { }, { actorType: "user", actorId: "board" })).rejects.toMatchObject({ status: 404 }); }); - it("reuses and revives a removed gallery app without requiring its applicationId", async () => { + it("reuses a removed gallery app while applying the omitted organization identity default", async () => { const company = await createCompany(db); const service = createTestToolAccessService(db); const actor = { actorType: "user" as const, actorId: "local-board" }; @@ -9078,11 +9185,217 @@ describeEmbeddedPostgres("tool access service", () => { expect(second.connectionId).toBe(first.connectionId); expect(second.application.status).toBe("draft"); expect(second.connection.status).toBe("draft"); - expect(second.connection.credentialPolicy).toBe("per_user"); + expect(second.connection.credentialPolicy).toBe("shared"); + const grants = await service.listConnectionGrants(second.connectionId, company.id); + expect(grants.grants).toEqual([ + expect.objectContaining({ kind: "organization", status: "active", isDefault: true }), + ]); await expect(db.select().from(toolApplications)).resolves.toHaveLength(1); await expect(db.select().from(toolConnections)).resolves.toHaveLength(1); }); + it("restores grants and credential policy when an identity-changing revival fails", async () => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + const actor = { + actorType: "user" as const, + actorId: "local-board", + actorSource: "local_implicit" as const, + }; + const fetchMock = mockToolsList([ + { name: "get_file_contents", annotations: { readOnlyHint: true } }, + ]); + + const first = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "organization", + name: "GitHub rollback", + credentialValues: { "credentials.authorization": "old-organization-token" }, + }, actor); + await service.archiveConnection(first.connectionId, company.id, actor); + const beforeConnection = await service.getConnection(first.connectionId, company.id); + const beforeGrants = await service.listConnectionGrants(first.connectionId, company.id); + fetchMock.mockRejectedValue(new Error("provider unavailable")); + + await expect(service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "user", + name: "GitHub rollback", + credentialValues: { "credentials.authorization": "new-personal-token" }, + }, actor)).rejects.toMatchObject({ status: 502 }); + + await expect(service.getConnection(first.connectionId, company.id)).resolves.toMatchObject({ + status: beforeConnection.status, + credentialPolicy: beforeConnection.credentialPolicy, + credentialSecretRefs: beforeConnection.credentialSecretRefs, + }); + const afterGrants = await service.listConnectionGrants(first.connectionId, company.id); + expect(afterGrants.grants).toEqual(beforeGrants.grants); + }); + + it("preserves a concurrent grant change when an identity-changing revival fails", async () => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + const actor = { + actorType: "user" as const, + actorId: "local-board", + actorSource: "local_implicit" as const, + }; + const fetchMock = mockToolsList([ + { name: "get_file_contents", annotations: { readOnlyHint: true } }, + ]); + + const first = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "organization", + name: "GitHub concurrent rollback", + credentialValues: { "credentials.authorization": "old-organization-token" }, + }, actor); + await service.archiveConnection(first.connectionId, company.id, actor); + fetchMock.mockImplementation(async () => { + const [personalGrant] = await db.select().from(connectionGrants).where(and( + eq(connectionGrants.connectionId, first.connectionId), + eq(connectionGrants.kind, "user"), + )).limit(1); + expect(personalGrant).toBeTruthy(); + const concurrentUpdateAt = new Date(Date.now() + 2_000); + await db.update(connectionGrants).set({ + status: "revoked", + credentialSecretRefs: [], + revokedAt: concurrentUpdateAt, + updatedAt: concurrentUpdateAt, + }).where(eq(connectionGrants.id, personalGrant!.id)); + throw new Error("provider unavailable"); + }); + + await expect(service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "user", + name: "GitHub concurrent rollback", + credentialValues: { "credentials.authorization": "new-personal-token" }, + }, actor)).rejects.toMatchObject({ status: 502 }); + + await expect(service.getConnection(first.connectionId, company.id)).resolves.toMatchObject({ + status: "archived", + credentialPolicy: "shared", + }); + const afterGrants = await service.listConnectionGrants(first.connectionId, company.id); + expect(afterGrants.grants).toEqual(expect.arrayContaining([ + expect.objectContaining({ kind: "organization", status: "revoked" }), + expect.objectContaining({ kind: "user", status: "revoked", credentialSecretRefs: [] }), + ])); + }); + + it("preserves a concurrent connection update when an identity-changing revival fails", async () => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + const actor = { + actorType: "user" as const, + actorId: "local-board", + actorSource: "local_implicit" as const, + }; + const fetchMock = mockToolsList([ + { name: "get_file_contents", annotations: { readOnlyHint: true } }, + ]); + + const first = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "organization", + name: "GitHub concurrent connection rollback", + credentialValues: { "credentials.authorization": "old-organization-token" }, + }, actor); + await service.archiveConnection(first.connectionId, company.id, actor); + fetchMock.mockImplementation(async () => { + const concurrentUpdateAt = new Date(Date.now() + 2_000); + const [connection] = await db.select().from(toolConnections).where(eq( + toolConnections.id, + first.connectionId, + )); + await db.update(toolConnections).set({ + status: "active", + enabled: true, + config: { ...connection.config, concurrentOAuthCompletion: true }, + transportConfig: { ...connection.transportConfig, concurrentOAuthCompletion: true }, + updatedAt: concurrentUpdateAt, + }).where(eq(toolConnections.id, first.connectionId)); + throw new Error("provider unavailable"); + }); + + await expect(service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "user", + name: "GitHub concurrent connection rollback", + credentialValues: { "credentials.authorization": "new-personal-token" }, + }, actor)).rejects.toMatchObject({ status: 502 }); + + await expect(service.getConnection(first.connectionId, company.id)).resolves.toMatchObject({ + status: "active", + enabled: true, + credentialPolicy: "per_user", + config: expect.objectContaining({ concurrentOAuthCompletion: true }), + }); + const afterGrants = await service.listConnectionGrants(first.connectionId, company.id); + const personalGrant = afterGrants.grants.find((grant) => grant.kind === "user"); + expect(personalGrant).toMatchObject({ status: "active" }); + expect(personalGrant?.credentialSecretRefs).toHaveLength(1); + const [preservedSecret] = await db.select().from(companySecrets).where(eq( + companySecrets.id, + personalGrant!.credentialSecretRefs[0]!.secretId, + )); + expect(preservedSecret.deletedAt).toBeNull(); + }); + + it("fails closed when an identity-changing revival cannot roll back", async () => { + const company = await createCompany(db); + const service = createTestToolAccessService(db); + const actor = { + actorType: "user" as const, + actorId: "local-board", + actorSource: "local_implicit" as const, + }; + const fetchMock = mockToolsList([ + { name: "get_file_contents", annotations: { readOnlyHint: true } }, + ]); + + const first = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "organization", + name: "GitHub rollback failure", + credentialValues: { "credentials.authorization": "old-organization-token" }, + }, actor); + await service.archiveConnection(first.connectionId, company.id, actor); + fetchMock.mockRejectedValue(new Error("provider unavailable")); + const runTransaction = db.transaction.bind(db); + vi.spyOn(db, "transaction") + .mockImplementationOnce(runTransaction) + .mockRejectedValueOnce(new Error("rollback unavailable")); + + await expect(service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "mcp-key", + grantKind: "user", + name: "GitHub rollback failure", + credentialValues: { "credentials.authorization": "new-personal-token" }, + }, actor)).rejects.toMatchObject({ + status: 500, + details: { code: "connection_identity_rollback_failed" }, + }); + + await expect(service.getConnection(first.connectionId, company.id)).resolves.toMatchObject({ + status: "draft", + enabled: false, + healthStatus: "error", + lastError: "connection_identity_rollback_failed", + }); + }); + it("automatically gives same-named connections distinct names", async () => { const company = await createCompany(db); const service = createTestToolAccessService(db); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 6d55ae9357..284b3510f2 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -1769,6 +1769,22 @@ function stableHash(value: unknown): string { return createHash("sha256").update(JSON.stringify(value, Object.keys(flattenKeys(value)).sort())).digest("hex"); } +function connectionSetupMutationFingerprint(row: typeof toolConnections.$inferSelect): string { + return stableHash({ + name: row.name, + transport: row.transport, + status: row.status, + enabled: row.enabled, + config: row.config, + transportConfig: row.transportConfig, + credentialRefs: row.credentialRefs, + credentialSecretRefs: row.credentialSecretRefs, + credentialSource: row.credentialSource, + externalCredential: row.externalCredential, + credentialPolicy: row.credentialPolicy, + }); +} + function flattenKeys(value: unknown, keys: Record = {}): Record { if (value && typeof value === "object") { for (const [key, nested] of Object.entries(value as Record)) { @@ -3897,6 +3913,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} async function ensureDefaultOrganizationGrant( connection: typeof toolConnections.$inferSelect, dbClient: ToolAccessMutationDb = db, + onMutation?: (mutation: { + previous: typeof connectionGrants.$inferSelect | null; + current: typeof connectionGrants.$inferSelect; + }) => void, ) { const [existing] = await dbClient .select() @@ -3929,6 +3949,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} .where(eq(connectionGrants.id, existing.id)) .returning(); if (!updated) throw new Error("Failed to update default connection grant"); + onMutation?.({ previous: existing, current: updated }); return updated; } const [created] = await dbClient @@ -3943,6 +3964,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }) .returning(); if (!created) throw new Error("Failed to create default connection grant"); + onMutation?.({ previous: null, current: created }); return created; } @@ -8830,13 +8852,29 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} )); name = nextAvailableConnectionName(requestedName, connectionNames.map((row) => row.name)); } - const retainedGrantKind: ConnectionGrantKind | null = retainedConnection + const previousGrantKind: ConnectionGrantKind | null = retainedConnection ? retainedConnection.credentialPolicy === "per_user" ? "user" : retainedConnection.credentialPolicy === "per_agent" ? "agent" : "organization" : null; + // An explicit resume/application reconnect continues the retained identity. + // A fresh gallery connect may still reuse an archived row for stable history + // and company-unique naming, but an explicit Access choice is a new identity + // decision and must replace the archived policy. Without this distinction, + // removing a dedicated-agent connection and reconnecting the default personal + // account leaves `per_agent` behind and the OAuth callback cannot persist its + // user grant. + const retainsIdentity = Boolean( + retainedConnection + && ( + retainedConnection.status === "draft" + || requestedResumeConnection + || input.applicationId + ) + ); + const retainedGrantKind = retainsIdentity ? previousGrantKind : null; // The route can authorize an explicit resume before entering the service, // but name/source recovery happens here. Do not let a caller submit a // personal grant choice to pass the route and then inherit an implicitly @@ -8844,8 +8882,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // unrestricted instance operator; every authenticated user must still hold // current connection-manager authority before this retained row is touched. if ( - retainedGrantKind === "organization" - && input.grantKind === "user" + previousGrantKind + && input.grantKind + && previousGrantKind !== input.grantKind && actor?.actorType === "user" && actor.actorSource !== "local_implicit" ) { @@ -8868,7 +8907,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} eq(principalPermissionGrants.permissionKey, "tools:manage_connections"), )).limit(1); if (!roleCanManage && !explicitManagerGrant) { - throw forbidden("Only a company owner, admin, or member with connection-manager permission can share credentials with the organization."); + throw forbidden("Only a company owner, admin, or member with connection-manager permission can change this connection's credential identity."); } } const requestedGrantKind = retainedGrantKind ?? input.grantKind ?? "organization"; @@ -9069,6 +9108,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // or API key, without carrying credentials across a method/provider change. const canRetainCredentialMaterial = Boolean( retainedConnection + && previousGrantKind === requestedGrantKind && galleryEntry && retainedSource === galleryEntry.slug && retainedMethodKey === method?.key, @@ -9076,17 +9116,19 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const retainedCredentialSecretRefs = canRetainCredentialMaterial ? (retainedPersonalIdentity?.grant?.credentialSecretRefs ?? retainedConnection?.credentialSecretRefs ?? []) : []; - // Only the personal path changes the policy; every existing gallery app keeps - // the shared default it has today. - const credentialPolicy: ToolConnectionCredentialPolicy | undefined = personalIdentityUserId + const credentialPolicy: ToolConnectionCredentialPolicy = requestedGrantKind === "user" ? "per_user" - : dedicatedAgentId + : requestedGrantKind === "agent" ? "per_agent" - : undefined; + : "shared"; const connectionOwnership = isPaperclipCloudConnectorStrategy(method?.oauthStrategy) ? "platform_shared" : "customer"; let applicationRow: typeof toolApplications.$inferSelect | null = null; let connectionRow: typeof toolConnections.$inferSelect | null = null; let revivedConnectionPrevious: typeof toolConnections.$inferSelect | null = retainedConnection ?? null; + let revivedGrantMutation: { + previous: typeof connectionGrants.$inferSelect | null; + current: typeof connectionGrants.$inferSelect; + } | null = null; try { const credentialFields = credentialSource === "vercel_connect" @@ -9273,9 +9315,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} credentialSecretRefs: connectionCredentialSecretRefs, credentialSource, externalCredential, - // Identity is immutable for a retained connection. A fresh - // connection still derives it from the explicit Access choice. - credentialPolicy: revivedConnectionPrevious.credentialPolicy, + credentialPolicy, updatedAt: new Date(), }).where(eq(toolConnections.id, revivedConnectionPrevious.id)).returning(); } else { @@ -9298,7 +9338,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} transportConfig: config, credentialRefs, credentialSecretRefs: connectionCredentialSecretRefs, - ...(credentialPolicy ? { credentialPolicy } : {}), + credentialPolicy, createdByAgentId: actor?.actorType === "agent" ? actor.actorId ?? null : null, createdByUserId: actor?.actorType === "user" ? actor.actorId ?? null : null, }).returning(); @@ -9323,17 +9363,29 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // "Connected" with nothing behind it, so the grant is left to the // callback and only the organization grant is suppressed. if (credentialSecretRefs.length > 0) { + let changedGrant: typeof connectionGrants.$inferSelect; + let previousGrant: typeof connectionGrants.$inferSelect | null = null; if (retainedPersonalIdentity?.grant) { - await db.update(connectionGrants).set({ + const [currentGrant] = await db.select().from(connectionGrants).where(eq( + connectionGrants.id, + retainedPersonalIdentity.grant.id, + )).limit(1); + if (!currentGrant) throw conflict("The personal credential changed during setup. Please try again."); + previousGrant = currentGrant; + [changedGrant] = await db.update(connectionGrants).set({ credentialSecretRefs, status: "active", revokedAt: null, revokedByAgentId: null, revokedByUserId: null, updatedAt: new Date(), - }).where(eq(connectionGrants.id, retainedPersonalIdentity.grant.id)); + }).where(and( + eq(connectionGrants.id, currentGrant.id), + eq(connectionGrants.updatedAt, currentGrant.updatedAt), + )).returning(); + if (!changedGrant) throw conflict("The personal credential changed during setup. Please try again."); } else { - await db.insert(connectionGrants).values({ + [changedGrant] = await db.insert(connectionGrants).values({ companyId, connectionId: connectionRow.id, kind: "user", @@ -9342,7 +9394,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} status: "active", isDefault: false, createdByUserId: personalIdentityUserId, - }); + }).returning(); + if (!changedGrant) throw new Error("Failed to create personal connection grant"); + } + if (revivedConnectionPrevious) { + revivedGrantMutation = { previous: previousGrant, current: changedGrant }; } await db.insert(toolAccessAuditEvents).values({ companyId, @@ -9361,7 +9417,15 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // Managed OAuth creates the credential-bearing grant in the callback. // Keep the connection free of organization secrets from the outset. } else { - await ensureDefaultOrganizationGrant(connectionRow); + const organizationGrant = await ensureDefaultOrganizationGrant( + connectionRow, + db, + revivedConnectionPrevious + ? (mutation) => { + revivedGrantMutation = mutation; + } + : undefined, + ); if (credentialSource === "vercel_connect") { const derived = deriveVercelConnectSubject({ credential: externalCredential!, @@ -9369,20 +9433,37 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} companyId, grantKind: "organization", }); - await db.update(connectionGrants).set({ - externalCredential: { - provider: "vercel_connect", - subjectType: externalCredential!.principalMode, - ...(derived.subjectId ? { subjectId: derived.subjectId } : {}), - }, - credentialSecretRefs: [], - updatedAt: now(), - }).where(and( - eq(connectionGrants.companyId, companyId), - eq(connectionGrants.connectionId, connectionRow.id), - eq(connectionGrants.kind, "organization"), - eq(connectionGrants.isDefault, true), - )); + const updatedOrganizationGrant = await db.transaction(async (tx) => { + const [lockedGrant] = await tx.select().from(connectionGrants).where(eq( + connectionGrants.id, + organizationGrant.id, + )).limit(1).for("update"); + if ( + !lockedGrant + || lockedGrant.updatedAt.getTime() !== organizationGrant.updatedAt.getTime() + ) { + throw conflict("The organization credential changed during setup. Please try again."); + } + const [updated] = await tx.update(connectionGrants).set({ + externalCredential: { + provider: "vercel_connect", + subjectType: externalCredential!.principalMode, + ...(derived.subjectId ? { subjectId: derived.subjectId } : {}), + }, + credentialSecretRefs: [], + updatedAt: now(), + }).where(eq(connectionGrants.id, organizationGrant.id)).returning(); + return updated; + }); + if (!updatedOrganizationGrant) { + throw conflict("The organization credential changed during setup. Please try again."); + } + if (revivedGrantMutation) { + revivedGrantMutation = { + previous: revivedGrantMutation.previous, + current: updatedOrganizationGrant, + }; + } } } await syncCredentialBindings(connectionRow, personalIdentityUserId || dedicatedAgentId ? credentialSecretRefs : []); @@ -9472,33 +9553,131 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }, }; } catch (error) { + let identityRollbackError: unknown = null; + let preserveConcurrentRevival = false; if (connectionRow && revivedConnectionPrevious) { - await db.update(toolConnections).set({ - name: revivedConnectionPrevious.name, - transport: revivedConnectionPrevious.transport, - status: revivedConnectionPrevious.status, - enabled: revivedConnectionPrevious.enabled, - config: revivedConnectionPrevious.config, - transportConfig: revivedConnectionPrevious.transportConfig, - credentialRefs: revivedConnectionPrevious.credentialRefs, - credentialSecretRefs: revivedConnectionPrevious.credentialSecretRefs, - credentialSource: revivedConnectionPrevious.credentialSource, - externalCredential: revivedConnectionPrevious.externalCredential, - updatedAt: new Date(), - }).where(eq(toolConnections.id, revivedConnectionPrevious.id)).catch(() => undefined); + const attemptedConnection = connectionRow; + try { + await db.transaction(async (tx) => { + const [latestConnection] = await tx.select().from(toolConnections).where(and( + eq(toolConnections.id, revivedConnectionPrevious.id), + eq(toolConnections.companyId, companyId), + )).limit(1).for("update"); + // Health/catalog failures from this setup may update only health + // fields and `updatedAt`, so compare the identity/configuration + // fields this attempt owned. If another request changed any of + // those fields, its newer connection state is authoritative. + const connectionMutationIsStillCurrent = Boolean( + latestConnection + && connectionSetupMutationFingerprint(latestConnection) + === connectionSetupMutationFingerprint(attemptedConnection), + ); + if (connectionMutationIsStillCurrent) { + await tx.update(toolConnections).set({ + name: revivedConnectionPrevious.name, + transport: revivedConnectionPrevious.transport, + status: revivedConnectionPrevious.status, + enabled: revivedConnectionPrevious.enabled, + config: revivedConnectionPrevious.config, + transportConfig: revivedConnectionPrevious.transportConfig, + credentialRefs: revivedConnectionPrevious.credentialRefs, + credentialSecretRefs: revivedConnectionPrevious.credentialSecretRefs, + credentialSource: revivedConnectionPrevious.credentialSource, + externalCredential: revivedConnectionPrevious.externalCredential, + credentialPolicy: revivedConnectionPrevious.credentialPolicy, + updatedAt: new Date(), + }).where(eq(toolConnections.id, revivedConnectionPrevious.id)); + } else { + preserveConcurrentRevival = true; + } + + if (connectionMutationIsStillCurrent && revivedGrantMutation) { + const { previous, current } = revivedGrantMutation; + const [latestGrant] = await tx.select().from(connectionGrants).where(eq( + connectionGrants.id, + current.id, + )).limit(1).for("update"); + const mutationIsStillCurrent = Boolean( + latestGrant + && latestGrant.updatedAt.getTime() === current.updatedAt.getTime(), + ); + // A grant manager may have changed this grant while provider + // setup was in flight. Restore/delete only the exact version this + // attempt wrote; a newer version is authoritative and remains + // untouched. + if (previous && mutationIsStillCurrent) { + await tx.update(connectionGrants).set({ + kind: previous.kind, + subjectUserId: previous.subjectUserId, + subjectAgentId: previous.subjectAgentId, + providerTenant: previous.providerTenant, + credentialSecretRefs: previous.credentialSecretRefs, + externalCredential: previous.externalCredential, + status: previous.status, + isDefault: previous.isDefault, + createdByAgentId: previous.createdByAgentId, + createdByUserId: previous.createdByUserId, + revokedAt: previous.revokedAt, + revokedByAgentId: previous.revokedByAgentId, + revokedByUserId: previous.revokedByUserId, + lastUsedAt: previous.lastUsedAt, + updatedAt: previous.updatedAt, + }).where(eq(connectionGrants.id, current.id)); + } else if (!previous && mutationIsStillCurrent) { + await tx.delete(connectionGrants).where(eq(connectionGrants.id, current.id)); + } + } + }); + } catch (rollbackError) { + identityRollbackError = rollbackError; + // The attempted identity and its grants may no longer agree. Keep the + // connection unusable until a manager explicitly reconnects it, and + // surface the restoration failure instead of returning only the + // original provider error. + try { + await db.update(toolConnections).set({ + status: "draft", + enabled: false, + healthStatus: "error", + healthMessage: "Connection identity restoration failed. Reconnect this app to continue.", + lastError: "connection_identity_rollback_failed", + updatedAt: new Date(), + }).where(and( + eq(toolConnections.id, revivedConnectionPrevious.id), + eq(toolConnections.companyId, companyId), + )); + } catch (quarantineError) { + identityRollbackError = new AggregateError( + [rollbackError, quarantineError], + "Connection identity rollback and quarantine both failed", + ); + } + } } else if (connectionRow) { await db.delete(toolConnections).where(eq(toolConnections.id, connectionRow.id)).catch(() => undefined); } - if (applicationRow && !existingApplication) { + if (!preserveConcurrentRevival && applicationRow && !existingApplication) { await db.delete(toolApplications).where(eq(toolApplications.id, applicationRow.id)).catch(() => undefined); - } else if (existingApplication && applicationRow && applicationRow.status !== existingApplication.status) { + } else if ( + !preserveConcurrentRevival + && existingApplication + && applicationRow + && applicationRow.status !== existingApplication.status + ) { await db.update(toolApplications) .set({ status: existingApplication.status, archivedAt: existingApplication.archivedAt, updatedAt: new Date() }) .where(eq(toolApplications.id, existingApplication.id)) .catch(() => undefined); } - for (const secretId of createdSecretIds) { - await secrets.remove(secretId).catch(() => undefined); + if (!preserveConcurrentRevival) { + for (const secretId of createdSecretIds) { + await secrets.remove(secretId).catch(() => undefined); + } + } + if (identityRollbackError) { + throw new HttpError(500, "Connection setup failed and its prior identity could not be restored.", { + code: "connection_identity_rollback_failed", + }); } throw error; }