Skip to content

Commit c5b0bbc

Browse files
committed
fix(credentials): address review round 1
Azure DevOps tools build every project-scoped URL through one traversal-safe helper, so a project or build id can't climb into another organization with a saved token; Tinybird and Upstash restricted-token replies are covered by tests.
1 parent b9e8099 commit c5b0bbc

19 files changed

Lines changed: 104 additions & 32 deletions

‎apps/sim/lib/credentials/token-service-accounts/validators/token-checks.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,3 +392,41 @@ describe('saved credentials for a host the user chose', () => {
392392
}
393393
)
394394
})
395+
396+
describe('saved credentials whose vendor answers a restricted token in its error', () => {
397+
const secureFetch = inputValidationMockFns.mockSecureFetchWithValidation
398+
399+
it.each([
400+
[
401+
'rejects an unknown Tinybird token',
402+
validateTinybirdServiceAccount,
403+
jsonResponse({ error: 'invalid authentication token. Token tok not found' }, 403),
404+
'invalid_credentials',
405+
],
406+
[
407+
'accepts a Tinybird token without read access',
408+
validateTinybirdServiceAccount,
409+
jsonResponse({ error: 'Not enough permissions to read from datasources' }, 403),
410+
null,
411+
],
412+
[
413+
'accepts an Upstash token whose ACL refuses PING',
414+
validateUpstashServiceAccount,
415+
jsonResponse({ error: 'NOPERM this user has no permissions to run the ping command' }, 400),
416+
null,
417+
],
418+
[
419+
'rejects an unknown Upstash token',
420+
validateUpstashServiceAccount,
421+
jsonResponse({ error: 'WRONGPASS invalid or missing auth token' }, 401),
422+
'invalid_credentials',
423+
],
424+
])('%s', async (_, validate, reply, code) => {
425+
secureFetch.mockReset().mockResolvedValue(reply)
426+
427+
const result = validate({ apiToken: 'tok', domain: 'https://instance.example.com' })
428+
429+
if (code) await expect(result).rejects.toMatchObject({ code })
430+
else await expect(result).resolves.toMatchObject({ principal: null })
431+
})
432+
})

‎apps/sim/tools/azure_devops/add_comment.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type {
44
AzureDevOpsComment,
55
} from '@/tools/azure_devops/types'
66
import type { AzureDevOpsRawComment } from '@/tools/azure_devops/utils'
7-
import { formatComment, mapComment } from '@/tools/azure_devops/utils'
7+
import { azureDevOpsProjectUrl, formatComment, mapComment } from '@/tools/azure_devops/utils'
88
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
99
import type { ToolConfig } from '@/tools/types'
1010

