Skip to content

Commit 1d60a2e

Browse files
committed
fix(hubspot): guard objectType in the selector routes and the poller
objectType reaches a provider URL from a free-text customObjectTypeId, via two selector routes and the background poller. Both surfaces are authenticated calls with the caller's own credential against a fixed host, so this is request re-aiming, not exfiltration. Also closes two discovery blind spots in the path_safety suite: a number-typed path param was invisible to the sentinel, and a param sharing a segment resolved to -1.
1 parent 59c4b7f commit 1d60a2e

6 files changed

Lines changed: 457 additions & 18 deletions

File tree

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* The HubSpot `properties` and `pipelines` selector routes assemble their
5+
* provider URL *here*, from a query parameter, so the reflective `request.url`
6+
* probe in `tools/hubspot/path_safety.test.ts` cannot see them.
7+
*
8+
* `objectType` is genuinely caller-supplied — the contract constrains it only
9+
* to a non-empty string, and the selector resolves it from the free-text
10+
* `customObjectTypeId` trigger field. `BUILT_IN_PATH` maps the four known
11+
* slugs to a safe constant, but the `?? objectType` fallback puts the raw value
12+
* straight into a path segment. `encodeURIComponent` does not help: `.` and
13+
* `..` are unreserved, and the WHATWG parser removes a dot segment *after*
14+
* percent-decoding, so `objectType='..'` re-aims the request one level up with
15+
* the caller's HubSpot bearer token still attached.
16+
*
17+
* Every assertion resolves the outgoing URL through `new URL(...)`, the same
18+
* normalization `fetch` applies, and compares whole-pathname segment shape
19+
* rather than the template text.
20+
*/
21+
import { createMockRequest } from '@sim/testing'
22+
import { beforeEach, describe, expect, it, vi } from 'vitest'
23+
24+
const { mockAuthorizeCredentialUse, mockRefreshAccessToken } = vi.hoisted(() => ({
25+
mockAuthorizeCredentialUse: vi.fn(),
26+
mockRefreshAccessToken: vi.fn(),
27+
}))
28+
29+
vi.mock('@/lib/auth/credential-access', () => ({
30+
authorizeCredentialUse: mockAuthorizeCredentialUse,
31+
}))
32+
33+
vi.mock('@/lib/oauth/credential-service', () => ({
34+
refreshAccessTokenIfNeeded: mockRefreshAccessToken,
35+
}))
36+
37+
import { GET as GET_PIPELINES } from '@/app/api/tools/hubspot/pipelines/route'
38+
import { GET as GET_PROPERTIES } from '@/app/api/tools/hubspot/properties/route'
39+
40+
const ORIGIN = 'https://api.hubapi.com'
41+
const CREDENTIAL_ID = 'cred-1'
42+
43+
/**
44+
* Values that must be rejected outright. `' .. '` is included because a
45+
* padded dot segment is only inert if nothing later trims it.
46+
*/
47+
const REJECTED = ['..', '.', '%2e%2e', 'a/b', ' .. ', '..%2f..', '../..'] as const
48+
49+
/**
50+
* The three legal `objectType` spellings from HubSpot's Schemas API —
51+
* portal-qualified custom object, `{meta-type}-{unique id}`, and a bare custom
52+
* object name — plus the built-in slug that maps through `BUILT_IN_PATH`.
53+
*/
54+
const LEGITIMATE = ['p7878787_my_object', '2-123456', '0-1', 'my_object'] as const
55+
56+
let fetchMock: ReturnType<typeof vi.fn>
57+
58+
beforeEach(() => {
59+
vi.clearAllMocks()
60+
mockAuthorizeCredentialUse.mockResolvedValue({
61+
ok: true,
62+
credentialOwnerUserId: 'user-1',
63+
resolvedCredentialId: CREDENTIAL_ID,
64+
})
65+
mockRefreshAccessToken.mockResolvedValue('inert-token')
66+
fetchMock = vi.fn().mockResolvedValue({
67+
ok: true,
68+
status: 200,
69+
json: async () => ({ results: [] }),
70+
text: async () => '',
71+
})
72+
vi.stubGlobal('fetch', fetchMock)
73+
})
74+
75+
function request(path: string, objectType: string) {
76+
const url = new URL(`http://localhost:3000/api/tools/hubspot/${path}`)
77+
url.searchParams.set('credentialId', CREDENTIAL_ID)
78+
url.searchParams.set('objectType', objectType)
79+
return createMockRequest('GET', undefined, {}, url.toString())
80+
}
81+
82+
function outgoingUrl(): URL {
83+
expect(fetchMock).toHaveBeenCalledTimes(1)
84+
return new URL(fetchMock.mock.calls[0][0] as string)
85+
}
86+
87+
const ROUTES = [
88+
{ name: 'properties', handler: GET_PROPERTIES, collection: 'properties' },
89+
{ name: 'pipelines', handler: GET_PIPELINES, collection: 'pipelines' },
90+
] as const
91+
92+
describe.each(ROUTES)(
93+
'GET /api/tools/hubspot/$name path safety',
94+
({ name, handler, collection }) => {
95+
it('maps a built-in slug to its documented plural constant', async () => {
96+
const response = await handler(request(name, 'contact'))
97+
expect(response.status).toBe(200)
98+
99+
const url = outgoingUrl()
100+
expect(url.origin).toBe(ORIGIN)
101+
expect(url.pathname.split('/')).toEqual(['', 'crm', 'v3', collection, 'contacts'])
102+
expect([...url.searchParams.keys()]).toEqual([])
103+
})
104+
105+
it.each(LEGITIMATE)('passes %j through byte-identically', async (objectType) => {
106+
const response = await handler(request(name, objectType))
107+
expect(response.status).toBe(200)
108+
109+
const url = outgoingUrl()
110+
expect(fetchMock.mock.calls[0][0]).toBe(`${ORIGIN}/crm/v3/${collection}/${objectType}`)
111+
expect(url.pathname.split('/')).toEqual(['', 'crm', 'v3', collection, objectType])
112+
})
113+
114+
it.each(REJECTED)(
115+
'rejects objectType=%j with a 400 and never calls HubSpot',
116+
async (objectType) => {
117+
const response = await handler(request(name, objectType))
118+
119+
expect(response.status).toBe(400)
120+
expect((await response.json()).error).toMatch(/objectType/)
121+
expect(fetchMock).not.toHaveBeenCalled()
122+
}
123+
)
124+
}
125+
)

apps/sim/app/api/tools/hubspot/pipelines/route.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { type NextRequest, NextResponse } from 'next/server'
33
import { hubspotPipelinesSelectorContract } from '@/lib/api/contracts/selectors/hubspot'
44
import { parseRequest } from '@/lib/api/server'
55
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
6-
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
6+
import { validateAlphanumericId, validatePathSegment } from '@/lib/core/security/input-validation'
77
import { generateRequestId } from '@/lib/core/utils/request'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service'
@@ -12,6 +12,13 @@ export const dynamic = 'force-dynamic'
1212

1313
const logger = createLogger('HubSpotPipelinesAPI')
1414

15+
/**
16+
* Built-in object slugs map to a safe plural constant; anything else falls
17+
* through to the caller-supplied `objectType`, which the contract constrains
18+
* only to a non-empty string. That value lands in a path segment, so it is
19+
* validated before use — `encodeURIComponent` alone leaves a `.`/`..` segment
20+
* intact and the WHATWG parser removes it, re-aiming the authenticated request.
21+
*/
1522
const BUILT_IN_PATH: Record<string, string> = {
1623
contact: 'contacts',
1724
company: 'companies',
@@ -58,6 +65,12 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5865
}
5966

6067
const pathSegment = BUILT_IN_PATH[objectType] ?? objectType
68+
const pathSegmentValidation = validatePathSegment(pathSegment, { paramName: 'objectType' })
69+
if (!pathSegmentValidation.isValid) {
70+
logger.warn(`[${requestId}] Invalid objectType: ${pathSegmentValidation.error}`)
71+
return NextResponse.json({ error: pathSegmentValidation.error }, { status: 400 })
72+
}
73+
6174
const response = await fetch(
6275
`https://api.hubapi.com/crm/v3/pipelines/${encodeURIComponent(pathSegment)}`,
6376
{ headers: { Authorization: `Bearer ${accessToken}` } }

apps/sim/app/api/tools/hubspot/properties/route.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { type NextRequest, NextResponse } from 'next/server'
33
import { hubspotPropertiesSelectorContract } from '@/lib/api/contracts/selectors/hubspot'
44
import { parseRequest } from '@/lib/api/server'
55
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
6-
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
6+
import { validateAlphanumericId, validatePathSegment } from '@/lib/core/security/input-validation'
77
import { generateRequestId } from '@/lib/core/utils/request'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service'
@@ -12,6 +12,13 @@ export const dynamic = 'force-dynamic'
1212

1313
const logger = createLogger('HubSpotPropertiesAPI')
1414

15+
/**
16+
* Built-in object slugs map to a safe plural constant; anything else falls
17+
* through to the caller-supplied `objectType`, which the contract constrains
18+
* only to a non-empty string. That value lands in a path segment, so it is
19+
* validated before use — `encodeURIComponent` alone leaves a `.`/`..` segment
20+
* intact and the WHATWG parser removes it, re-aiming the authenticated request.
21+
*/
1522
const BUILT_IN_PATH: Record<string, string> = {
1623
contact: 'contacts',
1724
company: 'companies',
@@ -61,6 +68,12 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6168
}
6269

6370
const pathSegment = BUILT_IN_PATH[objectType] ?? objectType
71+
const pathSegmentValidation = validatePathSegment(pathSegment, { paramName: 'objectType' })
72+
if (!pathSegmentValidation.isValid) {
73+
logger.warn(`[${requestId}] Invalid objectType: ${pathSegmentValidation.error}`)
74+
return NextResponse.json({ error: pathSegmentValidation.error }, { status: 400 })
75+
}
76+
6477
const response = await fetch(
6578
`https://api.hubapi.com/crm/v3/properties/${encodeURIComponent(pathSegment)}`,
6679
{ headers: { Authorization: `Bearer ${accessToken}` } }

0 commit comments

Comments
 (0)