Skip to content

fix: Cloudflare keyed query conditions and rateLimit schema gaps - #267

Merged
gfargo merged 2 commits into
mainfrom
fix/query-key-scope-ratelimit-schema
Aug 24, 2026
Merged

fix: Cloudflare keyed query conditions and rateLimit schema gaps#267
gfargo merged 2 commits into
mainfrom
fix/query-key-scope-ratelimit-schema

Conversation

@gfargo

@gfargo gfargo commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Fixes #263

Summary

Two related, independently-verified translator/schema correctness bugs found during a wiki-documentation audit — lower severity than #261/#262 (filed the same day for more severe Vercel translator gaps), but real silent-failure modes on Cloudflare and across all three providers.

  • Cloudflare ignores key on query conditions. ExpressionBuilder.fromUnifiedCondition only bracket-scoped header/cookie conditions by key; a keyed query condition (e.g. "match ?debug=1 specifically") silently compiled to a bare http.request.uri.query comparison against the entire query string, matching far more broadly than intended.

    The fix isn't just adding query to that check — http.request.uri.query is a scalar String (the whole query string), while Cloudflare's actual indexable field, http.request.uri.args, types as Map<Array<String>>. A naive args["key"] eq "value" is an Array-vs-String mismatch the Cloudflare API rejects. Verified against Cloudflare's Ruleset Engine docs and the "Require specific headers" WAF guide for the real idioms: any(args["key"][*] eq "value") for value comparisons (a param can repeat) and has_key(args, "key") for exists/not_exists.

  • rateLimit.mitigationTimeout/countingExpression aren't in the Zod schema. Both fields are on UnifiedAction['rateLimit'] and read by every translator (unifiedToCloudflare, unifiedToVercel, buildFastlyRateLimit), but rateLimitSchema only validated requests/window/characteristics. Zod strips unrecognized keys by default, and VercelFirewallService/FastlyFirewallService's getChanges both diff against the parsed config — so a rule authored with either field silently lost it before reaching those translators, and each translator's own fallback fired instead of the value the user actually set. Cloudflare's service diffs against the raw config directly, so it didn't hit this specific path, but the schema gap itself was provider-independent.

Test plan

  • Regression tests for both fixes, each mutation-verified (temporarily reverted, confirmed the new tests fail legibly, restored, confirmed green)
  • pnpm compile && pnpm test && pnpm lint all pass (1740 tests, 0 lint errors)
  • End-to-end test confirming VercelFirewallService.getChanges no longer drops mitigationTimeout/countingExpression before reaching the translator layer

gfargo added 2 commits August 24, 2026 11:05
A query condition with a key (e.g. field: 'query', key: 'debug', matching
?debug=1 specifically) compiled to a bare http.request.uri.query comparison
against the entire query string — the key-scoping check only covered
header/cookie, so it was silently ignored. Wider matching than intended,
with no warning.

Fixing it isn't just adding query to that check: http.request.uri.query is
a scalar String (the whole query string), and Cloudflare's actual
indexable field, http.request.uri.args, types as Map<Array<String>> — a
bare args["key"] eq "value" is an Array-vs-String mismatch the API
rejects. Verified against Cloudflare's Ruleset Engine docs and the
"Require specific headers" WAF guide for the correct idioms: any(args["key"][*]
eq "value") for value comparisons (a param can repeat) and
has_key(args, "key") for exists/not_exists.
…od schema

Both fields are on UnifiedAction['rateLimit'] and read by every translator
(unifiedToCloudflare, unifiedToVercel, buildFastlyRateLimit), but
rateLimitSchema only validated requests/window/characteristics. Zod strips
unrecognized keys by default, and VercelFirewallService.getChanges/
syncRules and FastlyFirewallService.getChanges both diff against the
*parsed* config rather than the raw one — so a rule authored with either
field silently lost it before ever reaching those translators, and each
translator's own fallback (?? interval, etc.) fired instead of the value
the user actually set. Cloudflare's service diffs against the raw config
directly, so it didn't hit this specific path, but the schema gap itself
was provider-independent.
@gfargo
gfargo merged commit ea94767 into main Aug 24, 2026
2 checks passed
@gfargo
gfargo deleted the fix/query-key-scope-ratelimit-schema branch August 24, 2026 15:35
github-actions Bot pushed a commit that referenced this pull request Aug 24, 2026
## [3.14.2](v3.14.1...v3.14.2) (2026-08-24)

### Bug Fixes

* Cloudflare keyed query conditions and rateLimit schema gaps ([#267](#267)) ([ea94767](ea94767))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 3.14.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

gfargo added a commit that referenced this pull request Aug 24, 2026
…ons (#278)

Closes #269. Verified against Cloudflare's Ruleset Engine field and
function references: http.request.headers, http.request.uri.args
(query), and http.request.cookies (cookie) all type as
Map<Array<String>>. Indexing one yields Array<String>, so the previous
field["key"] eq "value" construct — used for a keyed header or cookie
condition — was an Array-vs-String type mismatch Cloudflare's API
rejects, and field["key"] exists was never valid syntax for a
Map-typed field either.

Fixed by compiling a keyed header/cookie condition to the same
any(field["key"][*] <op> value) / has_key(field, "key") idiom #263
already established for query, via a new shared
ExpressionBuilder.buildKeyedMapExpression helper. Two related fixes
fell out of the same research:

- header now always requires a key and throws a clear error without
  one, since there's no "all headers as one value" fallback the way
  cookie's unkeyed http.cookie is. The key is also lowercased before
  compiling, matching Cloudflare's documented lowercase-keyed header
  map (a mixed-case key would otherwise silently never match).
- cookie now uses http.request.cookies (Cloudflare's actual per-cookie
  Map field) instead of bracket-indexing http.cookie (a scalar String
  with no Map to index at all) when keyed. This field requires
  Cloudflare Pro/Business/Enterprise; doorman emits it regardless of
  plan and lets Cloudflare's API reject it on an unsupported plan, the
  same policy already applied to matches/regex.

WirefilterParser had no grammar for any(...)/has_key(...) function-call
syntax at all, so it could never parse these expressions back into
structured conditions — confirmed this was already silently broken for
the query fix #263 shipped. Verified end-to-end against the Cloudflare
mock server: doorman sync followed by doorman diff now reports zero
phantom changes for a rule with keyed header/cookie/query conditions;
before this fix diff would have reported the rule as unparseable and
re-added it as a "change" every time. Added any(...)/has_key(...)
parsing that reuses the parser's existing comparison/exists AST node
shapes, so leafToCondition/isLeaf/orGroupsToConditions needed no
changes — this closes the round-trip gap for header, cookie, and
(retroactively) the pre-existing query case alike.

Also fixes CloudflareRuleScenarios.test.ts's "header-based conditions"
fixture, which never actually exercised a valid header condition — it
folded the header name into value instead of using key, something only
exposed once fromUnifiedCondition started throwing on a keyed field it
can't build. Updated cloudflare.md's field mapping table and the
operator-mapping section, which still referenced #263's key-ignored bug
as an open gap even though #263 shipped as PR #267 well before this
session started.
github-actions Bot pushed a commit that referenced this pull request Aug 24, 2026
## [3.15.2](v3.15.1...v3.15.2) (2026-08-24)

### Bug Fixes

* emit valid wirefilter for keyed Cloudflare header/cookie conditions ([#278](#278)) ([c529e57](c529e57)), closes [#269](#269) [#263](#263) [#263](#263) [#263](#263) [#263](#263) [#267](#267)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cloudflare ignores query condition's key; rateLimit.mitigationTimeout/countingExpression unvalidated

1 participant