Skip to content

fix: emit valid wirefilter for keyed Cloudflare header/cookie conditions #278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gfargo merged 1 commit into from
Aug 24, 2026
Merged

fix: emit valid wirefilter for keyed Cloudflare header/cookie conditions #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions skills/doorman/references/cloudflare.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,26 @@ Doorman translates its unified rule format (`conditions`/`enabled`/`action: {typ

Cloudflare supports all 16 unified condition fields:

| Doorman Field | Cloudflare Field | Notes |
| -------------- | ----------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- |
| `ip` | `ip.src` | |
| `country` | `ip.geoip.country` | |
| `region` | `ip.geoip.subdivision_1` | Client geo subdivision (e.g. `"CA"`) — see the legacy-field note below for how this differs from Vercel's own `region` |
| `city` | `ip.geoip.city` | |
| `asn` | `ip.geoip.asnum` | |
| `path` | `http.request.uri.path` | |
| `host` | `http.host` | |
| `method` | `http.request.method` | |
| `header` | `http.request.headers["key"]` | Requires `key` (the header name) |
| `query` | `http.request.uri.query` | ⚠️ `key` is currently ignored — matches the whole query string, not one parameter ([doorman#263](https://github.com/gfargo/doorman/issues/263)) |
| `cookie` | `http.cookie["key"]` | Requires `key` (the cookie name) |
| `user_agent` | `http.user_agent` | |
| `referer` | `http.referer` | |
| `scheme` | `ssl` (boolean) | |
| `port` | `cf.edge.server_port` | |
| `threat_score` | `cf.threat_score` | Cloudflare bot/attack threat score (0-100) |
| Doorman Field | Cloudflare Field | Notes |
| -------------- | --------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- |
| `ip` | `ip.src` | |
| `country` | `ip.geoip.country` | |
| `region` | `ip.geoip.subdivision_1` | Client geo subdivision (e.g. `"CA"`) — see the legacy-field note below for how this differs from Vercel's own `region` |
| `city` | `ip.geoip.city` | |
| `asn` | `ip.geoip.asnum` | |
| `path` | `http.request.uri.path` | |
| `host` | `http.host` | |
| `method` | `http.request.method` | |
| `header` | `any(http.request.headers["key"][*] op value)` / `has_key(...)` | Requires `key` (the header name, lowercased before compiling) — see the keyed-field note below |
| `query` | `http.request.uri.query`, or `any(http.request.uri.args["key"][*] op value)` when keyed | Unkeyed matches the whole query string; `key` scopes to one parameter — see the keyed-field note below |
| `cookie` | `http.cookie`, or `any(http.request.cookies["key"][*] op value)` when keyed | Unkeyed matches the whole `Cookie` header; keyed requires Cloudflare Pro/Business/Enterprise — see the keyed-field note below |
| `user_agent` | `http.user_agent` | |
| `referer` | `http.referer` | |
| `scheme` | `ssl` (boolean) | |
| `port` | `cf.edge.server_port` | |
| `threat_score` | `cf.threat_score` | Cloudflare bot/attack threat score (0-100) |

**Keyed `header`/`cookie`/`query` conditions** ([doorman#269](https://github.com/gfargo/doorman/issues/269)): `http.request.headers`, `http.request.cookies`, and (when keyed) `http.request.uri.args` all type as Cloudflare's `Map<Array<String>>`, not a scalar `String` — indexing one yields an `Array<String>`, so a bare `field["key"] eq "value"` is an Array-vs-String type mismatch the Cloudflare API rejects. Doorman compiles a keyed condition to the idiom Cloudflare's Ruleset Engine actually documents instead: `any(field["key"][*] <op> value)` for a value comparison, `has_key(field, "key")` for `exists`/`not_exists`. A `header` condition always requires `key` — there's no "all headers as one value" fallback the way `cookie`'s unkeyed `http.cookie` is. `http.request.cookies` requires Cloudflare Pro, Business, or 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 below.

### Operator Mapping

Expand Down Expand Up @@ -134,7 +136,7 @@ The `RuleTranslator` surfaces warnings when a translation is lossy:
- **Unmapped managed-ruleset override action** — an `action`/`overrides[].action` in `managedRules` with no Cloudflare equivalent (only `log`/`deny`/`challenge`/`allow` map cleanly) is dropped with a warning
- **Negation edge cases** — complex negated conditions may produce subtly different behavior in Wirefilter

Doorman does **not** currently warn on the `rate_limit`/`redirect` omitted-config-object cases noted in Action Mapping above, or the `query`-condition `key`-ignored case noted in Field Mapping above ([doorman#263](https://github.com/gfargo/doorman/issues/263)) — both are known gaps, not something `doorman validate` catches today. Double-check those specifically rather than relying on validation output.
Doorman does **not** currently warn on the `rate_limit`/`redirect` omitted-config-object cases noted in Action Mapping above — a known gap, not something `doorman validate` catches today. Double-check those specifically rather than relying on validation output.

Run `doorman validate` to surface the warnings that do exist before deploying — it auto-detects Cloudflare from the config's `provider` field.

Expand Down
10 changes: 8 additions & 2 deletions src/lib/providers/cloudflare/__tests__/CloudflareRuleScenarios.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,15 +440,21 @@ describe('Cloudflare Rule Scenarios', () => {
type: 'challenge',
},
conditions: [
// The header name belongs in `key`, not folded into `value` — a
// header condition with no `key` has no Cloudflare translation at
// all (there's no "match any header" concept) and now throws
// rather than silently producing a comparison against the literal
// string this fixture used to pass as `value` (#269).
{
field: 'header',
key: 'X-Forwarded-For',
operator: 'eq',
value: 'X-Forwarded-For: suspicious-proxy',
value: 'suspicious-proxy',
},
{
field: 'header',
key: 'X-Real-IP',
operator: 'not_exists',
value: 'X-Real-IP',
},
],
}
Expand Down
96 changes: 60 additions & 36 deletions src/lib/translators/ExpressionBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,28 @@ 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`.
* Cloudflare fields that type as `Map<Array<String>>` rather than a scalar
* `String` — indexing one yields an `Array<String>`, so a naive
* `field["key"] eq "value"` is an Array-vs-String type mismatch the
* Cloudflare API rejects (verified against Cloudflare's Ruleset Engine
* field + function references, #263/#269). The correct idiom is
* `any(field["key"][*] <op> value)` for comparisons, `has_key(field, "key")`
* for existence — see `buildKeyedMapExpression`.
*
* `QUERY_ARGS_FIELD` is distinct from `http.request.uri.query` (the whole
* query string, a scalar `String`); `HEADERS_FIELD` is Cloudflare's real
* `header` field regardless of keying (there's no separate "all headers as
* one value" concept, unlike cookie); `COOKIES_MAP_FIELD` is distinct from
* `http.cookie` (the whole Cookie header as one scalar `String`, still used
* for a non-keyed cookie condition) — `http.request.cookies` requires
* Cloudflare Pro/Business/Enterprise (Ruleset Engine field reference), so
* doorman emits it regardless and lets Cloudflare's API reject it on an
* unsupported plan, the same policy already applied to `matches`/regex
* (see cloudflare.md).
*/
const QUERY_ARGS_FIELD = 'http.request.uri.args'
const HEADERS_FIELD = 'http.request.headers'
const COOKIES_MAP_FIELD = 'http.request.cookies'

/**
* Builds Cloudflare wirefilter expressions from structured conditions
Expand Down Expand Up @@ -111,15 +127,32 @@ 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.
// Keyed query/cookie conditions, and header conditions (always keyed —
// see the throw below), can't reuse the generic bracket-index path
// further down: their Cloudflare fields are `Map<Array<String>>` (see
// the field-constants comment above `QUERY_ARGS_FIELD`), so they need
// `buildKeyedMapExpression`'s `any(...)`/`has_key(...)` construct
// instead of a bare bracket comparison.
if (condition.key && condition.field === 'query') {
return this.buildKeyedQueryExpression(condition, condition.key)
return this.buildKeyedMapExpression(QUERY_ARGS_FIELD, condition.key, condition)
}
if (condition.field === 'header') {
if (!condition.key) {
// Unlike `cookie` below, `header` has no scalar "all headers as one
// string" field to fall back to — Cloudflare's header field is a
// Map, full stop — so a header condition genuinely needs to name
// which header it means. Fail loudly rather than silently emit
// `http.request.headers eq "..."`, comparing a Map against a String.
throw new Error('A "header" condition requires a key naming the header')
}
// Cloudflare's header map is keyed by lowercased header name (Ruleset
// Engine field reference, verified #269) — a mixed-case key would
// silently never match otherwise, since `any()` over a missing map
// entry is simply false, not an error.
return this.buildKeyedMapExpression(HEADERS_FIELD, condition.key.toLowerCase(), condition)
}
if (condition.key && condition.field === 'cookie') {
return this.buildKeyedMapExpression(COOKIES_MAP_FIELD, condition.key, condition)
}

const baseField = this.mapUnifiedFieldToCloudflare(condition.field)
Expand All @@ -132,16 +165,8 @@ export class ExpressionBuilder {
`Unsupported condition field '${condition.field}' for Cloudflare — filter it out with a warning before calling fromUnifiedCondition (see unifiedToCloudflare).`,
)
}
// `key` only makes sense as a bracket index for header/cookie fields
// (matching FieldMapper's Vercel-side behavior) — a header or cookie
// condition's key must not fall through to the headers field regardless
// of which of the two it actually is.
const field =
condition.key && (condition.field === 'header' || condition.field === 'cookie')
? `${baseField}["${escapeWirefilterString(condition.key)}"]`
: baseField

let expression = this.buildUnifiedExpression(field, condition.operator, condition.value)
let expression = this.buildUnifiedExpression(baseField, condition.operator, condition.value)

if (condition.negated) {
expression = `not (${expression})`
Expand All @@ -151,31 +176,30 @@ export class ExpressionBuilder {
}

/**
* 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.
* Builds a keyed comparison/exists expression against a Cloudflare
* `Map<Array<String>>`-typed field's keyed entry — query args, headers,
* or (Pro+) per-cookie values. See the field-constants comment above
* `QUERY_ARGS_FIELD`. Mirrors Cloudflare's own documented idioms:
* `any(field["key"][*] <op> value)` for value comparisons (an entry can
* repeat, so this matches if *any* occurrence satisfies the operator) and
* `has_key(field, "key")` for existence.
*/
private static buildKeyedQueryExpression(condition: UnifiedCondition, key: string): string {
private static buildKeyedMapExpression(mapField: string, key: string, condition: UnifiedCondition): string {
const escapedKey = escapeWirefilterString(key)
const keyedField = `${QUERY_ARGS_FIELD}["${escapedKey}"]`
const keyedField = `${mapField}["${escapedKey}"]`

let expression: string
if (condition.operator === 'exists') {
expression = `has_key(${QUERY_ARGS_FIELD}, "${escapedKey}")`
expression = `has_key(${mapField}, "${escapedKey}")`
} else if (condition.operator === 'not_exists') {
expression = `not (has_key(${QUERY_ARGS_FIELD}, "${escapedKey}"))`
expression = `not (has_key(${mapField}, "${escapedKey}"))`
} else if (condition.operator === 'not_contains') {
expression = `not (any(${keyedField}[*] contains ${this.formatValue(QUERY_ARGS_FIELD, condition.value)}))`
expression = `not (any(${keyedField}[*] contains ${this.formatValue(mapField, condition.value)}))`
} else if (condition.operator === 'not_in') {
expression = `not (any(${keyedField}[*] in ${this.formatValue(QUERY_ARGS_FIELD, condition.value)}))`
expression = `not (any(${keyedField}[*] in ${this.formatValue(mapField, condition.value)}))`
} else {
const operator = this.mapUnifiedOperator(condition.operator)
expression = `any(${keyedField}[*] ${operator} ${this.formatValue(QUERY_ARGS_FIELD, condition.value)})`
expression = `any(${keyedField}[*] ${operator} ${this.formatValue(mapField, condition.value)})`
}

return condition.negated ? `not (${expression})` : expression
Expand Down
Loading
Loading