Skip to content

Commit e02e8d9

Browse files
committed
fix(credentials): address the final audit
An Azure DevOps project or organization pasted already encoded is decoded once before the traversal-safe segment, so it isn't encoded twice; a base URL with a single-slash scheme is refused; Upstash accepts a proxy path like the legacy block; a Thrive tenant id must be printable ASCII.
1 parent ad46150 commit e02e8d9

5 files changed

Lines changed: 37 additions & 15 deletions

File tree

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
99

10-
import { tokenCheckValidator } from '@/lib/credentials/token-service-accounts/validators/http'
10+
import {
11+
normalizeHttpBaseUrl,
12+
tokenCheckValidator,
13+
} from '@/lib/credentials/token-service-accounts/validators/http'
1114

1215
const validate = tokenCheckValidator({
1316
serviceLabel: 'Example',
@@ -156,6 +159,13 @@ describe('tokenCheckValidator with a typed host', () => {
156159
)
157160
})
158161

162+
it('refuses a base URL with a mistyped scheme instead of reading the scheme as a host', () => {
163+
expect(() => normalizeHttpBaseUrl('https:/grafana.example.com')).toThrow(/scheme/)
164+
expect(normalizeHttpBaseUrl('grafana.example.com:3000/')).toBe(
165+
'https://grafana.example.com:3000'
166+
)
167+
})
168+
159169
it('refuses a host its normalizer rejects without sending the token anywhere', async () => {
160170
await expect(
161171
validateHost({ apiToken: 'good', domain: 'evil.example.com@tools.example.com' })

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,19 @@ export function readJsonBody(response: ProviderResponse, serviceLabel: string):
9898
* A typed http(s) base URL in the form tools build request paths on: a bare host
9999
* is taken as https, trailing slashes are dropped, and any other scheme,
100100
* credentials, a query, or a fragment is refused, since each would retarget the
101-
* request. `allowPath: false` also refuses a path, for tools that append to the
102-
* origin.
101+
* request.
103102
*/
104-
export function normalizeHttpBaseUrl(
105-
raw: string | undefined,
106-
{ allowPath = true }: { allowPath?: boolean } = {}
107-
): string {
103+
export function normalizeHttpBaseUrl(raw: string | undefined): string {
108104
const typed = (raw ?? '').trim()
109105
if (!typed) throw new Error('missing URL')
106+
// `https:/host` would otherwise get a second scheme and parse as a host named `https`.
107+
if (/^https?:(?!\/\/)/i.test(typed)) throw new Error('malformed scheme')
110108
const url = new URL(/^[a-z][a-z\d+.-]*:\/\//i.test(typed) ? typed : `https://${typed}`)
111109
if (url.protocol !== 'https:' && url.protocol !== 'http:') throw new Error('http(s) only')
112110
if (url.username || url.password || url.search || url.hash) {
113111
throw new Error('credentials, query, or fragment in URL')
114112
}
115-
const path = url.pathname.replace(/\/+$/, '')
116-
if (path && !allowPath) throw new Error('path not allowed')
117-
return `${url.origin}${path}`
113+
return `${url.origin}${url.pathname.replace(/\/+$/, '')}`
118114
}
119115

120116
/**

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,8 +1313,8 @@ export const validateThriveServiceAccount = tokenCheckValidator({
13131313
domain: allowedDomain(THRIVE_HOSTS),
13141314
workspaceId: ({ providerWorkspaceId }) => {
13151315
const tenantId = (providerWorkspaceId ?? '').trim()
1316-
// The tenant id is the Basic-auth username, so a colon would split it.
1317-
if (!tenantId || /[:\s]/.test(tenantId)) throw new Error('invalid tenant id')
1316+
// The tenant id is the Basic-auth username: printable ASCII, and no colon, which would split it.
1317+
if (!/^[\x21-\x39\x3b-\x7e]+$/.test(tenantId)) throw new Error('invalid tenant id')
13181318
return tenantId
13191319
},
13201320
request: ({ apiToken, domain, providerWorkspaceId = '' }) => ({
@@ -1397,7 +1397,7 @@ export const validateTypeformServiceAccount = tokenCheckValidator({
13971397
*/
13981398
export const validateUpstashServiceAccount = tokenCheckValidator({
13991399
serviceLabel: 'Upstash',
1400-
domain: ({ domain }) => normalizeHttpBaseUrl(domain, { allowPath: false }),
1400+
domain: ({ domain }) => normalizeHttpBaseUrl(domain),
14011401
egress: 'configuredEndpoint',
14021402
request: ({ apiToken, domain }) => ({
14031403
url: `${domain}/ping`,

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,13 @@ describe('Azure DevOps project-scoped URLs', () => {
564564
expect(url.pathname).toBe('/contoso/My%20Project/_apis/wit/workitems/7')
565565
})
566566

567-
it.each(['../../other-org/proj', '..', 'proj/../../other-org'])(
567+
it('sends a project name pasted already encoded from the address bar unchanged', () => {
568+
const url = new URL(buildUrl(getWorkItemTool, { ...base, project: 'My%20Project' } as never))
569+
570+
expect(url.pathname).toBe('/contoso/My%20Project/_apis/wit/workitems/7')
571+
})
572+
573+
it.each(['../../other-org/proj', '..', 'proj/../../other-org', '%2e%2e'])(
568574
'refuses a project that would climb into another organization (%s)',
569575
(project) => {
570576
expect(() => buildUrl(getWorkItemTool, { ...base, project } as never)).toThrow(/project/)

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,17 @@ import { safeUrlPathSegment } from '@/tools/url-path'
77
* climb into another organization's endpoints with the caller's token.
88
*/
99
export function azureDevOpsProjectUrl(params: { organization: string; project: string }): string {
10-
return `https://dev.azure.com/${safeUrlPathSegment(params.organization.trim(), 'organization')}/${safeUrlPathSegment(params.project.trim(), 'project')}`
10+
return `https://dev.azure.com/${pathSegment(params.organization, 'organization')}/${pathSegment(params.project, 'project')}`
11+
}
12+
13+
/** A name copied from the browser's address bar arrives encoded; decode it once so it isn't encoded twice. */
14+
function pathSegment(value: string, paramName: string): string {
15+
const typed = value.trim()
16+
let decoded = typed
17+
try {
18+
decoded = decodeURIComponent(typed)
19+
} catch {}
20+
return safeUrlPathSegment(decoded, paramName)
1121
}
1222

1323
/** States for Azure DevOps Basic process work items (Issue, Task, Epic). */

0 commit comments

Comments
 (0)