fix: Cloudflare keyed query conditions and rateLimit schema gaps

src/lib/providers/cloudflare/__tests__/translator.test.ts

@@ -134,6 +134,20 @@ describe('cloudflare/translator', () => {
       expect(result.enabled).toBe(true)
     })
 
+    // Regression test: a keyed `query` condition (e.g. "match ?debug=1
+    // specifically") previously compiled to a bare `http.request.uri.query`
+    // comparison against the *entire* query string, silently ignoring `key`
+    // and matching far more broadly than intended — see ExpressionBuilder's
+    // fromUnifiedCondition/buildKeyedQueryExpression for the fix.
+    it('scopes a keyed query condition to that argument, not the entire query string', () => {
+      const rule = makeUnifiedRule({
+        conditions: [{ field: 'query', operator: 'eq', value: '1', key: 'debug' }],
+      })
+      const { result } = unifiedToCloudflare(rule)
+      expect(result.expression).toBe('any(http.request.uri.args["debug"][*] eq "1")')
+      expect(result.expression).not.toBe('http.request.uri.query eq "1"')
+    })
+
     it('maps unified actions to Cloudflare actions', () => {
       const actionMappings: Array<{ unified: string; cf: string }> = [
         { unified: 'log', cf: 'log' },

src/lib/providers/vercel/__tests__/VercelFirewallService.test.ts

@@ -1063,6 +1063,55 @@ describe('VercelFirewallService', () => {
       expect(changes.hasChanges).toBe(false)
     })
 
+    // Regression test: rateLimitSchema didn't validate mitigationTimeout/
+    // countingExpression, so Zod silently stripped them — getChanges diffs
+    // against `configValidation.data` (the *parsed* config), not the raw
+    // one, so a rule authored with either field lost it before ever
+    // reaching unifiedToVercel. Asserting on rulesToAdd (sourced straight
+    // from configValidation.data.rules for a pure addition) proves the
+    // fields now survive that parse.
+    it('preserves rateLimit.mitigationTimeout and countingExpression through getChanges (regression test)', async () => {
+      jest.spyOn(client, 'fetchFirewallConfig').mockResolvedValue({
+        ...mockVercelConfig,
+        rules: [],
+        ips: [],
+      })
+
+      const config: UnifiedConfig = {
+        version: '2.0',
+        provider: 'vercel',
+        rules: [
+          {
+            id: 'rule_rl',
+            name: 'Rate Limit API',
+            enabled: true,
+            conditionLogic: 'AND',
+            conditions: [{ field: 'path', operator: 'starts_with', value: '/api/', group: 0 }],
+            action: {
+              type: 'rate_limit',
+              rateLimit: {
+                requests: 100,
+                window: '1m',
+                mitigationTimeout: 7200,
+                countingExpression: 'http.request.method eq "POST"',
+              },
+            },
+          },
+        ],
+        ips: [],
+      }
+
+      const changes = await service.getChanges(config)
+
+      expect(changes.rulesToAdd).toHaveLength(1)
+      expect(changes.rulesToAdd[0]?.action.rateLimit).toEqual({
+        requests: 100,
+        window: '1m',
+        mitigationTimeout: 7200,
+        countingExpression: 'http.request.method eq "POST"',
+      })
+    })
+
     it('should detect no changes for a redirect rule with permanent unset (regression test for #203)', async () => {
       jest.spyOn(client, 'fetchFirewallConfig').mockResolvedValue({
         ...mockVercelConfig,

src/lib/schemas/__tests__/commonSchemas.test.ts

@@ -155,6 +155,28 @@ describe('commonSchemas', () => {
     it('rejects invalid window format', () => {
       expect(rateLimitSchema.safeParse({ requests: 100, window: 'invalid' }).success).toBe(false)
     })
+
+    // Regression test: mitigationTimeout/countingExpression are on
+    // UnifiedAction['rateLimit'] (types/unified.ts) and read by every
+    // translator, but were missing from this schema — Zod silently strips
+    // unrecognized keys, so `success: true` alone doesn't prove they
+    // survive; asserting on the parsed data does.
+    it('round-trips mitigationTimeout and countingExpression without stripping them', () => {
+      const result = rateLimitSchema.safeParse({
+        requests: 100,
+        window: '1m',
+        mitigationTimeout: 7200,
+        countingExpression: 'http.request.method eq "POST"',
+      })
+      expect(result.success).toBe(true)
+      expect(result.success && result.data.mitigationTimeout).toBe(7200)
+      expect(result.success && result.data.countingExpression).toBe('http.request.method eq "POST"')
+    })
+
+    it('rejects a non-positive mitigationTimeout', () => {
+      const result = rateLimitSchema.safeParse({ requests: 100, window: '1m', mitigationTimeout: 0 })
+      expect(result.success).toBe(false)
+    })
   })
 
   describe('redirectSchema', () => {

src/lib/schemas/__tests__/unifiedSchemas.test.ts

@@ -320,6 +320,44 @@ describe('unifiedSchemas', () => {
       expect(result.success).toBe(true)
       expect(result.success && result.data.managedRules).toEqual(managedRules)
     })
+
+    // Same class of bug as managedRules above, on action.rateLimit instead:
+    // mitigationTimeout/countingExpression are on UnifiedAction['rateLimit']
+    // (types/unified.ts) and read by unifiedToCloudflare/unifiedToVercel/
+    // buildFastlyRateLimit, but rateLimitSchema didn't validate them —
+    // silently stripped on every safeParse. VercelFirewallService.getChanges
+    // and FastlyFirewallService.getChanges both diff against the *parsed*
+    // config, so a rule authored with either field lost it before it ever
+    // reached those translators.
+    it('round-trips action.rateLimit.mitigationTimeout and countingExpression through safeParse without stripping them', () => {
+      const result = unifiedConfigSchema.safeParse({
+        ...validConfig,
+        rules: [
+          {
+            name: 'Rate limited rule',
+            enabled: true,
+            conditions: [{ field: 'path', operator: 'eq', value: '/api' }],
+            action: {
+              type: 'rate_limit',
+              rateLimit: {
+                requests: 100,
+                window: '1m',
+                mitigationTimeout: 7200,
+                countingExpression: 'http.request.method eq "POST"',
+              },
+            },
+          },
+        ],
+      })
+
+      expect(result.success).toBe(true)
+      expect(result.success && result.data.rules[0]?.action.rateLimit).toEqual({
+        requests: 100,
+        window: '1m',
+        mitigationTimeout: 7200,
+        countingExpression: 'http.request.method eq "POST"',
+      })
+    })
   })
 
   describe('validateUnifiedConfig', () => {

src/lib/schemas/commonSchemas.ts

@@ -74,6 +74,13 @@ export const rateLimitSchema = z.object({
   requests: z.number().positive().int(),
   window: z.string().regex(/^\d+[smhd]$/),
   characteristics: z.array(z.string()).optional(),
+  // Zod strips unrecognized keys by default — without these, a rule authored
+  // with either field silently lost it on the first safeParse (every
+  // provider's getChanges/syncRules diffs against the parsed config, not the
+  // raw one), even though both are on UnifiedAction['rateLimit'] and read by
+  // every translator (unifiedToCloudflare/unifiedToVercel/buildFastlyRateLimit).
+  mitigationTimeout: z.number().positive().optional(),
+  countingExpression: z.string().optional(),
 })
 
 // Redirect schema

src/lib/translators/ExpressionBuilder.ts

@@ -11,6 +11,14 @@ import { ipAddressSchema } from '../schemas/commonSchemas'
  */
 const UNQUOTED_IP_FIELDS = new Set(['ip.src'])
 
+/**
+ * Cloudflare's indexable query-args field. Distinct from
+ * `http.request.uri.query` (the whole query string, a scalar `String`) —
+ * this one is a `Map<Array<String>>` keyed by argument name, used only when
+ * a query condition carries a `key`. See `buildKeyedQueryExpression`.
+ */
+const QUERY_ARGS_FIELD = 'http.request.uri.args'
+
 /**
  * Builds Cloudflare wirefilter expressions from structured conditions
  */
@@ -103,6 +111,17 @@ export class ExpressionBuilder {
    * Build expression from a single unified condition
    */
   public static fromUnifiedCondition(condition: UnifiedCondition): string {
+    // A keyed query condition can't reuse the generic bracket-index path
+    // below the way header/cookie do: Cloudflare's indexable query-args
+    // field (`http.request.uri.args`, aliased as QUERY_ARGS_FIELD) types as
+    // `Map<Array<String>>`, so `args["key"] eq "value"` is an Array-vs-String
+    // type mismatch the Cloudflare API rejects — it needs `any(args["key"][*]
+    // eq "value")` (and `has_key(...)` for exists/not_exists) instead. See
+    // buildKeyedQueryExpression.
+    if (condition.key && condition.field === 'query') {
+      return this.buildKeyedQueryExpression(condition, condition.key)
+    }
+
     const baseField = this.mapUnifiedFieldToCloudflare(condition.field)
     // `key` only makes sense as a bracket index for header/cookie fields
     // (matching FieldMapper's Vercel-side behavior) — a header or cookie
@@ -122,6 +141,37 @@ export class ExpressionBuilder {
     return expression
   }
 
+  /**
+   * Build a keyed query-parameter expression against Cloudflare's
+   * `Map<Array<String>>`-typed `http.request.uri.args` field — see the
+   * comment in `fromUnifiedCondition` for why this can't share the generic
+   * field-string path the way header/cookie conditions do. Mirrors
+   * Cloudflare's own documented idioms: `any(args["key"][*] <op> value)` for
+   * value comparisons (a query param can repeat, so this matches if *any*
+   * occurrence satisfies the operator) and `has_key(args, "key")` for
+   * existence.
+   */
+  private static buildKeyedQueryExpression(condition: UnifiedCondition, key: string): string {
+    const escapedKey = escapeWirefilterString(key)
+    const keyedField = `${QUERY_ARGS_FIELD}["${escapedKey}"]`
+
+    let expression: string
+    if (condition.operator === 'exists') {
+      expression = `has_key(${QUERY_ARGS_FIELD}, "${escapedKey}")`
+    } else if (condition.operator === 'not_exists') {
+      expression = `not (has_key(${QUERY_ARGS_FIELD}, "${escapedKey}"))`
+    } else if (condition.operator === 'not_contains') {
+      expression = `not (any(${keyedField}[*] contains ${this.formatValue(QUERY_ARGS_FIELD, condition.value)}))`
+    } else if (condition.operator === 'not_in') {
+      expression = `not (any(${keyedField}[*] in ${this.formatValue(QUERY_ARGS_FIELD, condition.value)}))`
+    } else {
+      const operator = this.mapUnifiedOperator(condition.operator)
+      expression = `any(${keyedField}[*] ${operator} ${this.formatValue(QUERY_ARGS_FIELD, condition.value)})`
+    }
+
+    return condition.negated ? `not (${expression})` : expression
+  }
+
   /**
    * `exists`/`not_exists` (Vercel: `ex`/`nex`) conditions carry no value and
    * wirefilter has no `not exists` binary operator — negation must wrap the

src/lib/translators/__tests__/ExpressionBuilder.test.ts

@@ -353,15 +353,79 @@ describe('ExpressionBuilder', () => {
       expect(result).toBe('http.cookie["a\\" or true or http.cookie[\\"a"] eq "x"')
     })
 
-    it('ignores an unsupported key on a query condition instead of mislabeling it as a header', () => {
+    it('scopes a keyed query condition to that argument via http.request.uri.args, not the whole query string', () => {
       const result = ExpressionBuilder.fromUnifiedCondition({
         field: 'query',
         operator: 'eq',
         value: 'abc',
         key: 'utm_source',
       })
-      expect(result).toBe('http.request.uri.query eq "abc"')
+      // http.request.uri.query is a scalar String (the whole query string);
+      // http.request.uri.args is the Map<Array<String>> Cloudflare actually
+      // indexes by argument name, and `any(...[*] eq ...)` is the type-valid
+      // way to compare one of its (possibly-repeated) values — a bare
+      // `args["utm_source"] eq "abc"` is an Array-vs-String mismatch.
+      expect(result).toBe('any(http.request.uri.args["utm_source"][*] eq "abc")')
       expect(result).not.toContain('headers')
+      expect(result).not.toBe('http.request.uri.query eq "abc"')
+    })
+
+    it('leaves an unkeyed query condition matching the whole query string', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'contains',
+        value: 'debug=true',
+      })
+      expect(result).toBe('http.request.uri.query contains "debug=true"')
+    })
+
+    it('builds a valueless exists expression for a keyed query condition via has_key', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'exists',
+        key: 'utm_source',
+      } as UnifiedCondition)
+      expect(result).toBe('has_key(http.request.uri.args, "utm_source")')
+    })
+
+    it('builds a valueless not_exists expression for a keyed query condition wrapped in not(...)', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'not_exists',
+        key: 'utm_source',
+      } as UnifiedCondition)
+      expect(result).toBe('not (has_key(http.request.uri.args, "utm_source"))')
+    })
+
+    it('builds a not_contains expression for a keyed query condition as a positive any(...) wrapped in not(...)', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'not_contains',
+        value: 'admin',
+        key: 'redirect',
+      })
+      expect(result).toBe('not (any(http.request.uri.args["redirect"][*] contains "admin"))')
+    })
+
+    it('wraps a negated keyed query condition in an outer not(...) around the any(...) expression', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'eq',
+        value: '1',
+        key: 'debug',
+        negated: true,
+      })
+      expect(result).toBe('not (any(http.request.uri.args["debug"][*] eq "1"))')
+    })
+
+    it('escapes quotes in a keyed query key so it cannot break out of the field reference', () => {
+      const result = ExpressionBuilder.fromUnifiedCondition({
+        field: 'query',
+        operator: 'eq',
+        value: 'x',
+        key: 'a" or true or http.request.uri.args["a',
+      })
+      expect(result).toBe('any(http.request.uri.args["a\\" or true or http.request.uri.args[\\"a"][*] eq "x")')
     })
 
     it('builds a valueless exists expression (regression test for #85)', () => {