test(cloudflare,discord): fuzz path params one at a time, not as a whole object

The previous suites filled every string param with the same fuzz value and
swallowed the throw:

    try { path = buildPath(tool, value) } catch { return }

URL construction is eager, so the first guarded param to throw aborted the
whole vector and every sibling param went untested. That inverted the
property the suites were written to hold: once a tool had one guard, a
*newly unguarded* sibling could no longer fail CI. It is the worst possible
shape for these two services, where `channelId` + `messageId`,
`serverId` + `roleId`, and `zoneId` + `rulesetId` + `ruleId` share one path.

Both suites now enumerate (tool, param) pairs — discovered by probing one
param at a time, so a new tool or a new path param appears with no edit here
— and fuzz exactly one param while holding every sibling at a safe value.
133 pairs across 84 tools, up from 84 whole-tool cases.

The vectors are also split by the outcome they must produce, replacing the
tolerant try/catch: MUST_REJECT (dot segments and anything carrying a path
separator) asserts a throw naming the offending param, and MUST_NEUTRALIZE
(`?`/`#` inside a segment) asserts the segment shape is preserved. Nothing
is skipped silently any more.

Verified red-first against the tightened suite by reverting one guard on a
multi-param tool — `messageId` on discord_delete_message and `ruleId` on
cloudflare_delete_ruleset_rule — to an `encodeURIComponent`-only version,
leaving their siblings guarded. That is exactly the case the old shape could
not see: it produces 16 named failures now, while the old whole-object
assertion passed 3/3 green on the identical code.

No source change: the pair enumeration confirms every param that reaches a
path is already guarded.

Author: waleedlatif1

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

@@ -19,33 +19,49 @@
  * performs — rather than string-matching the template output, because string
  * matching is exactly what let this through.
  *
- * The tool list is enumerated from the barrel, so a newly added Cloudflare tool
- * that interpolates an unguarded ID fails here without anyone editing this file.
+ * The suite enumerates **(tool, param) pairs** and fuzzes one param at a time,
+ * holding every sibling at a safe value. Fuzzing all params at once cannot work
+ * here: the first guard to throw aborts URL construction, so a tool's remaining
+ * params stop being exercised the moment one of them is fixed. Pair enumeration
+ * is what makes "a newly unguarded param fails CI" actually true for a tool that
+ * already has a guarded param — the dominant shape in this service, where
+ * `accountId` + `appId` + `policyId` and `zoneId` + `rulesetId` + `ruleId`
+ * share one path.
  */
 import { describe, expect, it } from 'vitest'
 import * as cloudflareTools from '@/tools/cloudflare'
 import type { ToolConfig } from '@/tools/types'
 
 const API_ORIGIN = 'https://api.cloudflare.com'
 const API_PREFIX = '/client/v4/'
+const CREDENTIAL_PARAM = 'apiKey'
 
 /**
+ * Vectors the guard must **reject outright**. Each is either a bare dot segment
+ * or carries a path separator, so encoding it would leave a live traversal.
  * 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 MUST_REJECT = [
   '..',
   '.',
   '  ..  ',
   '../../accounts/victim-account',
   '..%2f..%2faccounts/victim-account',
   '023e105f4ecef8ad9ca31a8372d0c353/../../accounts/victim-account',
-  '023e105f4ecef8ad9ca31a8372d0c353?account_id=attacker',
-  '023e105f4ecef8ad9ca31a8372d0c353#fragment',
   '023e105f4ecef8ad9ca31a8372d0c353/dns_records/../../../zones',
   '\\..\\..',
 ] as const
 
+/**
+ * Vectors that are not traversals but must not be able to reshape the request:
+ * a `?` or `#` inside a segment has to stay inside that segment.
+ */
+const MUST_NEUTRALIZE = [
+  '023e105f4ecef8ad9ca31a8372d0c353?account_id=attacker',
+  '023e105f4ecef8ad9ca31a8372d0c353#frag',
+] as const
+
 /** Values a real user legitimately supplies; none may be rejected or altered. */
 const LEGITIMATE_IDS = [
   '023e105f4ecef8ad9ca31a8372d0c353',
@@ -61,6 +77,8 @@ const LEGITIMATE_IDS = [
 ] as const
 
 const SAFE_ID = 'SAFEID'
+const PROBE = 'PROBEVALUE'
+const TRIM_SAMPLE = '023e105f4ecef8ad9ca31a8372d0c353'
 
 type AnyTool = ToolConfig<any, any>
 
@@ -74,112 +92,118 @@ function isCloudflareTool(value: unknown): value is AnyTool {
 }
 
 /**
- * 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 every string param at a known-safe value, then
+ * applies one override so exactly one param carries the value under test.
  */
-function buildParams(tool: AnyTool, value: string): Record<string, unknown> {
-  const params: Record<string, unknown> = { apiKey: 'cf-token' }
+function buildParams(
+  tool: AnyTool,
+  overrides: Record<string, unknown> = {}
+): Record<string, unknown> {
+  const params: Record<string, unknown> = { [CREDENTIAL_PARAM]: 'cf-token' }
   for (const [name, def] of Object.entries(tool.params ?? {})) {
-    if (name === 'apiKey') continue
+    if (name === CREDENTIAL_PARAM) continue
     const type = (def as { type?: string }).type
-    if (type === 'json' || type === 'array') {
+    if (type === 'json' || type === 'array' || type === 'file[]') {
       params[name] = []
     } else if (type === 'number') {
       params[name] = 1
     } else if (type === 'boolean') {
       params[name] = false
     } else {
-      params[name] = value
+      params[name] = SAFE_ID
     }
   }
-  return params
+  return { ...params, ...overrides }
 }
 
-function buildUrl(tool: AnyTool, value: string): URL {
+function buildUrl(tool: AnyTool, overrides: Record<string, unknown> = {}): 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, overrides) as any))
 }
 
 function segmentsOf(pathname: string): string[] {
   return pathname.split('/')
 }
 
