Skip to content

Commit 0b8f18c

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(atlassian): add shared selector credential safety
1 parent 62e98a4 commit 0b8f18c

7 files changed

Lines changed: 281 additions & 12 deletions

File tree

apps/sim/lib/atlassian/discovery.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,19 @@ describe('resolveAtlassianCloudId', () => {
123123
expect(fetchMock).toHaveBeenCalledTimes(1)
124124
})
125125

126+
it('omits provider response bodies from selector discovery errors', async () => {
127+
fetchMock.mockResolvedValue(failure(403, { marker: 'provider-body-secret-marker' }))
128+
129+
const error = await resolveAtlassianCloudId(
130+
options({
131+
retryOptions: { ...FAST, omitResponseBodyFromErrors: true },
132+
})
133+
).catch((caught) => caught as Error)
134+
135+
expect(error.message).toBe('Failed to fetch Jira accessible resources: 403')
136+
expect(error.message).not.toContain('provider-body-secret-marker')
137+
})
138+
126139
it('does not pin a failure in the cache', async () => {
127140
fetchMock.mockResolvedValueOnce(failure(403, { message: 'nope' }))
128141
await expect(resolveAtlassianCloudId(options({ retryOptions: FAST }))).rejects.toThrow()

apps/sim/lib/atlassian/discovery.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,20 @@ interface AccessibleResource {
102102
url: string
103103
}
104104

105+
export interface AtlassianDiscoveryRetryOptions extends RetryOptions {
106+
/**
107+
* Selector routes set this to avoid putting provider-controlled response
108+
* bodies into thrown errors, which the shared retry helper intentionally logs.
109+
*/
110+
omitResponseBodyFromErrors?: boolean
111+
}
112+
105113
interface ResolveAtlassianCloudIdOptions {
106114
domain: string
107115
accessToken: string
108116
/** Product name woven into the failure messages, e.g. `Jira` or `Confluence`. */
109117
product: string
110-
retryOptions?: RetryOptions
118+
retryOptions?: AtlassianDiscoveryRetryOptions
111119
}
112120

113121
const cloudIdCache = createAtlassianDiscoveryCache()
@@ -141,8 +149,9 @@ export function fetchAtlassianDiscoveryJson<T>(
141149
url: string,
142150
headers: Record<string, string>,
143151
failureLabel: string,
144-
retryOptions?: RetryOptions
152+
retryOptions?: AtlassianDiscoveryRetryOptions
145153
): Promise<T> {
154+
const { omitResponseBodyFromErrors = false, ...effectiveRetryOptions } = retryOptions ?? {}
146155
return retryWithExponentialBackoff(
147156
async () => {
148157
const response = await fetch(url, {
@@ -152,10 +161,10 @@ export function fetchAtlassianDiscoveryJson<T>(
152161
})
153162

154163
if (!response.ok) {
155-
const errorText = await response.text()
156-
const error: HTTPError = new Error(
157-
`${failureLabel}: ${response.status} - ${errorText || response.statusText}`
158-
)
164+
const errorDetail = omitResponseBodyFromErrors
165+
? ''
166+
: ` - ${(await response.text()) || response.statusText}`
167+
const error: HTTPError = new Error(`${failureLabel}: ${response.status}${errorDetail}`)
159168
error.status = response.status
160169
error.statusText = response.statusText
161170
const retryAfterMs = parseRetryAfter(response.headers.get('Retry-After'))
@@ -165,14 +174,14 @@ export function fetchAtlassianDiscoveryJson<T>(
165174

166175
return (await response.json()) as T
167176
},
168-
{ ...ATLASSIAN_DISCOVERY_RETRY_OPTIONS, ...retryOptions }
177+
{ ...ATLASSIAN_DISCOVERY_RETRY_OPTIONS, ...effectiveRetryOptions }
169178
)
170179
}
171180

172181
function fetchAccessibleResources(
173182
accessToken: string,
174183
product: string,
175-
retryOptions: RetryOptions | undefined
184+
retryOptions: AtlassianDiscoveryRetryOptions | undefined
176185
): Promise<AccessibleResource[]> {
177186
return fetchAtlassianDiscoveryJson<AccessibleResource[]>(
178187
ACCESSIBLE_RESOURCES_URL,
@@ -232,7 +241,10 @@ export async function resolveAtlassianCloudId(
232241
options: ResolveAtlassianCloudIdOptions
233242
): Promise<string> {
234243
const { domain, accessToken, product, retryOptions } = options
235-
const key = atlassianDiscoveryKey(normalizeAtlassianSiteUrl(domain), accessToken)
244+
const errorMode = retryOptions?.omitResponseBodyFromErrors
245+
? 'sanitized-errors'
246+
: 'standard-errors'
247+
const key = `${atlassianDiscoveryKey(normalizeAtlassianSiteUrl(domain), accessToken)}:${errorMode}`
236248

237249
return cloudIdCache.resolve(key, async () =>
238250
selectAtlassianCloudId(
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
getServiceAccountSecret: vi.fn(),
8+
providerMatches: vi.fn(),
9+
refreshToken: vi.fn(),
10+
resolveAccount: vi.fn(),
11+
}))
12+
13+
vi.mock('@/lib/oauth/credential-service', () => ({
14+
getAtlassianServiceAccountSecret: mocks.getServiceAccountSecret,
15+
refreshAccessTokenIfNeeded: mocks.refreshToken,
16+
resolveOAuthAccountId: mocks.resolveAccount,
17+
}))
18+
vi.mock('@/lib/selectors/application/credential-provider', () => ({
19+
selectorCredentialMatchesService: mocks.providerMatches,
20+
}))
21+
22+
import { resolveAtlassianSelectorCredential } from '@/lib/selectors/application/atlassian-credential'
23+
24+
describe('resolveAtlassianSelectorCredential', () => {
25+
beforeEach(() => {
26+
vi.clearAllMocks()
27+
mocks.providerMatches.mockResolvedValue(true)
28+
mocks.resolveAccount.mockResolvedValue({
29+
providerId: 'atlassian',
30+
credentialId: 'credential-1',
31+
})
32+
mocks.refreshToken.mockResolvedValue('oauth-access-token')
33+
})
34+
35+
it('binds a credential to the requested integration before reading or refreshing secrets', async () => {
36+
mocks.providerMatches.mockResolvedValue(false)
37+
38+
const result = await resolveAtlassianSelectorCredential({
39+
credentialId: 'credential-1',
40+
credentialOwnerUserId: 'owner-1',
41+
requestId: 'request-1',
42+
serviceId: 'jira',
43+
})
44+
45+
expect(result).toBeNull()
46+
expect(mocks.providerMatches).toHaveBeenCalledWith({
47+
credentialId: 'credential-1',
48+
credentialOwnerUserId: 'owner-1',
49+
serviceId: 'jira',
50+
})
51+
expect(mocks.resolveAccount).not.toHaveBeenCalled()
52+
expect(mocks.getServiceAccountSecret).not.toHaveBeenCalled()
53+
expect(mocks.refreshToken).not.toHaveBeenCalled()
54+
})
55+
56+
it('refreshes an OAuth token only after provider binding succeeds', async () => {
57+
const result = await resolveAtlassianSelectorCredential({
58+
credentialId: 'credential-1',
59+
credentialOwnerUserId: 'owner-1',
60+
requestId: 'request-1',
61+
serviceId: 'confluence',
62+
})
63+
64+
expect(result).toEqual({ accessToken: 'oauth-access-token' })
65+
expect(mocks.refreshToken).toHaveBeenCalledWith('credential-1', 'owner-1', 'request-1')
66+
})
67+
68+
it('reads an Atlassian service account only after provider binding succeeds', async () => {
69+
mocks.resolveAccount.mockResolvedValue({
70+
providerId: 'atlassian-service-account',
71+
credentialId: 'service-account-1',
72+
})
73+
mocks.getServiceAccountSecret.mockResolvedValue({
74+
apiToken: 'service-account-token',
75+
cloudId: 'cloud-1',
76+
})
77+
78+
const result = await resolveAtlassianSelectorCredential({
79+
credentialId: 'credential-1',
80+
credentialOwnerUserId: 'owner-1',
81+
requestId: 'request-1',
82+
serviceId: 'jira',
83+
})
84+
85+
expect(result).toEqual({ accessToken: 'service-account-token', cloudId: 'cloud-1' })
86+
expect(mocks.getServiceAccountSecret).toHaveBeenCalledWith('service-account-1')
87+
expect(mocks.refreshToken).not.toHaveBeenCalled()
88+
})
89+
})
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import {
2+
getAtlassianServiceAccountSecret,
3+
refreshAccessTokenIfNeeded,
4+
resolveOAuthAccountId,
5+
} from '@/lib/oauth/credential-service'
6+
import { ATLASSIAN_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
7+
import { selectorCredentialMatchesService } from '@/lib/selectors/application/credential-provider'
8+
9+
export async function resolveAtlassianSelectorCredential(input: {
10+
credentialId: string
11+
credentialOwnerUserId: string
12+
requestId: string
13+
serviceId: 'jira' | 'confluence'
14+
}): Promise<{ accessToken: string; cloudId?: string } | null> {
15+
const providerMatches = await selectorCredentialMatchesService({
16+
credentialId: input.credentialId,
17+
credentialOwnerUserId: input.credentialOwnerUserId,
18+
serviceId: input.serviceId,
19+
})
20+
if (!providerMatches) return null
21+
22+
const resolved = await resolveOAuthAccountId(input.credentialId)
23+
if (resolved?.providerId === ATLASSIAN_SERVICE_ACCOUNT_PROVIDER_ID && resolved.credentialId) {
24+
const secret = await getAtlassianServiceAccountSecret(resolved.credentialId)
25+
return { accessToken: secret.apiToken, cloudId: secret.cloudId }
26+
}
27+
28+
const accessToken = await refreshAccessTokenIfNeeded(
29+
input.credentialId,
30+
input.credentialOwnerUserId,
31+
input.requestId
32+
)
33+
return accessToken ? { accessToken } : null
34+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
resolveSelectorProviderValue,
7+
selectorProviderFailure,
8+
} from '@/lib/selectors/server/provider-errors'
9+
10+
describe('selectorProviderFailure', () => {
11+
it.each([
12+
[
13+
401,
14+
{
15+
error: 'Atlassian rejected this credential. Reconnect it and try again.',
16+
status: 401,
17+
authRequired: true,
18+
},
19+
],
20+
[403, { error: 'Atlassian denied selector access.', status: 403 }],
21+
[
22+
429,
23+
{
24+
error: 'Atlassian rate-limited selector discovery. Try again shortly.',
25+
status: 429,
26+
},
27+
],
28+
[400, { error: 'Atlassian selector discovery failed.', status: 502 }],
29+
[500, { error: 'Atlassian selector discovery failed.', status: 502 }],
30+
])('maps provider status %s to its stable public failure', (input, expected) => {
31+
expect(selectorProviderFailure('Atlassian', input)).toEqual(expected)
32+
})
33+
34+
it('sanitizes a provider discovery exception without serializing its body marker', async () => {
35+
const providerError = Object.assign(new Error('provider-body-secret-marker'), { status: 429 })
36+
37+
const result = await resolveSelectorProviderValue('Jira', async () => {
38+
throw providerError
39+
})
40+
41+
expect(result).toEqual({
42+
ok: false,
43+
failure: {
44+
error: 'Jira rate-limited selector discovery. Try again shortly.',
45+
status: 429,
46+
},
47+
upstreamStatus: 429,
48+
})
49+
expect(JSON.stringify(result)).not.toContain('provider-body-secret-marker')
50+
})
51+
})
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
export interface SelectorProviderFailure {
2+
error: string
3+
status: number
4+
authRequired?: true
5+
}
6+
7+
export type SelectorAtlassianProvider =
8+
| 'Atlassian'
9+
| 'Jira'
10+
| 'Confluence'
11+
| 'Jira Service Management'
12+
13+
/** Keeps provider-controlled discovery bodies out of selector errors and retry logs. */
14+
export const SELECTOR_ATLASSIAN_DISCOVERY_OPTIONS = {
15+
omitResponseBodyFromErrors: true,
16+
} as const
17+
18+
function providerStatusFromError(error: unknown): number | undefined {
19+
if (!error || typeof error !== 'object' || !('status' in error)) return undefined
20+
const status = error.status
21+
return typeof status === 'number' && Number.isInteger(status) ? status : undefined
22+
}
23+
24+
/** Maps provider failures to stable selector-safe responses without reading response bodies. */
25+
export function selectorProviderFailure(
26+
provider: SelectorAtlassianProvider,
27+
status: number
28+
): SelectorProviderFailure {
29+
if (status === 401) {
30+
return {
31+
error: `${provider} rejected this credential. Reconnect it and try again.`,
32+
status: 401,
33+
authRequired: true,
34+
}
35+
}
36+
if (status === 403) {
37+
return { error: `${provider} denied selector access.`, status: 403 }
38+
}
39+
if (status === 429) {
40+
return { error: `${provider} rate-limited selector discovery. Try again shortly.`, status: 429 }
41+
}
42+
return { error: `${provider} selector discovery failed.`, status: 502 }
43+
}
44+
45+
export type SelectorProviderValueResult<T> =
46+
| { ok: true; value: T }
47+
| {
48+
ok: false
49+
failure: SelectorProviderFailure
50+
upstreamStatus?: number
51+
}
52+
53+
/** Converts provider discovery exceptions into a safe value boundary. */
54+
export async function resolveSelectorProviderValue<T>(
55+
provider: SelectorAtlassianProvider,
56+
resolve: () => Promise<T>
57+
): Promise<SelectorProviderValueResult<T>> {
58+
try {
59+
return { ok: true, value: await resolve() }
60+
} catch (error) {
61+
const upstreamStatus = providerStatusFromError(error)
62+
return {
63+
ok: false,
64+
failure: selectorProviderFailure(provider, upstreamStatus ?? 502),
65+
upstreamStatus,
66+
}
67+
}
68+
}

apps/sim/tools/jira/utils.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { createLogger } from '@sim/logger'
2-
import { resolveAtlassianCloudId } from '@/lib/atlassian/discovery'
3-
import type { RetryOptions } from '@/lib/knowledge/documents/utils'
2+
import {
3+
type AtlassianDiscoveryRetryOptions,
4+
resolveAtlassianCloudId,
5+
} from '@/lib/atlassian/discovery'
46
import { fetchWithRetry } from '@/lib/knowledge/documents/utils'
57

68
const logger = createLogger('JiraUtils')
@@ -250,7 +252,7 @@ export function normalizeJiraWorklogTimestamp(value: string): string {
250252
export function getJiraCloudId(
251253
domain: string,
252254
accessToken: string,
253-
retryOptions?: RetryOptions
255+
retryOptions?: AtlassianDiscoveryRetryOptions
254256
): Promise<string> {
255257
return resolveAtlassianCloudId({ domain, accessToken, product: 'Jira', retryOptions })
256258
}

0 commit comments

Comments
 (0)