fix: emit valid wirefilter for keyed Cloudflare header/cookie conditions - #278
Merged
Merged
Conversation
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.
|
🎉 This PR is included in version 3.15.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #269.
http.request.headers,http.request.uri.args(query), andhttp.request.cookies(cookie) all type asMap<Array<String>>per Cloudflare's Ruleset Engine field/function references — indexing one yieldsArray<String>, sofield["key"] eq "value"for a keyed header/cookie condition was an Array-vs-String type mismatch, andfield["key"] existswas never valid syntax for a Map-typed field either.header/cookieconditions now compile toany(field["key"][*] <op> value)/has_key(field, "key")— the same idiom Cloudflare ignores query condition's key; rateLimit.mitigationTimeout/countingExpression unvalidated #263 already established forquery— via a new sharedExpressionBuilder.buildKeyedMapExpressionhelper.headernow always requireskeyand throws a clear error without one (no "all headers as one value" fallback exists); the key is lowercased before compiling, matching Cloudflare's documented lowercase-keyed header map.cookienow useshttp.request.cookies(Cloudflare's real per-cookie Map field) instead of bracket-indexinghttp.cookie(a scalar String) when keyed. This field requires Cloudflare Pro/Business/Enterprise — doorman emits it regardless of plan and lets the API reject it on an unsupported plan, same policy asmatches/regex.WirefilterParserhad no grammar forany(...)/has_key(...)at all, so it could never parse these back into structured conditions — this was already silently broken for Cloudflare ignores query condition's key; rateLimit.mitigationTimeout/countingExpression unvalidated #263's shipped query fix, confirmed and fixed retroactively as part of building the same grammar support for header/cookie.CloudflareRuleScenarios.test.ts's "header-based conditions" fixture, which folded the header name intovalueinstead of usingkey— never actually exercised a valid header condition, only exposed once the new throw-on-unkeyed-header check landed.cloudflare.md's field mapping table and operator-mapping section (which still referenced Cloudflare ignores query condition's key; rateLimit.mitigationTimeout/countingExpression unvalidated #263'skey-ignored bug as an open gap, despite Cloudflare ignores query condition's key; rateLimit.mitigationTimeout/countingExpression unvalidated #263 shipping as PR fix: Cloudflare keyed query conditions and rateLimit schema gaps #267 well before this session).Verification
pnpm compile && pnpm jest && pnpm eslint .all clean (1774 tests passing).ExpressionBuilderfix and theWirefilterParsergrammar dispatch independently, confirmed the new/updated regression tests fail with legible diffs in both cases, then restored.demos/cloudflare-mock-server.mjswith a real built CLI: synced a rule with keyed header (eq+exists), keyed cookie, and keyed query conditions, fetched the ruleset back from the mock server and confirmed the exact wirefilter sent over the wire:doorman diffagain and confirmed zero phantom changes — proving the parser fix round-trips correctly against real (mocked) Cloudflare API responses, not just in isolated unit tests.Test plan
pnpm jest— all 1774 tests passpnpm compile— cleanpnpm eslint .— 0 errorscloudflare-mock-server.mjs(sync + diff round-trip)