-const DYNAMIC_PATH_TOOLS = Object.values(cloudflareTools)
+const TOOLS = Object.values(cloudflareTools)
   .filter(isCloudflareTool)
   .filter((tool) => typeof tool.request?.url === 'function')
-  .filter((tool) => {
-    try {
-      return buildUrl(tool, SAFE_ID).pathname.includes(SAFE_ID)
-    } catch {
-      return false
-    }
+
+/**
+ * Every (tool, param) pair where that param alone reaches the request path,
+ * discovered by probing one param at a time. A newly added tool — or a newly
+ * added path param on an existing tool — appears here with no edit to this file.
+ */
+const PATH_PARAM_PAIRS = TOOLS.flatMap((tool) =>
+  Object.keys(tool.params ?? {})
+    .filter((param) => param !== CREDENTIAL_PARAM)
+    .filter((param) => {
+      try {
+        return buildUrl(tool, { [param]: PROBE }).pathname.includes(PROBE)
+      } catch {
+        return false
+      }
+    })
+    .map((param) => ({ name: `${tool.id} / ${param}`, tool, param }))
+)
+
+describe('cloudflare path-param traversal safety', () => {
+  it('finds every (tool, param) pair that reaches the request path', () => {
+    expect(PATH_PARAM_PAIRS.length).toBeGreaterThanOrEqual(65)
   })
-  .map((tool) => ({ name: tool.id, tool }))
 
-describe('cloudflare path-ID traversal safety', () => {
-  it('covers every Cloudflare tool that interpolates an ID into its path', () => {
-    expect(DYNAMIC_PATH_TOOLS.length).toBeGreaterThanOrEqual(40)
+  it('covers multi-param paths, where whole-object fuzzing goes blind', () => {
+    const counts = new Map<string, number>()
+    for (const { tool } of PATH_PARAM_PAIRS) {
+      counts.set(tool.id, (counts.get(tool.id) ?? 0) + 1)
+    }
+    const multiParamTools = [...counts.values()].filter((count) => count > 1)
+
+    expect(multiParamTools.length).toBeGreaterThanOrEqual(15)
   })
 
-  describe.each(DYNAMIC_PATH_TOOLS)('$name', ({ tool }) => {
-    const baseline = segmentsOf(buildUrl(tool, SAFE_ID).pathname)
+  describe.each(PATH_PARAM_PAIRS)('$name', ({ tool, param }) => {
+    const baseline = segmentsOf(buildUrl(tool, { [param]: PROBE }).pathname)
 
-    it.each(TRAVERSAL_IDS)('cannot reshape the path with %j', (value) => {
-      let url: URL
-      try {
-        url = buildUrl(tool, value)
-      } catch {
-        return
-      }
+    it.each(MUST_REJECT)('rejects %j outright', (value) => {
+      expect(() => buildUrl(tool, { [param]: value })).toThrow(
+        new RegExp(`${param}|path traversal|path separator`)
+      )
+    })
+
+    it.each(MUST_NEUTRALIZE)('confines %j to a single segment', (value) => {
+      const url = buildUrl(tool, { [param]: value })
 
       expect(url.origin).toBe(API_ORIGIN)
       expect(url.pathname.startsWith(API_PREFIX)).toBe(true)
+      expect(url.searchParams.get('account_id')).toBeNull()
+      expect(url.hash).toBe('')
 
       const actual = segmentsOf(url.pathname)
       expect(actual).toHaveLength(baseline.length)
       baseline.forEach((segment, index) => {
-        if (segment === SAFE_ID) return
+        if (segment.includes(PROBE)) return
         expect(actual[index]).toBe(segment)
       })
     })
 
-    it.each(TRAVERSAL_IDS)('never smuggles a query parameter via %j', (value) => {
-      let url: URL
-      try {
-        url = buildUrl(tool, value)
-      } catch {
-        return
-      }
-
-      expect(url.searchParams.get('account_id')).toBeNull()
-    })
-
-    it('rejects a bare dot-dot segment instead of silently popping the prefix', () => {
-      expect(() => buildUrl(tool, '..')).toThrow(/path traversal is not allowed/)
-    })
-
-    it('rejects a bare dot segment', () => {
-      expect(() => buildUrl(tool, '.')).toThrow(/path traversal is not allowed/)
-    })
-
     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.replaceAll(PROBE, value))
       })
     })
 
-    it('trims surrounding whitespace off a legitimate ID', () => {
-      const actual = segmentsOf(buildUrl(tool, '  023e105f4ecef8ad9ca31a8372d0c353  ').pathname)
+    it('trims surrounding whitespace off a legitimate value', () => {
+      const actual = segmentsOf(buildUrl(tool, { [param]: `  ${TRIM_SAMPLE}  ` }).pathname)
 
       baseline.forEach((segment, index) => {
-        if (segment !== SAFE_ID) return
-        expect(actual[index]).toBe('023e105f4ecef8ad9ca31a8372d0c353')
+        expect(actual[index]).toBe(segment.replaceAll(PROBE, TRIM_SAMPLE))
       })
     })
   })