Skip to content

Commit d226be6

Browse files
committed
fix(bitbucket): stop advanced-field leakage and harden log, status, and selector paths
Splits the `closeSourceBranch` advanced subBlock into per-operation ids. Advanced fields serialize without evaluating their condition, so a value set on Create Pull Request reached Merge Pull Request and closed the source branch unprompted. Also: - read step logs through the byte-capped server transport and map an empty-log 416 to an empty result, keeping a genuine 416 an error - trim a step log's partial leading line after the character cap rather than before, and never return an empty log when the retained window held content - surface Bitbucket's `error.detail` alongside `error.message` - treat commit-status `key`/`state` as nullable so one malformed row cannot drop a page - match repository `full_name` case-insensitively and reject dot segments in a workspace slug before the outbound request - type `reviewerAccountIds` as the comma-separated string it is - trim optional Bitbucket query strings; correct the token lifetime to two hours
1 parent e74c06c commit d226be6

18 files changed

Lines changed: 316 additions & 80 deletions

File tree

apps/sim/app/api/tools/bitbucket/repositories/route.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,38 @@ describe('POST /api/tools/bitbucket/repositories', () => {
276276
expect(response.status).toBe(502)
277277
})
278278

279+
it('accepts a case-mismatched workspace slug the way Bitbucket itself resolves it', async () => {
280+
mockFetch.mockResolvedValueOnce(
281+
providerResponse({
282+
values: [
283+
{
284+
slug: 'sdk-core',
285+
uuid: '{repo-1}',
286+
name: 'SDK Core',
287+
full_name: 'acme-platform/sdk-core',
288+
},
289+
],
290+
next: 'https://api.bitbucket.org/2.0/repositories/acme-platform?page=2',
291+
})
292+
)
293+
294+
const response = await POST(request({ ...REQUEST_BODY, workspaceSlug: 'ACME-Platform' }), {})
295+
296+
expect(response.status).toBe(200)
297+
expect(await json(response)).toMatchObject({
298+
repositories: [{ slug: 'sdk-core', fullName: 'acme-platform/sdk-core' }],
299+
})
300+
})
301+
302+
it('rejects a workspace slug containing dot segments before it reaches Bitbucket', async () => {
303+
for (const workspaceSlug of ['.', '..', 'acme.platform']) {
304+
const response = await POST(request({ ...REQUEST_BODY, workspaceSlug }), {})
305+
306+
expect(response.status, workspaceSlug).toBe(400)
307+
expect(mockFetch).not.toHaveBeenCalled()
308+
}
309+
})
310+
279311
it('rejects a provider next link that crosses the selected workspace', async () => {
280312
mockFetch.mockResolvedValueOnce(
281313
providerResponse({

apps/sim/app/api/tools/bitbucket/repositories/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
145145
)
146146
}
147147

148+
const expectedFullNamePrefix = workspaceSlug.toLowerCase()
148149
const page = bitbucketRepositoryProviderPageSchema.safeParse(providerBody)
149150
if (
150151
!page.success ||
151152
(page.data.next && !isBitbucketRepositoriesCursor(page.data.next, workspaceSlug)) ||
152-
page.data.values.some((repository) => !repository.full_name.startsWith(`${workspaceSlug}/`))
153+
page.data.values.some(
154+
(repository) => !repository.full_name.toLowerCase().startsWith(`${expectedFullNamePrefix}/`)
155+
)
153156
) {
154157
logger.warn('Bitbucket returned a malformed repository page', { workspaceSlug })
155158
return NextResponse.json(

apps/sim/blocks/blocks/bitbucket.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ const SAMPLE_VALUES: Record<string, unknown> = {
6565
destinationBranch: 'main',
6666
description: 'Adds bounded log diagnostics.',
6767
reviewerAccountIds: '{reviewer-a}, {reviewer-b}',
68-
closeSourceBranch: 'true',
68+
createCloseSourceBranch: 'true',
69+
mergeCloseSourceBranch: 'true',
6970
draft: 'true',
7071
mergeStrategy: 'squash',
7172
message: 'Merge pull request 42',
@@ -294,6 +295,20 @@ describe('BitbucketBlock', () => {
294295
expect(() => buildRequestUrl('bitbucket_decline_pull_request', { prId })).toThrow(/prId/)
295296
})
296297

298+
it('does not let a create-pull-request close setting reach a merge', () => {
299+
const merged = buildRequestBody('bitbucket_merge_pull_request', {
300+
createCloseSourceBranch: 'true',
301+
mergeCloseSourceBranch: '',
302+
}) as Record<string, unknown>
303+
expect(merged).not.toHaveProperty('close_source_branch')
304+
305+
const created = buildRequestBody('bitbucket_create_pull_request', {
306+
createCloseSourceBranch: '',
307+
mergeCloseSourceBranch: 'true',
308+
}) as Record<string, unknown>
309+
expect(created).not.toHaveProperty('close_source_branch')
310+
})
311+
297312
it('does not turn a malformed optional parent ID into a top-level comment', () => {
298313
expect(() =>
299314
buildRequestBody('bitbucket_create_pull_request_comment', { parentId: { id: 7 } })
@@ -303,7 +318,7 @@ describe('BitbucketBlock', () => {
303318
it('preserves explicit false booleans and rejects other present boolean shapes', () => {
304319
expect(
305320
buildRequestBody('bitbucket_create_pull_request', {
306-
closeSourceBranch: false,
321+
createCloseSourceBranch: false,
307322
draft: 'false',
308323
})
309324
).toMatchObject({ close_source_branch: false, draft: false })

apps/sim/blocks/blocks/bitbucket.ts

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -599,15 +599,25 @@ export const BitbucketBlock: BlockConfig = {
599599
placeholder: 'Comma-separated Bitbucket user UUIDs',
600600
},
601601
{
602-
id: 'closeSourceBranch',
602+
id: 'createCloseSourceBranch',
603603
title: 'Close Source Branch',
604604
type: 'dropdown',
605605
mode: 'advanced',
606-
condition: {
607-
field: 'operation',
608-
value: ['bitbucket_create_pull_request', 'bitbucket_merge_pull_request'],
609-
},
606+
condition: { field: 'operation', value: 'bitbucket_create_pull_request' },
610607
options: [
608+
{ label: 'Unset', id: '' },
609+
{ label: 'No', id: 'false' },
610+
{ label: 'Yes', id: 'true' },
611+
],
612+
},
613+
{
614+
id: 'mergeCloseSourceBranch',
615+
title: 'Close Source Branch',
616+
type: 'dropdown',
617+
mode: 'advanced',
618+
condition: { field: 'operation', value: 'bitbucket_merge_pull_request' },
619+
options: [
620+
{ label: 'Unset', id: '' },
611621
{ label: 'No', id: 'false' },
612622
{ label: 'Yes', id: 'true' },
613623
],
@@ -955,7 +965,10 @@ export const BitbucketBlock: BlockConfig = {
955965
destinationBranch: optionalString(params.destinationBranch, 'destinationBranch'),
956966
description: optionalText(params.description, 'description'),
957967
reviewerUuids: stringList(params.reviewerAccountIds, 'reviewerAccountIds'),
958-
closeSourceBranch: optionalBoolean(params.closeSourceBranch, 'closeSourceBranch'),
968+
closeSourceBranch: optionalBoolean(
969+
params.createCloseSourceBranch,
970+
'createCloseSourceBranch'
971+
),
959972
draft: optionalBoolean(params.draft, 'draft'),
960973
}
961974
case 'bitbucket_merge_pull_request':
@@ -964,7 +977,10 @@ export const BitbucketBlock: BlockConfig = {
964977
prId: optionalInteger(params.prId, 'prId'),
965978
mergeStrategy: optionalString(params.mergeStrategy, 'mergeStrategy'),
966979
message: optionalText(params.message, 'message'),
967-
closeSourceBranch: optionalBoolean(params.closeSourceBranch, 'closeSourceBranch'),
980+
closeSourceBranch: optionalBoolean(
981+
params.mergeCloseSourceBranch,
982+
'mergeCloseSourceBranch'
983+
),
968984
}
969985
case 'bitbucket_get_pull_request_merge_task_status':
970986
return {
@@ -1045,8 +1061,18 @@ export const BitbucketBlock: BlockConfig = {
10451061
sourceBranch: { type: 'string', description: 'Pull request source branch' },
10461062
destinationBranch: { type: 'string', description: 'Pull request destination branch' },
10471063
description: { type: 'string', description: 'Pull request description' },
1048-
reviewerAccountIds: { type: 'array', description: 'Reviewer Bitbucket user UUIDs' },
1049-
closeSourceBranch: { type: 'boolean', description: 'Whether to close the source branch' },
1064+
reviewerAccountIds: {
1065+
type: 'string',
1066+
description: 'Comma-separated reviewer Bitbucket user UUIDs',
1067+
},
1068+
createCloseSourceBranch: {
1069+
type: 'boolean',
1070+
description: 'Whether to close the source branch after the pull request merges',
1071+
},
1072+
mergeCloseSourceBranch: {
1073+
type: 'boolean',
1074+
description: 'Whether to close the source branch as part of this merge',
1075+
},
10501076
draft: { type: 'boolean', description: 'Whether to create a draft pull request' },
10511077
mergeStrategy: { type: 'string', description: 'Pull request merge strategy' },
10521078
message: { type: 'string', description: 'Merge commit message' },

apps/sim/lib/api/contracts/selectors/bitbucket.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ const bitbucketSlugSchema = z
2222
const bitbucketWorkspaceUuidPattern =
2323
/^(?:\{[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\}|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$/i
2424

25+
/** Bitbucket workspace ids are alphanumeric with hyphens and underscores only. */
26+
const bitbucketWorkspaceSlugPattern = /^[a-z0-9][a-z0-9_-]*$/i
27+
2528
const bitbucketWorkspaceSlugSchema = bitbucketSlugSchema.refine(
26-
(slug) => !slug.includes('/') && !bitbucketWorkspaceUuidPattern.test(slug),
29+
(slug) => bitbucketWorkspaceSlugPattern.test(slug) && !bitbucketWorkspaceUuidPattern.test(slug),
2730
'Bitbucket workspace must be identified by its slug, not a UUID or path'
2831
)
2932

@@ -66,7 +69,8 @@ export function isBitbucketWorkspacesCursor(value: string): boolean {
6669
*/
6770
export function isBitbucketRepositoriesCursor(value: string, workspaceSlug: string): boolean {
6871
const url = parseBitbucketApiCursor(value)
69-
return url?.pathname === `${BITBUCKET_REPOSITORIES_PATH}/${encodeURIComponent(workspaceSlug)}`
72+
const expected = `${BITBUCKET_REPOSITORIES_PATH}/${encodeURIComponent(workspaceSlug)}`
73+
return url?.pathname.toLowerCase() === expected.toLowerCase()
7074
}
7175

7276
const bitbucketCursorSchema = z

apps/sim/lib/auth/connectors/providers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1485,7 +1485,7 @@ export function buildConnectorProviders(): GenericOAuthConfig[] {
14851485
responseType: 'code',
14861486
pkce: false,
14871487
authentication: 'basic',
1488-
accessTokenExpiresIn: 3600,
1488+
accessTokenExpiresIn: 7200,
14891489
redirectURI: `${getBaseUrl()}/api/auth/oauth2/callback/bitbucket`,
14901490
getToken: async ({ code, redirectURI }) => {
14911491
const basicAuth = Buffer.from(

apps/sim/lib/oauth/oauth.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ function getBitbucketConnector() {
114114
}
115115

116116
describe('Bitbucket OAuth Connector', () => {
117-
it('uses the canonical endpoints, scopes, Basic auth, and one-hour expiry', () => {
117+
it('uses the canonical endpoints, scopes, Basic auth, and two-hour expiry', () => {
118118
expect(getBitbucketConnector()).toMatchObject({
119119
providerId: 'bitbucket',
120120
authorizationUrl: 'https://bitbucket.org/site/oauth2/authorize',
@@ -132,7 +132,7 @@ describe('Bitbucket OAuth Connector', () => {
132132
responseType: 'code',
133133
pkce: false,
134134
authentication: 'basic',
135-
accessTokenExpiresIn: 3600,
135+
accessTokenExpiresIn: 7200,
136136
redirectURI: 'http://localhost:3000/api/auth/oauth2/callback/bitbucket',
137137
})
138138
})

apps/sim/tools/bitbucket/get_merge_task_status.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ export const bitbucketGetMergeTaskStatusTool: ToolConfig<
6161
transformResponse: async (response) => {
6262
const data = await bitbucketJson(response)
6363
if (data.type === 'error') {
64-
const message = stringField(record(data.error)?.message)
65-
if (!message?.trim()) throw new Error('Bitbucket returned a malformed merge task error')
66-
throw new Error(message)
64+
const error = record(data.error)
65+
const message = stringField(error?.message)?.trim()
66+
if (!message) throw new Error('Bitbucket returned a malformed merge task error')
67+
const detail = stringField(error?.detail)?.trim()
68+
throw new Error(detail && detail !== message ? `${message}: ${detail}` : message)
6769
}
6870

6971
const taskStatus = data.task_status

apps/sim/tools/bitbucket/get_pipeline_step_log.ts

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ import type {
33
BitbucketToolResponse,
44
} from '@/tools/bitbucket/types'
55
import {
6+
assertBitbucketResponseOk,
67
BITBUCKET_API_BASE,
78
BITBUCKET_DEFAULT_LOG_CHARACTERS,
89
BITBUCKET_ERROR_EXTRACTOR,
10+
BITBUCKET_LOG_TRANSFER_MAX_BYTES,
911
BITBUCKET_READ_RETRY,
1012
BITBUCKET_REPOSITORY_PARAMS,
1113
bitbucketHeaders,
14+
bitbucketMaxCharacters,
1215
bitbucketRawTail,
1316
bitbucketRepositoryPath,
1417
bitbucketTailRange,
@@ -22,6 +25,15 @@ interface BitbucketPipelineLogOutput {
2225
totalBytes: number | null
2326
}
2427

28+
/** A suffix range against an empty log is unsatisfiable; Bitbucket answers 416 rather than 200. */
29+
const BITBUCKET_RANGE_NOT_SATISFIABLE = 416
30+
/** RFC 7233 unsatisfied-range header for a zero-length log; any other 416 is a genuine failure. */
31+
const EMPTY_CONTENT_RANGE_PATTERN = /^bytes \*\/0$/
32+
33+
function stepLogUrl(params: BitbucketGetPipelineStepLogParams): string {
34+
return `${BITBUCKET_API_BASE}${bitbucketRepositoryPath(params.workspaceSlug, params.repoSlug)}/pipelines/${encodeBitbucketSegment(params.pipelineUuid, 'pipelineUuid')}/steps/${encodeBitbucketSegment(params.stepUuid, 'stepUuid')}/log`
35+
}
36+
2537
export const bitbucketGetPipelineStepLogTool: ToolConfig<
2638
BitbucketGetPipelineStepLogParams,
2739
BitbucketToolResponse<BitbucketPipelineLogOutput>
@@ -53,9 +65,30 @@ export const bitbucketGetPipelineStepLogTool: ToolConfig<
5365
default: BITBUCKET_DEFAULT_LOG_CHARACTERS,
5466
},
5567
},
68+
directExecution: async (params, signal) => {
69+
bitbucketMaxCharacters(params.maxCharacters, true)
70+
const { secureBitbucketRead } = await import('@/tools/bitbucket/utils.server')
71+
const response = await secureBitbucketRead(
72+
stepLogUrl(params),
73+
bitbucketHeaders(params.accessToken, {
74+
json: false,
75+
range: bitbucketTailRange(params.maxCharacters),
76+
}),
77+
BITBUCKET_LOG_TRANSFER_MAX_BYTES,
78+
{ stripAuthOnRedirect: true, signal }
79+
)
80+
if (
81+
response.status === BITBUCKET_RANGE_NOT_SATISFIABLE &&
82+
EMPTY_CONTENT_RANGE_PATTERN.test(response.headers.get('content-range') ?? '')
83+
) {
84+
await response.body?.cancel()
85+
return { success: true, output: { log: '', truncated: false, totalBytes: 0 } }
86+
}
87+
await assertBitbucketResponseOk(response)
88+
return { success: true, output: await bitbucketRawTail(response, params.maxCharacters) }
89+
},
5690
request: {
57-
url: (params) =>
58-
`${BITBUCKET_API_BASE}${bitbucketRepositoryPath(params.workspaceSlug, params.repoSlug)}/pipelines/${encodeBitbucketSegment(params.pipelineUuid, 'pipelineUuid')}/steps/${encodeBitbucketSegment(params.stepUuid, 'stepUuid')}/log`,
91+
url: stepLogUrl,
5992
method: 'GET',
6093
headers: (params) =>
6194
bitbucketHeaders(params.accessToken, {
@@ -65,10 +98,9 @@ export const bitbucketGetPipelineStepLogTool: ToolConfig<
6598
retry: BITBUCKET_READ_RETRY,
6699
stripAuthOnRedirect: true,
67100
},
68-
transformResponse: async (response, params) => ({
69-
success: true,
70-
output: await bitbucketRawTail(response, params?.maxCharacters),
71-
}),
101+
transformResponse: async () => {
102+
throw new Error('Bitbucket step-log reads require the byte-capped direct execution path')
103+
},
72104
outputs: {
73105
log: { type: 'string', description: 'Bounded trailing UTF-8 log text' },
74106
truncated: { type: 'boolean', description: 'Whether earlier log output was omitted' },

0 commit comments

Comments
 (0)