fix: Vercel region collides with unified region; 8 legacy fields leak invalid Cloudflare wirefilter - #275
Merged
Conversation
… invalid cloudflare wirefilter
Vercel's native `region` (edge/deployment location, e.g. "sfo1") passed
through mapVercelTypeToUnified unchanged, silently colliding with the
unified schema's own `region` field (client geo subdivision, mapped to
Cloudflare's ip.geoip.subdivision_1). A Vercel region condition became a
syntactically valid but semantically inverted Cloudflare rule with zero
warnings — a deny/challenge rule that could never fire, or a bypass/allow
rule that silently blocked everyone it was meant to exempt. Fixed by
namespacing Vercel's region to `vercel_region` so it can't collide; the
reverse mapUnifiedTypeToVercel keeps the round trip lossless.
Separately, mapUnifiedFieldToCloudflare's `mapping[field] || field`
fallback leaked any unmapped field name as a bare wirefilter identifier
instead of dropping it — geo_continent, protocol, target_path,
environment, ja3_digest, ja4_digest, and rate_limit_api_id all produced
invalid expressions with zero warnings. Now returns `string | null`
(matching mapUnifiedTypeToVercel/mapUnifiedFieldToFastly's existing
pattern), and unifiedToCloudflare drops unsupported conditions with a
TranslationWarning before building the expression, throwing if that
leaves a rule with none (mirroring unifiedToVercel's identical guard —
never sync a conditionless rule that would match all traffic).
geo_country_region turned out not to need the same "unmapped, drop with
warning" treatment: rules.md already documents it as Vercel's real
client-subdivision field ("Region/state code", e.g. "CA") — the same
concept as unified region, not a separate one — so it now maps there
directly instead of passing through as its own name.
Also adds the threat_score field (Cloudflare's cf.threat_score) that a
pre-existing test relied on this same silent-fallback bug for, and updates
cloudflare.md's field table and its now-inaccurate claim that legacy-only
fields "aren't reachable through a provider-tagged config" (they are —
UnifiedCondition.field accepts arbitrary strings).
Closes #273
…-collision-invalid-fields # Conflicts: # skills/doorman/references/cloudflare.md
|
🎉 This PR is included in version 3.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
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.
Fixes #270
Fixes #271
(Also closes #273, a duplicate of #270/#271 filed independently in this same session before the duplication was noticed — see the comment on #273 for details.)
Summary
Two related, independently-verified translator correctness bugs, found and empirically confirmed (running the real
vercelToUnified/unifiedToCloudflarefunctions against test fixtures, not just reading the code) during arules.mddoc-accuracy audit.Vercel region condition silently collides with unified schema's differently-meaning region field on Cloudflare sync #270 — Vercel's
regionsilently collides with the unifiedregionfield. Vercel's nativeregionmeans the edge/deployment region a request is served from (e.g."sfo1") —mapVercelTypeToUnifiedhad no entry for it, so it passed through unchanged into a unified condition withfield: "region". But the unified schema already has aregionFieldType meaning something else entirely — the client's geographic subdivision, which Cloudflare's translator maps toip.geoip.subdivision_1. The two silently shared one name: a Vercel region condition became a syntactically valid Cloudflare rule (zero warnings) whose condition could never actually match anything — a deny/challenge rule that never fires, or a bypass/allow rule that silently blocks everyone it was meant to exempt. Fixed by namespacing Vercel'sregiontovercel_region, which no other provider's field vocabulary uses;mapUnifiedTypeToVercelkeeps the reverse direction in sync so the round trip stays lossless.8 legacy-only Vercel condition types produce invalid Cloudflare filter syntax with no warning #271 — 8 other legacy-only Vercel fields leaked invalid wirefilter syntax.
geo_continent,protocol,target_path,environment,ja3_digest,ja4_digest, andrate_limit_api_idall pass through the same way, andmapUnifiedFieldToCloudflare'smapping[field] || fieldfallback then leaked each one as a bare, non-existent wirefilter identifier (e.g.geo_continent eq "NA") — again with zero warnings. Every other unsupported-field case in this codebase (mapUnifiedTypeToVercel,mapUnifiedFieldToFastly) already returnsnulland lets the caller drop the condition with aTranslationWarning;mapUnifiedFieldToCloudflarewas the only one that fell through instead. Now it returnsstring | nullto match, andunifiedToCloudflarefilters unsupported conditions with a warning before building the expression — throwing if that leaves zero conditions, mirroringunifiedToVercel's identical existing guard (never silently sync a conditionless rule that would match all traffic).geo_country_region(one of the 8 in #271) turned out not to need the drop-with-warning treatment:rules.mdalready documents it as Vercel's real client-subdivision field ("Region/state code", e.g."CA") — the same concept as unifiedregion, not a separate one — so it now maps there directly and translates normally.Also fixes a
threat_score(cf.threat_score) gap a pre-existing test relied on this exact same silent-fallback bug for, and correctscloudflare.md's field table plus a claim it made that turned out to be inaccurate against actual runtime behavior (legacy-only fields "aren't reachable through a provider-tagged config" — they are;UnifiedCondition.fieldaccepts arbitrary strings).Test plan
ja4_digest eq "test"string that used to leak into the expression — then restored and confirmed green)pnpm tsc --noEmit && pnpm test && pnpm lintall pass (1756 tests, 0 lint errors)region(client geo) path — still producesip.geoip.subdivision_1with zero warningsregion→ unifiedvercel_region→ Vercelregionround trip stays lossless