fix(cbinsights): stop paging and blank input bypassing the search guards

- Measure the firmographics empty-search guard against the filters alone.
  limit, nextPageToken, and sort were in the same object, so a request
  carrying only paging slipped past it and issued an unfiltered search over
  the whole database — which still spends credits.
- Reject a mistyped numeric bound instead of dropping it. A bad headcount,
  funding, or valuation filter silently widened the search, the same failure
  mode already fixed for ID lists.
- Treat an empty comma segment identically on the required and optional
  paths. A trailing or doubled comma is a separator artifact that cannot
  change which records are requested, so both paths now discard it; every
  other malformed entry is still rejected.

Author: waleedlatif1

apps/sim/tools/cbinsights/cbinsights.test.ts

@@ -246,6 +246,73 @@ describe('cbinsights request building', () => {
     expect(JSON.parse(String(calls[1].init.body))).toEqual({ keyword: 'fintech' })
   })
 
+  /*
+   * An empty segment is a separator artifact, not a value — dropping it cannot
+   * change which organizations are requested. The required and optional paths
+   * must agree, or the same typing succeeds on one operation and fails on
+   * another.
+   */
+  it('tolerates a trailing or doubled comma identically on both paths', async () => {
+    mockFetch([AUTH_OK, { body: { orgs: [] } }, { body: { orgs: [] } }])
+
+    await cbinsightsListFundingsTool.directExecution!({
+      ...CREDS,
+      orgIds: '129410, 1034157,',
+    } as never)
+    expect(JSON.parse(String(calls[1].init.body)).orgIds).toEqual([129410, 1034157])
+
+    await cbinsightsSearchFirmographicsTool.directExecution!({
+      ...CREDS,
+      sectorIds: '1,,2',
+    } as never)
+    expect(JSON.parse(String(calls[2].init.body)).sectorIds).toEqual([1, 2])
+  })
+
+  it('rejects a mistyped numeric bound rather than dropping it', async () => {
+    mockFetch([AUTH_OK])
+    await expect(
+      cbinsightsSearchFirmographicsTool.directExecution!({
+        ...CREDS,
+        keyword: 'fintech',
+        minCurrentHeadcount: 'fifty',
+      } as never)
+    ).rejects.toThrow(/"minCurrentHeadcount" must be a number/)
+  })
+
+  /*
+   * The guard measures the filters alone. Paging or sort slipping past it would
+   * issue an unfiltered search over the whole database — and still bill for it.
+   */
+  it('refuses a firmographics search carrying only paging or sort', async () => {
+    mockFetch([AUTH_OK])
+    await expect(
+      cbinsightsSearchFirmographicsTool.directExecution!({
+        ...CREDS,
+        limit: 100,
+        nextPageToken: 'tok',
+        sortField: 'mosaicOverall',
+      } as never)
+    ).rejects.toThrow(/at least one search parameter/)
+  })
+
+  it('still sends paging and sort alongside a real filter', async () => {
+    mockFetch([AUTH_OK, { body: { orgs: [] } }])
+    await cbinsightsSearchFirmographicsTool.directExecution!({
+      ...CREDS,
+      keyword: 'fintech',
+      limit: 25,
+      nextPageToken: 'tok',
+      sortField: 'mosaicOverall',
+    } as never)
+
+    expect(JSON.parse(String(calls[1].init.body))).toEqual({
+      keyword: 'fintech',
+      limit: 25,
+      nextPageToken: 'tok',
+      sort: { field: 'mosaicOverall', direction: 'desc' },
+    })
+  })
+
   it('rejects a non-integer organization ID rather than interpolating it into the path', async () => {
     mockFetch([AUTH_OK])
     await expect(

apps/sim/tools/cbinsights/search_firmographics.ts

@@ -307,7 +307,7 @@ export const cbinsightsSearchFirmographicsTool: ToolConfig<
   request: { url: () => '', method: 'POST', headers: () => ({}) },
 
   directExecution: async (params, signal) => {
-    const body = compactBody({
+    const filters = compactBody({
       keyword: params.keyword?.trim(),
       orgIds: parseIdListParam(params.orgIds, 'orgIds'),
       orgNames: parseStringListParam(params.orgNames, 'orgNames'),
@@ -338,19 +338,47 @@ export const cbinsightsSearchFirmographicsTool: ToolConfig<
         params.lastFundingRoundCategoryIds,
         'lastFundingRoundCategoryIds'
       ),
-      minCurrentHeadcount: parseIntegerParam(params.minCurrentHeadcount),
-      maxCurrentHeadcount: parseIntegerParam(params.maxCurrentHeadcount),
-      minTotalFundingInMillions: parseNumberParam(params.minTotalFundingInMillions),
-      maxTotalFundingInMillions: parseNumberParam(params.maxTotalFundingInMillions),
-      minValuationInMillions: parseNumberParam(params.minValuationInMillions),
-      maxValuationInMillions: parseNumberParam(params.maxValuationInMillions),
+      minCurrentHeadcount: parseIntegerParam(params.minCurrentHeadcount, 'minCurrentHeadcount'),
+      maxCurrentHeadcount: parseIntegerParam(params.maxCurrentHeadcount, 'maxCurrentHeadcount'),
+      minTotalFundingInMillions: parseNumberParam(
+        params.minTotalFundingInMillions,
+        'minTotalFundingInMillions'
+      ),
+      maxTotalFundingInMillions: parseNumberParam(
+        params.maxTotalFundingInMillions,
+        'maxTotalFundingInMillions'
+      ),
+      minValuationInMillions: parseNumberParam(
+        params.minValuationInMillions,
+        'minValuationInMillions'
+      ),
+      maxValuationInMillions: parseNumberParam(
+        params.maxValuationInMillions,
+        'maxValuationInMillions'
+      ),
       minLastFundingDate: params.minLastFundingDate?.trim(),
       maxLastFundingDate: params.maxLastFundingDate?.trim(),
       vcBacked: parseBooleanParam(params.vcBacked),
-      limit: clampLimit(params.limit),
-      nextPageToken: params.nextPageToken,
     })
 
+    /*
+     * The guard has to measure the *filters* alone. Folding limit, the page
+     * token, or the sort into the same object would let a request carrying only
+     * paging past it — which is an unfiltered search over the whole database,
+     * and it still spends credits.
+     */
+    if (Object.keys(filters).length === 0) {
+      throw new Error('CB Insights firmographics search requires at least one search parameter')
+    }
+
+    const body: Record<string, unknown> = {
+      ...filters,
+      ...compactBody({
+        limit: clampLimit(params.limit),
+        nextPageToken: params.nextPageToken,
+      }),
+    }
+
     /* The API takes one sort object; the block exposes it as two plain fields
        so neither has to be typed as JSON. */
     const sortField = params.sortField?.trim()
@@ -361,10 +389,6 @@ export const cbinsightsSearchFirmographicsTool: ToolConfig<
       }
     }
 
-    if (Object.keys(body).length === 0) {
-      throw new Error('CB Insights firmographics search requires at least one search parameter')
-    }
-
     return cbInsightsRequest<{
       orgs?: unknown
       nextPageToken?: unknown

apps/sim/tools/cbinsights/utils.ts

@@ -279,7 +279,7 @@ export function requireOrgIds(value: unknown): number[] {
       }
       raw = parsed
     } else {
-      raw = trimmed.split(',')
+      raw = splitCommaList(trimmed)
     }
   } else if (value !== undefined && value !== null) {
     raw = [value]
@@ -334,6 +334,23 @@ export function clampLimit(value: unknown): number | undefined {
   return Math.min(Math.max(Math.trunc(parsed), LIMIT_MIN), LIMIT_MAX)
 }
 
+/**
+ * Splits a hand-typed comma list, discarding empty segments.
+ *
+ * An empty segment is a separator artifact — a trailing comma, or a double one —
+ * and carries no value, so dropping it cannot change *which* records are
+ * requested. That is the opposite of dropping a mistyped entry like `notanid`,
+ * which silently discards an ID the caller meant to include. Both the required
+ * and the optional paths route through here so the same typing produces the same
+ * result on every operation.
+ */
+function splitCommaList(value: string): string[] {
+  return value
+    .split(',')
+    .map((entry) => entry.trim())
+    .filter((entry) => entry !== '')
+}
+
 /**
  * Parses a JSON-array param that may arrive already parsed, tolerating a bare
  * comma-separated list for the flat ID filters.
@@ -346,10 +363,7 @@ export function parseListParam(value: unknown, paramName: string): unknown[] | u
     const trimmed = value.trim()
     if (trimmed === '') return undefined
     if (!trimmed.startsWith('[')) {
-      const entries = trimmed
-        .split(',')
-        .map((entry) => entry.trim())
-        .filter(Boolean)
+      const entries = splitCommaList(trimmed)
       return entries.length > 0 ? entries : undefined
     }
     let parsed: unknown
@@ -389,16 +403,25 @@ export function parseStringListParam(value: unknown, paramName: string): string[
   return values.length > 0 ? values : undefined
 }
 
-/** Coerces an optional numeric filter, dropping a value that is not a number. */
-export function parseNumberParam(value: unknown): number | undefined {
+/**
+ * Coerces an optional numeric filter, rejecting a value that is not a number.
+ *
+ * Dropping it would widen the search rather than narrow it — a mistyped headcount
+ * or valuation bound would silently disappear and the broader query would still
+ * spend credits. Same reasoning as the ID lists.
+ */
+export function parseNumberParam(value: unknown, paramName: string): number | undefined {
   if (value === undefined || value === null || value === '') return undefined
-  const parsed = typeof value === 'number' ? value : Number(value)
-  return Number.isFinite(parsed) ? parsed : undefined
+  const parsed = typeof value === 'number' ? value : Number(String(value).trim())
+  if (!Number.isFinite(parsed)) {
+    throw new Error(`CB Insights "${paramName}" must be a number (received "${String(value)}")`)
+  }
+  return parsed
 }
 
-/** Coerces an optional integer filter. */
-export function parseIntegerParam(value: unknown): number | undefined {
-  const parsed = parseNumberParam(value)
+/** Coerces an optional integer filter, rejecting a value that is not a number. */
+export function parseIntegerParam(value: unknown, paramName: string): number | undefined {
+  const parsed = parseNumberParam(value, paramName)
   return parsed === undefined ? undefined : Math.trunc(parsed)
 }