Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
|
@greptile review |
PR SummaryHigh Risk Overview Extends OAuth-facing endpoints to list and authorize service-account credentials and to mint access tokens for them via a new Updates UI and tooling to support adding service accounts (paste/upload JSON key), selecting them in workflows (with an impersonated-email field), and ensuring copilot/tool execution can resolve and request tokens for either OAuth or service-account credentials. Written by Cursor Bugbot for commit 7ec0259. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Impersonation field shown for non-service-account credentials
- The impersonation input now renders only when the selected credential is a service account by gating on
isServiceAccountinstead of provider-level support.
- The impersonation input now renders only when the selected credential is a service account by gating on
- ✅ Fixed: Return type mismatch:
undefinedinstead offalsehasExternalApiCredentialsnow coalesces the optional chaining result with?? falseso it always returns a boolean.
- ✅ Fixed: Serializer orphan logic is broader than intended
- The orphan serialization path was narrowed to only include the known
impersonateUserEmailkey instead of all orphan sub-blocks with values.
- The orphan serialization path was narrowed to only include the known
Or push these changes by commenting:
@cursor push 035b79165e
Preview (035b79165e)
diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx
--- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx
+++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx
@@ -10,7 +10,6 @@
import {
getCanonicalScopesForProvider,
getProviderIdFromServiceId,
- getServiceAccountProviderForProviderId,
OAUTH_PROVIDERS,
type OAuthProvider,
parseProvider,
@@ -122,11 +121,6 @@
[selectedCredential]
)
- const supportsServiceAccount = useMemo(
- () => !!getServiceAccountProviderForProviderId(effectiveProviderId),
- [effectiveProviderId]
- )
-
const selectedCredentialSet = useMemo(
() => credentialSets.find((cs) => cs.id === selectedCredentialSetId),
[credentialSets, selectedCredentialSetId]
@@ -377,7 +371,7 @@
className={overlayContent ? 'pl-7' : ''}
/>
- {supportsServiceAccount && !isPreview && (
+ {isServiceAccount && !isPreview && (
<div className='mt-2.5 flex flex-col gap-2.5'>
<div className='flex items-center gap-1.5 pl-0.5'>
<Label>
diff --git a/apps/sim/lib/auth/hybrid.ts b/apps/sim/lib/auth/hybrid.ts
--- a/apps/sim/lib/auth/hybrid.ts
+++ b/apps/sim/lib/auth/hybrid.ts
@@ -25,7 +25,7 @@
export function hasExternalApiCredentials(headers: Headers): boolean {
if (headers.has(API_KEY_HEADER)) return true
const auth = headers.get('authorization')
- return auth?.startsWith(BEARER_PREFIX)
+ return auth?.startsWith(BEARER_PREFIX) ?? false
}
export interface AuthResult {
diff --git a/apps/sim/serializer/index.ts b/apps/sim/serializer/index.ts
--- a/apps/sim/serializer/index.ts
+++ b/apps/sim/serializer/index.ts
@@ -347,14 +347,17 @@
)
)
- const isOrphanWithValue =
- matchingConfigs.length === 0 && subBlock.value != null && subBlock.value !== ''
+ const isImpersonateUserEmailOrphanWithValue =
+ id === 'impersonateUserEmail' &&
+ matchingConfigs.length === 0 &&
+ subBlock.value != null &&
+ subBlock.value !== ''
if (
(matchingConfigs.length > 0 && shouldInclude) ||
hasStarterInputFormatValues ||
isLegacyAgentField ||
- isOrphanWithValue
+ isImpersonateUserEmailOrphanWithValue
) {
params[id] = subBlock.value
}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| disabled={effectiveDisabled} | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Impersonation field shown for non-service-account credentials
Medium Severity
The "Impersonated Account" input field (with a required-looking asterisk) is gated on supportsServiceAccount, which checks whether the provider has a service account variant. It renders for every Google credential selector even when the user selects a regular OAuth credential. The condition likely needs to use isServiceAccount (which checks whether the selected credential is a service account) instead of supportsServiceAccount, so the field only appears when relevant.
Additional Locations (1)
| if (headers.has(API_KEY_HEADER)) return true | ||
| const auth = headers.get('authorization') | ||
| return auth !== null && auth.startsWith(BEARER_PREFIX) | ||
| return auth?.startsWith(BEARER_PREFIX) |
There was a problem hiding this comment.
Return type mismatch: undefined instead of false
Medium Severity
hasExternalApiCredentials is declared to return boolean, but auth?.startsWith(BEARER_PREFIX) evaluates to undefined (not false) when auth is null. The previous code auth !== null && auth.startsWith(BEARER_PREFIX) always returned a boolean. While undefined is falsy like false, this breaks strict equality checks and violates the function's type contract.
| hasStarterInputFormatValues || | ||
| isLegacyAgentField | ||
| isLegacyAgentField || | ||
| isOrphanWithValue |
There was a problem hiding this comment.
Serializer orphan logic is broader than intended
Low Severity
The isOrphanWithValue condition serializes any sub-block that has a value but no matching config definition. This was added to pass through impersonateUserEmail, but it will also serialize any other stale or unintended sub-block values (e.g., leftover values from a block type change). A more targeted check like id === 'impersonateUserEmail' would avoid leaking unexpected parameters to tool execution.
Greptile SummaryThis PR introduces Google Service Account as a new credential type, enabling workspace admins to upload a JSON key file and impersonate Google Workspace users across tools (starting with Gmail). The implementation spans the full stack: a new DB enum value + column + migration, encrypted key storage, a JWT two-legged OAuth (2LO) token generation path in the API, access-control checks that mirror the existing OAuth credential membership model, and UI changes in the integrations manager and credential selector. Key changes:
Issues found:
Confidence Score: 4/5Safe to merge after fixing the TypeScript type regression in hybrid.ts and the impersonation field visibility bug in credential-selector.tsx. Two P1 issues remain: a TypeScript type error that will break builds under strict mode, and a UX/logic bug where the impersonation email field (marked required with an asterisk) appears for regular OAuth credentials. The remaining findings are P2 (label text, serializer scope, limited service coverage). apps/sim/lib/auth/hybrid.ts and apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx require attention before merging. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Credential Selector (UI)
participant TokenAPI as /api/auth/oauth/token
participant CredAPI as /api/credentials
participant Utils as oauth/utils.ts
participant Google as Google Token Endpoint
Note over UI,Google: Service Account Setup
UI->>CredAPI: POST /api/credentials {type:'service_account', serviceAccountJson}
CredAPI->>CredAPI: Validate JSON (type, client_email, private_key, project_id)
CredAPI->>CredAPI: encryptSecret(serviceAccountJson)
CredAPI->>CredAPI: INSERT credential row + provision credentialMembers
Note over UI,Google: Token Retrieval at Workflow Execution
UI->>TokenAPI: POST /api/auth/oauth/token {credentialId, scopes, impersonateEmail}
TokenAPI->>Utils: resolveOAuthAccountId(credentialId)
Utils-->>TokenAPI: {credentialType:'service_account', credentialId}
TokenAPI->>TokenAPI: authorizeCredentialUse (membership + workspace check)
TokenAPI->>Utils: getServiceAccountToken(credentialId, scopes, impersonateEmail)
Utils->>Utils: decryptSecret(encryptedServiceAccountKey)
Utils->>Utils: Build JWT {iss, sub, scope, aud, iat, exp}
Utils->>Utils: createSign('RSA-SHA256').sign(private_key)
Utils->>Google: POST token_uri {grant_type: jwt-bearer, assertion: jwt}
Google-->>Utils: {access_token}
Utils-->>TokenAPI: access_token
TokenAPI-->>UI: {accessToken}
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return auth?.startsWith(BEARER_PREFIX) | ||
| } |
There was a problem hiding this comment.
boolean | undefined instead of boolean
Headers.get() returns string | null. When auth is null, the optional chain auth?.startsWith(…) evaluates to undefined, not false. The function is typed as returning boolean, so TypeScript strict mode will reject this as a type error. The original auth !== null && auth.startsWith(BEARER_PREFIX) was correct.
| return auth?.startsWith(BEARER_PREFIX) | |
| } | |
| return auth !== null && auth.startsWith(BEARER_PREFIX) |
| <div className='mt-1.5'> | ||
| <label className='inline-flex cursor-pointer items-center gap-1.5 text-[12px] text-[var(--text-muted)] hover:text-[var(--text-secondary)]'> | ||
| <input | ||
| type='file' |
There was a problem hiding this comment.
deleteCredential.isPending is already used to disable the button (good), but the label still only checks disconnectOAuthService.isPending. For service accounts the disconnect goes through deleteCredential, so the button will show "Disconnect" (not "Disconnecting…") while the request is pending.
| <div className='mt-1.5'> | |
| <label className='inline-flex cursor-pointer items-center gap-1.5 text-[12px] text-[var(--text-muted)] hover:text-[var(--text-secondary)]'> | |
| <input | |
| type='file' | |
| {disconnectOAuthService.isPending || deleteCredential.isPending ? 'Disconnecting...' : 'Disconnect'} |
1 participant
Summary
Add google service support as a integration. Allows users with admin credentials to assume roles on behalf of their google workspace users.
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos