test(hubspot,stripe,salesforce): fuzz path params one at a time

The suites inherited a coverage hole from the Vercel template they were
modelled on: `buildParams` filled every string parameter with the same
fuzz value and the assertion swallowed a throw with `catch { return }`.
The first guarded parameter therefore threw and silently retired every
sibling from the run, so once a tool had one guard the rest of its path
parameters stopped being tested. The reported property — "a newly added
unguarded path parameter fails CI" — did not hold.

HubSpot is where this bit hardest: the association tools put four
parameters (`objectType`, `objectId`, `toObjectType`, `toObjectId`) into
one path, so three of them were never exercised. Reverting the `objectId`
guard in `delete_association` produced zero failures.

The unit of coverage is now a (tool, parameter) pair. Each case fuzzes
exactly one parameter and pins every sibling to a safe placeholder, so a
throw is attributable to the parameter under test and is only then a
pass. Discovery enumerates pairs from the barrel, so a new unguarded path
parameter on an already-guarded tool now fails.

Each pair also asserts a bare `.` and `..` are rejected outright. The
shape check alone cannot see a dot in the final segment: `x/.` normalizes
to `x/`, preserving both the segment count and every other segment.

Adds a fixture pinning the exact path-parameter set of the three
association tools, so dropping one from the sweep fails rather than
quietly shrinking coverage.

92 pairs total: 28 Stripe, 41 HubSpot, 23 Salesforce. 2247 assertions.

Author: waleedlatif1

apps/sim/tools/hubspot/path_safety.test.ts

@@ -17,20 +17,33 @@
  * Every assertion here resolves the built URL with `new URL(...)` — the same
  * normalization `fetch` performs — rather than string-matching the template
  * output, because string matching is exactly what let this through.
+ *
+ * The unit of coverage is a **(tool, parameter) pair**, not a tool. Fuzzing
+ * every parameter of a tool at once and tolerating a throw is unsound: the
+ * first guarded parameter throws and silently retires every sibling from the
+ * suite. HubSpot is where that mattered most — the association tools put the
+ * four parameters `objectType`, `objectId`, `toObjectType` and `toObjectType`
+ * into a single path, so guarding the first one would have retired the other
+ * three. Each case below fuzzes exactly one parameter while holding its
+ * siblings at a safe value, which is what makes "a newly added unguarded path
+ * parameter fails CI" actually true. A throw is only accepted as a pass
+ * *because* the thrown-at parameter is the one under test.
  */
 import { describe, expect, it } from 'vitest'
+import { hubspotCreateAssociationTool } from '@/tools/hubspot/create_association'
 import { hubspotDeleteAssociationTool } from '@/tools/hubspot/delete_association'
 import { hubspotDeleteCompanyTool } from '@/tools/hubspot/delete_company'
 import { hubspotDeleteContactTool } from '@/tools/hubspot/delete_contact'
 import { hubspotDeleteDealTool } from '@/tools/hubspot/delete_deal'
 import * as hubspotTools from '@/tools/hubspot/index'
+import { hubspotListAssociationsTool } from '@/tools/hubspot/list_associations'
 import type { ToolConfig } from '@/tools/types'
 
 /**
  * The bare `.` and `..` entries are the whole point: their omission is why an
  * `encodeURIComponent`-only fix looks correct while the hole stays live.
  */
