feat(credentials): agent-initiated oauth credential reconnect#5488
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Display names for server-created connect drafts use shared Chat credential link chips parse Reviewed by Cursor Bugbot for commit 5baf4ab. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@cursor review |
|
Greptile SummaryThis PR adds agent-initiated OAuth credential reconnect support: the
Confidence Score: 4/5Safe to merge with the audit-log display name caveat noted — the credential itself is always reconnected correctly. The auth guards, workspace isolation, provider-mismatch check, and draft contamination prevention are all solid. The one defect is that agent-initiated reconnects write an incremented display name (e.g., "Justin's Gmail 2") into the draft because the credential being reconnected is already counted in takenNames; handleReconnectCredential uses that name only for the audit log — the credential's actual display name and accountId update are unaffected. Audit records for reconnects will therefore mis-identify the credential name until this is corrected. apps/sim/app/api/auth/oauth2/authorize/route.ts — the createConnectDraft collision-numbering path for reconnects. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Agent
participant ToolHandler as oauth.ts
participant AuthorizeRoute as authorize route
participant DB
participant BetterAuth
participant Browser
Agent->>ToolHandler: executeOAuthGetAuthLink(providerName, credentialId)
ToolHandler->>DB: getCredentialActorContext — validate type/workspace/admin
DB-->>ToolHandler: actor
ToolHandler-->>Agent: oauth_url with credentialId param
Note over Browser: User clicks Reconnect chip
Browser->>AuthorizeRoute: "GET authorize?providerId=X&credentialId=cred-1"
AuthorizeRoute->>DB: checkWorkspaceAccess
AuthorizeRoute->>DB: getCredentialActorContext — re-validate admin+type+provider
AuthorizeRoute->>DB: createConnectDraft with credentialId
AuthorizeRoute->>BetterAuth: oAuth2LinkAccount
BetterAuth-->>AuthorizeRoute: provider URL
AuthorizeRoute-->>Browser: redirect to provider
Browser->>BetterAuth: OAuth callback
BetterAuth->>DB: account.create.after hook
DB->>DB: handleReconnectCredential — UPDATE credential SET accountId
Note over DB: credential.id preserved, old orphaned account deleted
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Agent
participant ToolHandler as oauth.ts
participant AuthorizeRoute as authorize route
participant DB
participant BetterAuth
participant Browser
Agent->>ToolHandler: executeOAuthGetAuthLink(providerName, credentialId)
ToolHandler->>DB: getCredentialActorContext — validate type/workspace/admin
DB-->>ToolHandler: actor
ToolHandler-->>Agent: oauth_url with credentialId param
Note over Browser: User clicks Reconnect chip
Browser->>AuthorizeRoute: "GET authorize?providerId=X&credentialId=cred-1"
AuthorizeRoute->>DB: checkWorkspaceAccess
AuthorizeRoute->>DB: getCredentialActorContext — re-validate admin+type+provider
AuthorizeRoute->>DB: createConnectDraft with credentialId
AuthorizeRoute->>BetterAuth: oAuth2LinkAccount
BetterAuth-->>AuthorizeRoute: provider URL
AuthorizeRoute-->>Browser: redirect to provider
Browser->>BetterAuth: OAuth callback
BetterAuth->>DB: account.create.after hook
DB->>DB: handleReconnectCredential — UPDATE credential SET accountId
Note over DB: credential.id preserved, old orphaned account deleted
Reviews (1): Last reviewed commit: "feat(credentials): agent-initiated oauth..." | Re-trigger Greptile |
| let takenNames: ReadonlySet<string> = new Set<string>() | ||
| try { | ||
| const rows = await db | ||
| .select({ displayName: credential.displayName }) | ||
| .from(credential) | ||
| .where(and(eq(credential.workspaceId, workspaceId), eq(credential.type, 'oauth'))) | ||
| takenNames = new Set(rows.map((row) => row.displayName.toLowerCase())) | ||
| } catch { | ||
| // Fall back to service name only | ||
| // Best effort — proceed without collision numbering | ||
| } | ||
|
|
||
| const displayName = defaultCredentialDisplayName(userName, serviceName, takenNames) |
There was a problem hiding this comment.
takenNames
For a reconnect, createConnectDraft queries all workspace OAuth credentials into takenNames — including the credential that is being reconnected (e.g., "Justin's Gmail"). defaultCredentialDisplayName sees that name as taken and returns "Justin's Gmail 2". handleReconnectCredential then uses draft.displayName as both resourceName and the description field in the audit record, so the audit log for the reconnect event will read Reconnected OAuth credential "Justin's Gmail 2" when the credential is actually named "Justin's Gmail". The reconnect itself is functionally correct (accountId is updated without touching displayName), but the audit trail is inaccurate for every agent-initiated reconnect where the credential's display name matches the generated default pattern.
The fix is to skip collision numbering when credentialId is present — for reconnects the draft's displayName should reflect the existing credential's actual name, not a freshly de-duplicated one.
Greptile SummaryThis PR adds agent-initiated OAuth credential reconnect: the
Confidence Score: 4/5The reconnect authorization chain is well-guarded with redundant credential-admin checks at both the tool-handler and route layers. The main imprecision is in the audit log for reconnects, where the draft display name gets a collision suffix because the credential being reconnected is counted in takenNames. Cross-workspace attacks, non-admin reconnects, type and provider mismatches, and draft cross-contamination are all explicitly checked and tested. The two issues found are the misleading audit log name during reconnects and a redundant workspace-access DB round-trip in the tool handler — neither affects credential data or user-facing behavior. apps/sim/app/api/auth/oauth2/authorize/route.ts — the createConnectDraft call in the reconnect path should skip or narrow collision detection so the draft's displayName matches the existing credential name for correct audit logs. Important Files Changed
Reviews (2): Last reviewed commit: "feat(credentials): agent-initiated oauth..." | Re-trigger Greptile |
| let takenNames: ReadonlySet<string> = new Set<string>() | ||
| try { | ||
| const rows = await db | ||
| .select({ displayName: credential.displayName }) | ||
| .from(credential) | ||
| .where(and(eq(credential.workspaceId, workspaceId), eq(credential.type, 'oauth'))) | ||
| takenNames = new Set(rows.map((row) => row.displayName.toLowerCase())) | ||
| } catch { | ||
| // Fall back to service name only | ||
| // Best effort — proceed without collision numbering | ||
| } | ||
|
|
||
| const displayName = defaultCredentialDisplayName(userName, serviceName, takenNames) |
There was a problem hiding this comment.
When credentialId is set (reconnect path), takenNames is populated from all oauth credentials in the workspace — which includes the credential being reconnected. So defaultCredentialDisplayName returns a suffixed name (e.g., "Justin's Gmail 2" instead of "Justin's Gmail") because the credential's own name is already in the set. handleReconnectCredential uses draft.displayName only for the audit log (resourceName, description) and never writes it back to the credential's displayName column, so the credential itself is unaffected — but the audit entry reads "Reconnected OAuth credential 'Justin's Gmail 2'" when the credential is still named 'Justin's Gmail'.
For the reconnect case, collision detection provides no value (no new credential is being named), so takenNames should be passed as an empty set when credentialId is provided.
| `integrations page and press Reconnect on the credential there.` | ||
| ) | ||
| } | ||
| const actor = await getCredentialActorContext(reconnect.credentialId, reconnect.userId) |
There was a problem hiding this comment.
ensureWorkspaceAccess (called on line 20) fetches workspace access via checkWorkspaceAccess internally but discards the result. getCredentialActorContext here is called without { workspaceAccess }, so it re-fetches workspace access for the same user/workspace pair a second time. The route handler avoids this by passing { workspaceAccess: access } from the already-resolved checkWorkspaceAccess result. Consider returning the access object from ensureWorkspaceAccess (or using a local checkWorkspaceAccess call in generateOAuthLink) so it can be threaded into getCredentialActorContext.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5baf4ab. Configure here.
| // (and freshly clocked) when the OAuth callback's `account.create.after` | ||
| // hook runs. If this throws, we never start the OAuth flow. | ||
| await createConnectDraft({ userId, workspaceId, providerId }) | ||
| await createConnectDraft({ userId, workspaceId, providerId, credentialId }) |
There was a problem hiding this comment.
Trello Shopify reconnect draft leak
Medium Severity
The OAuth2 authorize route accepts credentialId for Trello and Shopify and always writes a reconnect pending draft before calling oAuth2LinkAccount, but those providers use separate authorize flows and are rejected in the copilot tool. A failed or abandoned /api/auth/oauth2/authorize attempt can leave a reconnect draft that Trello/Shopify token storage later consumes via processCredentialDraft, rebinding the credential unintentionally.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5baf4ab. Configure here.
Summary
oauth_get_auth_linkaccepts an optionalcredentialId(tool param + prompts land in the companion mothership PR)/api/auth/oauth2/authorizeacceptscredentialId: requires credential-admin access (same check as the draft POST route), rejects cross-workspace / non-oauth / provider-mismatched ids, and threads the id into the pending credential draft so the existinghandleReconnectCredentialpath rebinds the samecredential.idto the fresh account rowcredentialId(or null) on both the insert and conflict paths — a plain connect can never inherit a stale reconnect draft and silently rebind an existing credentiallib/credentials/display-name.tsand applied it to copilot-path connects — agent-created credentials get "Justin's Gmail 2" instead of identical duplicate namescredentialId(derived from the URL, not from model-emitted tag fields)Note:
tool-schemas-v1.tshas a ~195-line key-style diff — pre-existing generator drift caught up by regen (bun run mship:checkpasses); not feature churn.Merge order: this PR merges/deploys before the mothership companion — the new tool param crosses the tool-catalog codegen boundary.
Type of Change
Testing
bun run lintclean,bun run check:api-validation:strictpassed, tsc cleancredential.id, swaps the account row, deletes the orphaned account, dependent workflow still runsChecklist
🤖 Generated with Claude Code
Companion PRs