Skip to content

Commit ac6f684

Browse files
committed
fix(credentials): close wave 2a review findings on GitHub and Bitbucket tokens
- The GitHub token service is `github-token`, so it no longer shares an id with the GitHub sign-in provider: parseProvider and getProviderIdFromServiceId answer for sign-in exactly as before, and a token-only service is never marked connected from a sign-in account whose id prefixes it. - Suggested connect rows offer each integration once and count it connected when any of its services' OAuth or service-account credentials is saved (GitHub's Search App and saved tokens share one integration). - Assistant GitHub calls bind through the connected-account policy the Assistant projects (the person's github-repositories OAuth account) in the token resolver too, on both the ordinary and managed paths; workflow runs and saved workspace tokens are unaffected. - Bitbucket verification reads one repository (`workspace/repository`), the request Atlassian documents for workspace, project, and repository access tokens alike; listing a workspace is not documented for repository tokens. - prepare-workload-identity test uses the central credentials-access mock.
1 parent 8a8fcf6 commit ac6f684

24 files changed

Lines changed: 391 additions & 85 deletions

File tree

‎apps/docs/content/docs/integrations/bitbucket-service-account.mdx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ A repository access token can only reach its repository, and a project access to
4545
Search for "Bitbucket" and open it, then click **Add to Sim** and choose **Add access token**
4646
</Step>
4747
<Step>
48-
Paste the token and enter the workspace slug it belongs to (the `my-workspace` in `bitbucket.org/my-workspace`)
48+
Paste the token and enter a repository it can read as `workspace/repository` (the `my-workspace/my-repository` in `bitbucket.org/my-workspace/my-repository`). For a repository access token, that is its own repository
4949
</Step>
5050
<Step>
51-
Click **Add access token**. Sim verifies the token by listing a repository in that workspace, so the token needs the `repository` scope
51+
Click **Add access token**. Sim verifies the token by reading that repository once, so the token needs the `repository` scope
5252
</Step>
5353
</Steps>
5454

@@ -58,8 +58,8 @@ Add a Bitbucket block and select the saved access token in the account dropdown,
5858

5959
Two things need a signed-in account and do not work with an access token:
6060

61-
- **List Workspaces** reads the signed-in user's workspaces, so Sim refuses to run it with an access token. Use a connected account for it.
62-
- The workspace picker lists the signed-in user's workspaces, so it cannot load them for an access token. Switch the workspace field to manual entry and type the slug.
61+
- **List Workspaces** lists the workspaces of the person who connected the account, so Sim refuses to run it with an access token. Use a connected account for it.
62+
- The workspace picker lists a person's workspaces, so it cannot load them for an access token. Switch the workspace field to manual entry and type the slug.
6363

6464
## Rotate tokens and handle permission failures
6565

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { INTEGRATION_METADATA } from '@sim/deployment-config/integration-metadata'
2+
import { describe, expect, it } from 'vitest'
3+
import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth'
4+
import { unconnectedIntegrations } from '@/app/workspace/[workspaceId]/home/components/suggested-actions/connect-candidates'
5+
6+
const SERVICES = Object.values(OAUTH_PROVIDERS).flatMap((provider) =>
7+
Object.entries(provider.services).map(([id, service]) => ({ ...service, id }))
8+
)
9+
10+
function githubCandidates(connected: string[]) {
11+
return unconnectedIntegrations(SERVICES, new Set(connected), INTEGRATION_METADATA).filter(
12+
({ slug }) => slug === 'github'
13+
)
14+
}
15+
16+
describe('suggested connect rows', () => {
17+
it('offers GitHub once, through the service its catalog entry names', () => {
18+
expect(githubCandidates([]).map(({ service }) => service.id)).toEqual(['github-token'])
19+
})
20+
21+
it.each([
22+
['a saved personal access token', 'github-service-account'],
23+
['a Search OAuth account', 'github-repositories'],
24+
['a GitHub App installation', 'github-app-installation'],
25+
])('treats GitHub as connected by %s', (_, providerId) => {
26+
expect(githubCandidates([providerId])).toEqual([])
27+
})
28+
29+
it("still offers GitHub when only another integration's credential is saved", () => {
30+
expect(githubCandidates(['slack', 'gitlab-service-account'])).toHaveLength(1)
31+
})
32+
})
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { credentialProviderMatchesService, type ServiceProviderIdentity } from '@/lib/oauth/utils'
2+
3+
interface ConnectableService extends ServiceProviderIdentity {
4+
/** Service key in `OAUTH_PROVIDERS`. */
5+
id: string
6+
name: string
7+
}
8+
9+
interface CatalogIntegration {
10+
name: string
11+
slug: string
12+
oauthServiceId?: string
13+
}
14+
15+
/**
16+
* One connect candidate per catalog integration that no saved credential powers
17+
* yet. Several services can share an integration (GitHub has its Search App and
18+
* its saved tokens), so an integration counts as connected when a credential
19+
* matches any of them — by OAuth id or service-account id — and is offered once,
20+
* through the service its catalog entry names.
21+
*/
22+
export function unconnectedIntegrations<S extends ConnectableService>(
23+
services: readonly S[],
24+
connectedProviderIds: ReadonlySet<string>,
25+
catalog: readonly CatalogIntegration[]
26+
): { service: S; slug: string }[] {
27+
const integrationByName = new Map(catalog.map((entry) => [entry.name.toLowerCase(), entry]))
28+
const servicesBySlug = new Map<string, S[]>()
29+
for (const service of services) {
30+
const integration = integrationByName.get(service.name.toLowerCase())
31+
if (!integration) continue
32+
const group = servicesBySlug.get(integration.slug)
33+
if (group) group.push(service)
34+
else servicesBySlug.set(integration.slug, [service])
35+
}
36+
37+
const candidates: { service: S; slug: string }[] = []
38+
for (const [slug, group] of servicesBySlug) {
39+
const connected = group.some((service) =>
40+
[...connectedProviderIds].some((providerId) =>
41+
credentialProviderMatchesService(providerId, service)
42+
)
43+
)
44+
if (connected) continue
45+
const oauthServiceId = catalog.find((entry) => entry.slug === slug)?.oauthServiceId
46+
const service = group.find((candidate) => candidate.id === oauthServiceId) ?? group[0]
47+
candidates.push({ service, slug })
48+
}
49+
return candidates
50+
}

‎apps/sim/app/workspace/[workspaceId]/home/components/suggested-actions/suggested-actions.tsx‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@/lib/integrations/oauth-service'
1616
import { captureEvent } from '@/lib/posthog/client'
1717
import { ConnectOAuthModal } from '@/app/workspace/[workspaceId]/components/connect-oauth-modal'
18+
import { unconnectedIntegrations } from '@/app/workspace/[workspaceId]/home/components/suggested-actions/connect-candidates'
1819
import type {
1920
Action,
2021
ActionIcon,
@@ -29,11 +30,6 @@ import { useKnowledgeBasesQuery } from '@/hooks/queries/kb/knowledge'
2930
import { useOAuthConnections } from '@/hooks/queries/oauth/connections'
3031
import { useTablesList } from '@/hooks/queries/tables'
3132

32-
/** Lookup integration slug by OAuth service display name (case-insensitive). */
33-
const SLUG_BY_LOWER_NAME: ReadonlyMap<string, string> = new Map(
34-
INTEGRATION_METADATA.map((i) => [i.name.toLowerCase(), i.slug])
35-
)
36-
3733
/** Lookup base block type by catalog slug, for the connect-row popularity weight. */
3834
const TYPE_BY_SLUG: ReadonlyMap<string, string> = new Map(
3935
INTEGRATION_METADATA.map((i) => [i.slug, stripVersionSuffix(i.type)])
@@ -177,11 +173,11 @@ function toIntegrationAction(service: ServiceInfo, slug: string): Action {
177173
* prompts; fresh workspaces get two of each.
178174
*/
179175
function computeActions(services: readonly ServiceInfo[], signals: Signals): Action[] {
180-
const connectCandidates = services.flatMap((s) => {
181-
if (signals.connectedProviders.has(s.providerId)) return []
182-
const slug = SLUG_BY_LOWER_NAME.get(s.name.toLowerCase())
183-
return slug ? [{ service: s, slug }] : []
184-
})
176+
const connectCandidates = unconnectedIntegrations(
177+
services,
178+
signals.connectedProviders,
179+
INTEGRATION_METADATA
180+
)
185181
const connectCount = signals.connectedProviders.size === 0 ? 2 : 1
186182
const integrations = weightedSample(
187183
connectCandidates,

‎apps/sim/blocks/blocks/github.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2490,7 +2490,7 @@ export const GitHubV3Block: BlockConfig = {
24902490
...withSavedCredential(GitHubV2Block, {
24912491
id: 'credential',
24922492
type: 'oauth-input',
2493-
serviceId: 'github',
2493+
serviceId: 'github-token',
24942494
title: 'GitHub Credential',
24952495
}),
24962496
}
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* Drives the Assistant's GitHub token resolution through the real token resolver
3+
* (`lib/oauth/token-resolution`), so the tool policy it binds against is the one the
4+
* resolver actually reads, not a mock's.
5+
*/
6+
import { auditMock } from '@sim/testing/mocks/audit.mock'
7+
import { authOAuthUtilsMock, authOAuthUtilsMockFns } from '@sim/testing/mocks/auth-oauth-utils.mock'
8+
import { posthogServerMock } from '@sim/testing/mocks/posthog-server.mock'
9+
import { beforeEach, describe, expect, it, vi } from 'vitest'
10+
11+
const { mockAuthorizeCredentialUseForAuth, mockPersonalCredentialUseCase } = vi.hoisted(() => ({
12+
mockAuthorizeCredentialUseForAuth: vi.fn(),
13+
mockPersonalCredentialUseCase: vi.fn(),
14+
}))
15+
16+
vi.mock('@sim/audit', () => auditMock)
17+
vi.mock('@/lib/posthog/server', () => posthogServerMock)
18+
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
19+
vi.mock('@/lib/auth/credential-access', () => ({
20+
authorizeCredentialUseForAuth: mockAuthorizeCredentialUseForAuth,
21+
}))
22+
vi.mock('@/lib/mothership/application/execute-credential-use-case', () => ({
23+
executeCopilotCredentialUseCase: mockPersonalCredentialUseCase,
24+
}))
25+
vi.unmock('@/lib/oauth/utils')
26+
27+
import { resolveExecutorCredentialToken } from '@/executor/utils/credential-token'
28+
import { getToolMetadata } from '@/tools/metadata'
29+
30+
const { mockGetCredential, mockRefreshTokenIfNeeded, mockResolveOAuthAccountId } =
31+
authOAuthUtilsMockFns
32+
33+
const ASSISTANT = {
34+
userId: 'person',
35+
workspaceId: 'ws-1',
36+
chatId: 'chat-1',
37+
toolCallId: 'call-1',
38+
copilotToolExecution: true,
39+
requestMode: 'assistant' as const,
40+
}
41+
42+
function resolve(overrides: Partial<Parameters<typeof resolveExecutorCredentialToken>[0]> = {}) {
43+
return resolveExecutorCredentialToken({
44+
requestId: 'req-1',
45+
credentialId: 'own-account',
46+
userId: 'person',
47+
toolId: 'github_search_users_v2',
48+
copilotExecutionContext: ASSISTANT,
49+
...overrides,
50+
})
51+
}
52+
53+
describe('Assistant GitHub calls through the real token resolver', () => {
54+
beforeEach(async () => {
55+
const actual = await vi.importActual<typeof import('@/tools/metadata')>('@/tools/metadata')
56+
vi.mocked(getToolMetadata).mockImplementation(actual.getToolMetadata)
57+
mockPersonalCredentialUseCase.mockResolvedValue({ id: 'own-account' })
58+
mockAuthorizeCredentialUseForAuth.mockResolvedValue({
59+
ok: true,
60+
requesterUserId: 'person',
61+
credentialOwnerUserId: 'person',
62+
workspaceId: 'ws-1',
63+
resolvedCredentialId: 'own-account',
64+
})
65+
mockRefreshTokenIfNeeded.mockResolvedValue({ accessToken: 'personal-github-oauth' })
66+
})
67+
68+
it("resolves the person's own GitHub account for a GitHub tool", async () => {
69+
mockResolveOAuthAccountId.mockResolvedValue({
70+
credentialType: 'oauth',
71+
credentialId: 'own-account',
72+
providerId: 'github-repositories',
73+
accountId: 'account-1',
74+
workspaceId: 'ws-1',
75+
usedCredentialTable: true,
76+
})
77+
mockGetCredential.mockResolvedValue({ providerId: 'github-repositories', scope: 'repo' })
78+
79+
await expect(resolve()).resolves.toMatchObject({ accessToken: 'personal-github-oauth' })
80+
})
81+
82+
it('never hands the Assistant a saved workspace GitHub token', async () => {
83+
mockResolveOAuthAccountId.mockResolvedValue({
84+
credentialType: 'service_account',
85+
credentialId: 'workspace-token',
86+
providerId: 'github-service-account',
87+
accountId: '',
88+
workspaceId: 'ws-1',
89+
usedCredentialTable: true,
90+
})
91+
92+
await expect(resolve({ credentialId: 'workspace-token' })).rejects.toThrow(
93+
'The selected credential type is not supported by this tool'
94+
)
95+
expect(authOAuthUtilsMockFns.mockResolveServiceAccountToken).not.toHaveBeenCalled()
96+
})
97+
98+
it("does not let a workflow run bind GitHub tools to a person's GitHub account", async () => {
99+
mockResolveOAuthAccountId.mockResolvedValue({
100+
credentialType: 'oauth',
101+
credentialId: 'own-account',
102+
providerId: 'github-repositories',
103+
accountId: 'account-1',
104+
workspaceId: 'ws-1',
105+
usedCredentialTable: true,
106+
})
107+
mockGetCredential.mockResolvedValue({ providerId: 'github-repositories', scope: 'repo' })
108+
109+
await expect(resolve({ copilotExecutionContext: undefined })).rejects.toThrow(
110+
'The selected credential type is not supported by this tool'
111+
)
112+
expect(mockRefreshTokenIfNeeded).not.toHaveBeenCalled()
113+
})
114+
})

‎apps/sim/executor/utils/credential-token.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ export async function resolveExecutorCredentialToken(
115115
workflowId,
116116
executionId: params.executionId,
117117
toolId,
118+
assistantConnectedAccount: copilotExecutionContext?.requestMode === 'assistant',
118119
scopes: params.scopes,
119120
impersonateEmail: params.impersonateEmail,
120121
callerUserId: userId && params.enforceCredentialAccess ? userId : undefined,

‎apps/sim/hooks/queries/oauth/connections.test.tsx‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
*/
44

55
import { act, type ReactNode } from 'react'
6-
import { apiClientRequestMock } from '@sim/testing/mocks/api-client-request.mock'
6+
import {
7+
apiClientRequestMock,
8+
apiClientRequestMockFns,
9+
} from '@sim/testing/mocks/api-client-request.mock'
710
import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
811
import { libDesktopMock, libDesktopMockFns } from '@sim/testing/mocks/lib-desktop.mock'
912
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
@@ -13,9 +16,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1316
vi.mock('@/lib/api/client/request', () => apiClientRequestMock)
1417
vi.mock('@/lib/auth/client', () => authClientMock)
1518
vi.mock('@/lib/desktop', () => libDesktopMock)
16-
vi.mock('@/lib/oauth', () => ({ OAUTH_PROVIDERS: {} }))
19+
const { oauthProviders } = vi.hoisted(() => ({
20+
oauthProviders: {} as Record<string, { services: Record<string, unknown> }>,
21+
}))
22+
vi.mock('@/lib/oauth', () => ({ OAUTH_PROVIDERS: oauthProviders }))
1723

18-
import { useConnectOAuthService } from '@/hooks/queries/oauth/connections'
24+
import { useConnectOAuthService, useOAuthConnections } from '@/hooks/queries/oauth/connections'
1925

2026
const beginOAuthConnect = vi.fn()
2127
const oauthLink = authClientMockFns.mockClient.oauth2.link
@@ -72,3 +78,69 @@ describe('useConnectOAuthService', () => {
7278
}
7379
)
7480
})
81+
82+
describe('useOAuthConnections', () => {
83+
afterEach(() => {
84+
for (const key of Object.keys(oauthProviders)) delete oauthProviders[key]
85+
})
86+
87+
it('never marks a token-only service connected from a sign-in account sharing its prefix', async () => {
88+
oauthProviders['github-token'] = {
89+
services: {
90+
'github-token': {
91+
name: 'GitHub',
92+
providerId: 'github-token',
93+
serviceAccountProviderId: 'github-service-account',
94+
authType: 'service_account',
95+
scopes: [],
96+
},
97+
},
98+
}
99+
oauthProviders.dub = {
100+
services: { dub: { name: 'Dub', providerId: 'dub', scopes: [] } },
101+
}
102+
apiClientRequestMockFns.mockRequestJson.mockResolvedValue({
103+
connections: [
104+
{
105+
provider: 'github',
106+
baseProvider: 'github',
107+
featureType: 'default',
108+
isConnected: true,
109+
scopes: ['read:user'],
110+
lastConnected: '2026-11-01T00:00:00.000Z',
111+
accounts: [{ id: 'sign-in', name: 'octo (GitHub)' }],
112+
},
113+
{
114+
provider: 'dub-legacy',
115+
baseProvider: 'dub',
116+
featureType: 'default',
117+
isConnected: true,
118+
scopes: [],
119+
lastConnected: '2026-11-01T00:00:00.000Z',
120+
accounts: [{ id: 'dub-account', name: 'Dub' }],
121+
},
122+
],
123+
})
124+
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
125+
const container = document.createElement('div')
126+
const root = createRoot(container)
127+
let services: ReturnType<typeof useOAuthConnections>['data']
128+
function Probe() {
129+
services = useOAuthConnections().data
130+
return null
131+
}
132+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
133+
await act(async () => {
134+
root.render(
135+
<QueryClientProvider client={queryClient}>
136+
<Probe />
137+
</QueryClientProvider>
138+
)
139+
})
140+
await vi.waitFor(() => expect(services).toBeDefined())
141+
142+
expect(services?.find((service) => service.id === 'github-token')?.isConnected).toBe(false)
143+
expect(services?.find((service) => service.id === 'dub')?.isConnected).toBe(true)
144+
act(() => root.unmount())
145+
})
146+
})

‎apps/sim/hooks/queries/oauth/connections.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ async function fetchOAuthConnections(signal?: AbortSignal): Promise<ServiceInfo[
8888
}
8989
}
9090

91+
// A service with no OAuth flow is never connected by an account: its id can
92+
// still share a prefix with a sign-in provider's (`github-token` and `github`).
93+
if (service.authType === 'service_account') return service
94+
9195
const connectionWithScopes = connections.find((conn: OAuthConnectionResponse) => {
9296
if (!conn.baseProvider || !service.providerId.startsWith(conn.baseProvider)) {
9397
return false

0 commit comments

Comments
 (0)