[wrangler] Warn when compliance region environment variable overrides config - #15058
[wrangler] Warn when compliance region environment variable overrides config#15058edmundhung wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 2b629b5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Summary: replaces the hard error on a CLOUDFLARE_COMPLIANCE_REGION vs compliance_region conflict with a warning, letting the env var take precedence.
One thing worth calling out (design/safety), plus a minor note. The code itself is functionally correct.
| const resolvedRegion = getCloudflareComplianceRegion(config); | ||
| if (resolvedRegion === configuredRegion) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Minor: readConfig is invoked multiple times within some commands (e.g. ConfigController re-reads on file changes during wrangler dev), so this warning may be emitted repeatedly in a single session. This matches the existing behaviour of logWarningsWithUpgradeHint, so it may be acceptable, but if you want it shown once, consider de-duplicating.
|
Review posted successfully to PR #15058. I'm Bonk, and I've done a quick review of your PR. Summary: the PR replaces the hard error on a Issues raised (ranked):
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return ( | ||
| complianceRegionFromEnv || complianceConfig?.compliance_region || "public" | ||
| ); |
There was a problem hiding this comment.
🔴 Commands that only work in the public region can now silently talk to the FedRAMP endpoint
Commands that declare they only support the public region now have that declaration silently overridden by the environment variable (complianceRegionFromEnv || complianceConfig?.compliance_region at packages/workers-utils/src/environment-variables/misc-variables.ts:102-104) with no error or warning, so those commands quietly send their requests to the FedRAMP High servers instead.
Impact: Users with the FedRAMP environment variable set will see Pages, tail and similar commands fail or behave unexpectedly against the wrong servers, with no message explaining why.
Why the public-only marker is no longer honoured
Many call sites pass the COMPLIANCE_REGION_CONFIG_PUBLIC sentinel (packages/workers-utils/src/environment-variables/misc-variables.ts:75-79), documented as "Used for commands that explicitly do not support compliance regions other than 'public'" — e.g. packages/wrangler/src/tail/createTail.ts:86, packages/wrangler/src/pages/upload.ts:116, packages/wrangler/src/api/pages/deploy.ts:158.
Previously, when CLOUDFLARE_COMPLIANCE_REGION=fedramp_high was set, the removed conflict check threw a UserError, so the user was told the command could not run in that region. After this PR the env value simply wins, and getComplianceRegionSubdomain returns .fed, so requests go to api.fed.cloudflare.com for commands that explicitly do not support it.
The new warning added in packages/wrangler/src/config/index.ts:72-89 only fires when config.compliance_region is set in the user's config file during readConfig/readPagesConfig; it never covers the sentinel path, so the override is completely silent here.
Prompt for agents
Removing the conflict check in getCloudflareComplianceRegion means the CLOUDFLARE_COMPLIANCE_REGION environment variable now silently overrides COMPLIANCE_REGION_CONFIG_PUBLIC, the sentinel used by commands (Pages commands, tail, etc.) that explicitly do not support any region other than "public". Those commands will now issue requests against api.fed.cloudflare.com with no error or warning. Consider distinguishing the explicit "public-only" sentinel from a user-configured compliance_region: either keep erroring (or at least warn) when the env var conflicts with the public-only sentinel, or have those call sites bypass env resolution entirely so they always resolve to "public".
Was this helpful? React with 👍 or 👎 to provide feedback.
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
✅ All changesets look good |
| ) => { | ||
| const complianceRegionFromEnv = getCloudflareComplianceRegionFromEnv(); | ||
| if ( | ||
| complianceRegionFromEnv !== undefined && | ||
| complianceConfig?.compliance_region !== undefined && | ||
| complianceRegionFromEnv !== complianceConfig.compliance_region | ||
| ) { | ||
| throw new UserError( | ||
| dedent` | ||
| The compliance region has been set to different values in two places: | ||
| - \`CLOUDFLARE_COMPLIANCE_REGION\` environment variable: \`${complianceRegionFromEnv}\` | ||
| - \`compliance_region\` configuration property: \`${complianceConfig.compliance_region}\` | ||
| `, | ||
| { telemetryMessage: false } | ||
| ); | ||
| } | ||
| return ( | ||
| complianceRegionFromEnv || complianceConfig?.compliance_region || "public" | ||
| ); |
There was a problem hiding this comment.
🟡 Account info command reports the wrong source for the compliance region
The compliance region source shown by the account info output is chosen purely from the configured value (complianceConfig?.compliance_region at packages/wrangler/src/user/whoami.ts:98) even though the environment variable now wins, so users are told the region came from their configuration file when it actually came from the environment.
Impact: Someone checking which compliance region they are operating in sees a misleading explanation of where that region came from, making a misconfigured environment variable hard to spot.
How removing the conflict error exposes the mislabelling in printComplianceRegion
Before this PR, getCloudflareComplianceRegion threw a UserError whenever CLOUDFLARE_COMPLIANCE_REGION and compliance_region disagreed, so printComplianceRegion (packages/wrangler/src/user/whoami.ts:95-105) could never be reached with conflicting values. Now the env var silently wins (packages/workers-utils/src/environment-variables/misc-variables.ts:100-105). With compliance_region: "public" in config and CLOUDFLARE_COMPLIANCE_REGION=fedramp_high, complianceRegion resolves to fedramp_high, but since complianceConfig.compliance_region is truthy the message says the region is set "via the Wrangler configuration".
A fix would be to compare the resolved region against the configured one (as logWarningsIfComplianceRegionIsOverridden in packages/wrangler/src/config/index.ts:72-89 does) and attribute the source to the environment variable when they differ.
(Refers to lines 100-105)
Prompt for agents
In packages/wrangler/src/user/whoami.ts, printComplianceRegion determines the "source" of the compliance region by checking whether complianceConfig.compliance_region is set. Previously a conflict between CLOUDFLARE_COMPLIANCE_REGION and the configured compliance_region threw a UserError, so a conflicting state was unreachable. After this PR the environment variable silently takes precedence, so when the config sets one region and the env var another, whoami reports the resolved (env-var) region but attributes it to the Wrangler configuration. Update the source attribution so it reflects which value actually won — e.g. attribute to the environment variable whenever the resolved region differs from the configured one, or when the env var is set.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ) => { | ||
| const complianceRegionFromEnv = getCloudflareComplianceRegionFromEnv(); | ||
| if ( | ||
| complianceRegionFromEnv !== undefined && | ||
| complianceConfig?.compliance_region !== undefined && | ||
| complianceRegionFromEnv !== complianceConfig.compliance_region | ||
| ) { | ||
| throw new UserError( | ||
| dedent` | ||
| The compliance region has been set to different values in two places: | ||
| - \`CLOUDFLARE_COMPLIANCE_REGION\` environment variable: \`${complianceRegionFromEnv}\` | ||
| - \`compliance_region\` configuration property: \`${complianceConfig.compliance_region}\` | ||
| `, | ||
| { telemetryMessage: false } | ||
| ); | ||
| } | ||
| return ( | ||
| complianceRegionFromEnv || complianceConfig?.compliance_region || "public" | ||
| ); |
There was a problem hiding this comment.
🟨 Conflicting compliance region settings no longer block the command, allowing FedRAMP-configured projects to target the public region
The hard error that previously stopped Wrangler when CLOUDFLARE_COMPLIANCE_REGION disagreed with the configured compliance_region was removed (packages/workers-utils/src/environment-variables/misc-variables.ts:100-105), so a project explicitly configured for fedramp_high will now silently deploy against the public API endpoints whenever a stray or attacker-influenced environment variable sets public, downgraded to only a warning emitted at config-read time (packages/wrangler/src/config/index.ts:72-89).
(Refers to lines 100-105)
Was this helpful? React with 👍 or 👎 to provide feedback.
workers-devprod
left a comment
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
3 participants
Warn when
CLOUDFLARE_COMPLIANCE_REGIONconflicts with the configured compliance region. The environment variable now takes precedence and Wrangler continues instead of rejecting the command, while explaining how the region was resolved.This aligns Wrangler's behavior with cf's compliance-region resolution.
A picture of a cute animal (not mandatory, but encouraged)