@@ -51,7 +51,7 @@ export const addCommentTool: ToolConfig<AddCommentParams, AddCommentResponse> =
5151
request: {
5252
url: (params) => {
5353
const url = new URL(
54-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/wit/workitems/${Number(params.workItemId)}/comments`
54+
`${azureDevOpsProjectUrl(params)}/_apis/wit/workitems/${Number(params.workItemId)}/comments`
5555
)
5656
url.searchParams.set('api-version', '7.0-preview.3')
5757
return url.toString()

‎apps/sim/tools/azure_devops/azure-devops.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,3 +554,20 @@ describe('Azure DevOps trigger event matching', () => {
554554
expect(azureDevOpsHandler.extractIdempotencyId!(null)).toBeNull()
555555
})
556556
})
557+
558+
describe('Azure DevOps project-scoped URLs', () => {
559+
const base = { accessToken: 'pat', organization: 'contoso', workItemId: 7 }
560+
561+
it('keeps a project name with spaces in its own path segment, as before', () => {
562+
const url = new URL(buildUrl(getWorkItemTool, { ...base, project: 'My Project' } as never))
563+
564+
expect(url.pathname).toBe('/contoso/My%20Project/_apis/wit/workitems/7')
565+
})
566+
567+
it.each(['../../other-org/proj', '..', 'proj/../../other-org'])(
568+
'refuses a project that would climb into another organization (%s)',
569+
(project) => {
570+
expect(() => buildUrl(getWorkItemTool, { ...base, project } as never)).toThrow(/project/)
571+
}
572+
)
573+
})

‎apps/sim/tools/azure_devops/create_work_item.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { AzureDevOpsJsonPatchOp, AzureDevOpsRawWorkItem } from '@/tools/azu
77
import {
88
appendEffortPatchOp,
99
appendFieldPatchOp,
10+
azureDevOpsProjectUrl,
1011
formatWorkItem,
1112
mapWorkItem,
1213
} from '@/tools/azure_devops/utils'
@@ -134,7 +135,7 @@ export const createWorkItemTool: ToolConfig<CreateWorkItemParams, CreateWorkItem
134135

135136
request: {
136137
url: (params) =>
137-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/wit/workitems/$${encodeURIComponent(params.workItemType)}?api-version=7.2-preview.3`,
138+
`${azureDevOpsProjectUrl(params)}/_apis/wit/workitems/$${encodeURIComponent(params.workItemType)}?api-version=7.2-preview.3`,
138139
method: 'POST',
139140
headers: (params) => ({
140141
'Content-Type': 'application/json-patch+json',

‎apps/sim/tools/azure_devops/get_build_log.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import type { GetBuildLogParams, GetBuildLogResponse } from '@/tools/azure_devops/types'
2+
import { azureDevOpsProjectUrl } from '@/tools/azure_devops/utils'
23
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
34
import type { ToolConfig } from '@/tools/types'
5+
import { safeUrlPathSegment } from '@/tools/url-path'
46

57
export const getBuildLogTool: ToolConfig<GetBuildLogParams, GetBuildLogResponse> = {
68
id: 'azure_devops_get_build_log',
@@ -58,7 +60,7 @@ export const getBuildLogTool: ToolConfig<GetBuildLogParams, GetBuildLogResponse>
5860
request: {
5961
url: (params) => {
6062
const url = new URL(
61-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/build/builds/${params.buildId}/logs/${params.logId}`
63+
`${azureDevOpsProjectUrl(params)}/_apis/build/builds/${safeUrlPathSegment(params.buildId, 'buildId')}/logs/${safeUrlPathSegment(params.logId, 'logId')}`
6264
)
6365
url.searchParams.set('api-version', '7.2-preview.2')
6466
if (params.startLine !== undefined)

‎apps/sim/tools/azure_devops/get_build_timeline.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {
33
GetBuildTimelineParams,
44
GetBuildTimelineResponse,
55
} from '@/tools/azure_devops/types'
6+
import { azureDevOpsProjectUrl } from '@/tools/azure_devops/utils'
67
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
78
import type { ToolConfig } from '@/tools/types'
89

@@ -43,7 +44,7 @@ export const getBuildTimelineTool: ToolConfig<GetBuildTimelineParams, GetBuildTi
4344

4445
request: {
4546
url: (params) =>
46-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/build/builds/${Number(params.buildId)}/timeline?api-version=7.2-preview.3`,
47+
`${azureDevOpsProjectUrl(params)}/_apis/build/builds/${Number(params.buildId)}/timeline?api-version=7.2-preview.3`,
4748
method: 'GET',
4849
headers: (params) => ({
4950
'Content-Type': 'application/json',

‎apps/sim/tools/azure_devops/get_comments.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type {
44
GetCommentsResponse,
55
} from '@/tools/azure_devops/types'
66
import type { AzureDevOpsRawComment } from '@/tools/azure_devops/utils'
7-
import { formatComment, mapComment } from '@/tools/azure_devops/utils'
7+
import { azureDevOpsProjectUrl, formatComment, mapComment } from '@/tools/azure_devops/utils'
88
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
99
import type { ToolConfig } from '@/tools/types'
1010

@@ -76,7 +76,7 @@ export const getCommentsTool: ToolConfig<GetCommentsParams, GetCommentsResponse>
7676
request: {
7777
url: (params) => {
7878
const url = new URL(
79-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/wit/workitems/${Number(params.workItemId)}/comments`
79+
`${azureDevOpsProjectUrl(params)}/_apis/wit/workitems/${Number(params.workItemId)}/comments`
8080
)
8181
url.searchParams.set('api-version', '7.2-preview.4')
8282
if (params.top) url.searchParams.set('$top', Number(params.top).toString())

‎apps/sim/tools/azure_devops/get_pipeline.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import type { GetPipelineParams, GetPipelineResponse } from '@/tools/azure_devops/types'
2+
import { azureDevOpsProjectUrl } from '@/tools/azure_devops/utils'
23
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
34
import type { ToolConfig } from '@/tools/types'
5+
import { safeUrlPathSegment } from '@/tools/url-path'
46

57
export const getPipelineTool: ToolConfig<GetPipelineParams, GetPipelineResponse> = {
68
id: 'azure_devops_get_pipeline',
@@ -46,7 +48,7 @@ export const getPipelineTool: ToolConfig<GetPipelineParams, GetPipelineResponse>
4648
request: {
4749
url: (params) => {
4850
const url = new URL(
49-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/pipelines/${params.pipelineId}`
51+
`${azureDevOpsProjectUrl(params)}/_apis/pipelines/${safeUrlPathSegment(params.pipelineId, 'pipelineId')}`
5052
)
5153
url.searchParams.set('api-version', '7.2-preview.1')
5254
if (params.pipelineVersion)

‎apps/sim/tools/azure_devops/get_pipeline_run.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import type { GetPipelineRunParams, GetPipelineRunResponse } from '@/tools/azure_devops/types'
2+
import { azureDevOpsProjectUrl } from '@/tools/azure_devops/utils'
23
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
34
import type { ToolConfig } from '@/tools/types'
5+
import { safeUrlPathSegment } from '@/tools/url-path'
46

57
export const getPipelineRunTool: ToolConfig<GetPipelineRunParams, GetPipelineRunResponse> = {
68
id: 'azure_devops_get_pipeline_run',
@@ -46,7 +48,7 @@ export const getPipelineRunTool: ToolConfig<GetPipelineRunParams, GetPipelineRun
4648
request: {
4749
url: (params) => {
4850
const url = new URL(
49-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/pipelines/${params.pipelineId}/runs/${params.runId}`
51+
`${azureDevOpsProjectUrl(params)}/_apis/pipelines/${safeUrlPathSegment(params.pipelineId, 'pipelineId')}/runs/${safeUrlPathSegment(params.runId, 'runId')}`
5052
)
5153
url.searchParams.set('api-version', '7.2-preview.1')
5254
return url.toString()

‎apps/sim/tools/azure_devops/get_work_item.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { GetWorkItemParams, GetWorkItemResponse } from '@/tools/azure_devops/types'
22
import type { AzureDevOpsRawWorkItem } from '@/tools/azure_devops/utils'
3-
import { formatWorkItem, mapWorkItem } from '@/tools/azure_devops/utils'
3+
import { azureDevOpsProjectUrl, formatWorkItem, mapWorkItem } from '@/tools/azure_devops/utils'
44
import { AZURE_DEVOPS_CREDENTIAL_OAUTH } from '@/tools/shared/saved-credentials'
55
import type { ToolConfig } from '@/tools/types'
66

@@ -42,7 +42,7 @@ export const getWorkItemTool: ToolConfig<GetWorkItemParams, GetWorkItemResponse>
4242
request: {
4343
url: (params) => {
4444
const url = new URL(
45-
`https://dev.azure.com/${params.organization.trim()}/${params.project.trim()}/_apis/wit/workitems/${Number(params.workItemId)}`
45+
`${azureDevOpsProjectUrl(params)}/_apis/wit/workitems/${Number(params.workItemId)}`
4646
)
4747
url.searchParams.set('$expand', 'all')
4848
url.searchParams.set('api-version', '7.2-preview.3')

0 commit comments

Comments
 (0)