-const TRAVERSAL_IDS = [
+const TRAVERSAL_VALUES = [
   '..',
   '.',
   '  ..  ',
@@ -57,10 +70,17 @@ const LEGITIMATE_IDS = [
   'hs_object_id',
 ] as const
 
-const SAFE_ID = 'SAFEID'
+/** The value the parameter under test carries when a path is being mapped. */
+const TARGET = 'TARGETVALUE'
+
+/** The value every *other* string parameter is pinned to while one is fuzzed. */
+const SIBLING = 'SIBLINGVALUE'
 
 type AnyTool = ToolConfig<any, any>
 
+/** Parameters that carry credentials or the host, never a path segment. */
+const FIXED_PARAMS: Record<string, unknown> = { accessToken: 'token' }
+
 function isHubSpotTool(value: unknown): value is AnyTool {
   return (
     typeof value === 'object' &&
@@ -70,112 +90,182 @@ function isHubSpotTool(value: unknown): value is AnyTool {
   )
 }
 
+/** The placeholder a sibling parameter holds, chosen so the URL still builds. */
+function siblingValue(type: string | undefined): unknown {
+  if (type === 'json' || type === 'array') return []
+  if (type === 'number') return 1
+  if (type === 'boolean') return false
+  return SIBLING
+}
+
 /**
- * Builds a param object for a tool, filling every declared string param with
- * `value` so whichever one reaches the path is exercised.
+ * Builds a param object with exactly one parameter under test.
+ *
+ * Every sibling is pinned to a safe placeholder, so a failure can only be
+ * attributed to `target`. This is the whole difference from a fill-everything
+ * sweep, where the first parameter to throw hides all the others.
  */
-function buildParams(tool: AnyTool, value: string): Record<string, unknown> {
-  const params: Record<string, unknown> = { accessToken: 'token' }
+function buildParams(tool: AnyTool, target: string, value: string): Record<string, unknown> {
+  const params: Record<string, unknown> = { ...FIXED_PARAMS }
   for (const [name, def] of Object.entries(tool.params ?? {})) {
-    if (name === 'accessToken') continue
-    const type = (def as { type?: string }).type
-    if (type === 'json' || type === 'array') {
-      params[name] = []
-    } else if (type === 'number') {
-      params[name] = 1
-    } else if (type === 'boolean') {
-      params[name] = false
-    } else {
-      params[name] = value
-    }
+    if (name in FIXED_PARAMS) continue
+    params[name] = name === target ? value : siblingValue((def as { type?: string }).type)
   }
   return params
 }
 
-function buildUrl(tool: AnyTool, value: string): URL {
+function buildUrl(tool: AnyTool, target: string, value: string): URL {
   const url = tool.request?.url
   if (typeof url !== 'function') {
     throw new Error(`${tool.id} does not build its URL from params`)
   }
-  return new URL(url(buildParams(tool, value) as any))
+  return new URL(url(buildParams(tool, target, value) as any))
 }
 
 function segmentsOf(pathname: string): string[] {
   return pathname.split('/')
 }
 
-const DYNAMIC_PATH_TOOLS = Object.values(hubspotTools)
+/** True when `target` actually lands in the path rather than the query string. */
+function reachesPath(tool: AnyTool, target: string): boolean {
+  try {
+    return buildUrl(tool, target, TARGET).pathname.includes(TARGET)
+  } catch {
+    return false
+  }
+}
+
+interface PathParamCase {
+  name: string
+  param: string
+  tool: AnyTool
+}
+
+const PATH_PARAM_CASES: PathParamCase[] = Object.values(hubspotTools)
   .filter(isHubSpotTool)
   .filter((tool) => typeof tool.request?.url === 'function')
-  .filter((tool) => {
-    try {
-      return buildUrl(tool, SAFE_ID).pathname.includes(SAFE_ID)
-    } catch {
-      return false
-    }
-  })
-  .map((tool) => ({ name: tool.id, tool }))
+  .flatMap((tool) =>
+    Object.keys(tool.params ?? {})
+      .filter((param) => !(param in FIXED_PARAMS))
+      .filter((param) => reachesPath(tool, param))
+      .map((param) => ({ name: `${tool.id} / ${param}`, param, tool }))
+  )
 
-describe('hubspot path-id traversal safety', () => {
-  it('covers every HubSpot tool that interpolates an id into its path', () => {
-    expect(DYNAMIC_PATH_TOOLS.length).toBeGreaterThanOrEqual(28)
+describe('hubspot path-parameter traversal safety', () => {
+  it('covers every (hubspot tool, path parameter) pair', () => {
+    expect(PATH_PARAM_CASES.length).toBeGreaterThanOrEqual(35)
   })
 
-  describe.each(DYNAMIC_PATH_TOOLS)('$name', ({ tool }) => {
-    const baseline = segmentsOf(buildUrl(tool, SAFE_ID).pathname)
+  describe.each(PATH_PARAM_CASES)('$name', ({ tool, param }) => {
+    const baseline = segmentsOf(buildUrl(tool, param, TARGET).pathname)
 
-    it.each(TRAVERSAL_IDS)('cannot reshape the path with %j', (value) => {
+    it.each(TRAVERSAL_VALUES)('cannot reshape the path with %j', (value) => {
       let url: URL
       try {
-        url = buildUrl(tool, value)
+        url = buildUrl(tool, param, value)
       } catch {
         return
       }
 
       expect(url.origin).toBe('https://api.hubapi.com')
+      expect(url.pathname.startsWith('/')).toBe(true)
 
       const actual = segmentsOf(url.pathname)
       expect(actual).toHaveLength(baseline.length)
       baseline.forEach((segment, index) => {
-        if (segment === SAFE_ID) return
+        if (segment === TARGET) return
         expect(actual[index]).toBe(segment)
       })
     })
 
+    /**
+     * The shape check above cannot see a bare dot in the *final* segment: `x/.`
+     * normalizes to `x/`, which keeps the segment count and leaves every other
+     * segment intact. Rejection is the only observable difference, so assert it
+     * directly for every parameter rather than only the hand-picked ones below.
+     */
+    it.each(['.', '..'])('rejects the bare %j segment', (value) => {
+      expect(() => buildUrl(tool, param, value)).toThrow(/path traversal/)
+    })
+
     it.each(LEGITIMATE_IDS)('passes %j through unchanged', (value) => {
-      const actual = segmentsOf(buildUrl(tool, value).pathname)
+      const actual = segmentsOf(buildUrl(tool, param, value).pathname)
 
       expect(actual).toHaveLength(baseline.length)
       baseline.forEach((segment, index) => {
-        expect(actual[index]).toBe(segment === SAFE_ID ? value : segment)
+        expect(actual[index]).toBe(segment === TARGET ? value : segment)
       })
     })
   })
 })
 
-const HIGH_RISK_TOOLS: ReadonlyArray<{ name: string; tool: AnyTool }> = [
-  { name: 'hubspot_delete_contact', tool: hubspotDeleteContactTool },
-  { name: 'hubspot_delete_company', tool: hubspotDeleteCompanyTool },
-  { name: 'hubspot_delete_deal', tool: hubspotDeleteDealTool },
-  { name: 'hubspot_delete_association', tool: hubspotDeleteAssociationTool },
+const HIGH_RISK_CASES: ReadonlyArray<{ name: string; tool: AnyTool; param: string }> = [
+  { name: 'hubspot_delete_contact', tool: hubspotDeleteContactTool, param: 'contactId' },
+  { name: 'hubspot_delete_company', tool: hubspotDeleteCompanyTool, param: 'companyId' },
+  { name: 'hubspot_delete_deal', tool: hubspotDeleteDealTool, param: 'dealId' },
+  { name: 'hubspot_delete_association', tool: hubspotDeleteAssociationTool, param: 'objectId' },
 ]
 
-describe.each(HIGH_RISK_TOOLS)('$name id path safety', ({ tool }) => {
+describe.each(HIGH_RISK_CASES)('$name path safety', ({ tool, param }) => {
   it('rejects a bare dot-dot segment instead of silently popping the resource', () => {
-    expect(() => buildUrl(tool, '..')).toThrow(/path traversal/)
+    expect(() => buildUrl(tool, param, '..')).toThrow(/path traversal/)
   })
 
   it('rejects a bare dot segment', () => {
-    expect(() => buildUrl(tool, '.')).toThrow(/path traversal/)
+    expect(() => buildUrl(tool, param, '.')).toThrow(/path traversal/)
   })
 
-  it('does not let the id inject query parameters', () => {
-    const url = buildUrl(tool, '12345?properties=email')
+  it('does not let the value inject query parameters', () => {
+    const url = buildUrl(tool, param, '12345?properties=email')
 
     expect(url.searchParams.get('properties')).toBeNull()
   })
 
-  it('preserves a legitimate numeric id verbatim after trimming', () => {
-    expect(buildUrl(tool, '  12345  ').pathname).toContain('/12345')
+  it('preserves a legitimate value verbatim after trimming', () => {
+    expect(buildUrl(tool, param, '  12345  ').pathname).toContain(`/${'12345'}`)
+  })
+})
+
+/**
+ * The regression this file exists to prevent. A fill-everything sweep reported
+ * these tools as covered while only the *first* of their four path parameters
+ * was ever reached, so reverting any of the other three produced no failure.
+ */
+describe('association tools expose every path parameter separately', () => {
+  const ASSOCIATION_TOOLS = [
+    {
+      name: 'hubspot_create_association',
+      tool: hubspotCreateAssociationTool,
+      params: ['objectType', 'objectId', 'toObjectType', 'toObjectId'],
+    },
+    {
+      name: 'hubspot_delete_association',
+      tool: hubspotDeleteAssociationTool,
+      params: ['objectType', 'objectId', 'toObjectType', 'toObjectId'],
+    },
+    {
+      name: 'hubspot_list_associations',
+      tool: hubspotListAssociationsTool,
+      params: ['objectType', 'objectId', 'toObjectType'],
+    },
+  ] as const
+
+  it.each(ASSOCIATION_TOOLS)(
+    '$name contributes every path parameter it interpolates',
+    ({ tool, params }) => {
+      const covered = PATH_PARAM_CASES.filter((entry) => entry.tool === tool).map(
+        (entry) => entry.param
+      )
+
+      expect([...covered].sort()).toEqual([...params].sort())
+    }
+  )
+
+  it.each(
+    ASSOCIATION_TOOLS.flatMap(({ name, tool, params }) =>
+      params.map((param) => ({ name, tool, param }))
+    )
+  )('$name rejects a dot-dot segment in $param', ({ tool, param }) => {
+    expect(() => buildUrl(tool, param, '..')).toThrow(/path traversal/)
   })
 })