Inject CF-Connecting-IP from workerd clientIp

[penalosa]

Fixes CUSTESC-46864, fixes #7588

When deployed, Workers can access the IP of the client that initiated a request using the CF-Connecting-IP header. However, this wasn't available locally, and so there was previously no way to know the IP address of a connecting client from a wrangler dev session.

As it turns out, workerd injects a clientIp (that appears to be of the form {ip}:{port}— @danlapid or @kentonv could you confirm the format here?) using the request.cf property. Previously, the request.cf injected by workerd wouldn't make it to a user worker, because Miniflare injects a more fleshed out request.cf value that reflects what would be seen in production. As such, this PR extracts the clientIp from workerd's request.cf and puts it in the CF-Connecting-IP header before Miniflare injects it's own request.cf value.

---

• Tests
  • TODO (before merge)
  • Tests included
  • Tests not necessary because:
• E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
  • I don't know
  • Required
  • Not required because: covered by Miniflare tests
• Public documentation
  • TODO (before merge)
  • Cloudflare docs PR(s):
  • Documentation not necessary because: matching (documented) production behaviour

[changeset-bot]

🦋 Changeset detected

Latest commit: 81cd9e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
miniflare	Minor
@cloudflare/pages-shared	Patch
@cloudflare/vitest-pool-workers	Patch
wrangler	Patch

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

[danlapid]

clientIp can be: "<ipv4>:<port>" or "[<ipv6>]:port" or "(inet_ntop error)" or "unix-abstract:<unix_socket_path>" or "unix:<unix_socket_path>" or "(unknown address family <address family int>)".
There are many cases here but I'm not sure all of them really need to be handled.
IMO the logic should be:

1. Try to find ":" in the clientIp string (otherwise don't set CF-Connecting-IP at all)
2. Try to find "[" and "]" in the clientIp string split on ":"'s first part and if found set CF-Connecting-IP to be the ipv6 address between those
3. Try to find three dots in the clientIp string split on ":"'s second part and if found set CF-Connecting-IP to be the first part of the split.
4. Don't set CF-Connecting-IP.

(Notice header format in https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#cf-connecting-ip-in-worker-subrequests for IPV6 does not contain square brackets).

[github-actions]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-wrangler-7702

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7702/npm-package-wrangler-7702

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-wrangler-7702 dev path/to/script.js

Additional artifacts:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-workers-bindings-extension-7702 -O ./cloudflare-workers-bindings-extension.0.0.0-v8860856ae.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v8860856ae.vsix

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-create-cloudflare-7702 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-kv-asset-handler-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-miniflare-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-pages-shared-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-unenv-preset-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-vitest-pool-workers-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-workers-editor-shared-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-workers-shared-7702

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12691563647/npm-package-cloudflare-workflows-shared-7702

Note that these links will no longer work once the GitHub Actions artifact expires.

---

wrangler@3.100.0 includes the following runtime dependencies:

Package	Constraint	Resolved
miniflare	workspace:*	3.20241230.0
workerd	1.20241230.0	1.20241230.0
workerd --version	1.20241230.0	2024-12-30

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

[danlapid]

LGTM but I don't feel like I can comment on mini flare code 😅
Up to you to decide if you want more eyes on this

[petebacondarwin]

LGTM - nice solution

[godfat]

Does this header come from the client?

If so, wouldn't this cause a security concern that this header can be spoofed (because we have to trust it)? I know that running wrangler dev is for the development anyway, but I don't feel too comfortable if this can cause a security concern even if it's just for local development (and running on CI without deploying to Cloudflare)

[penalosa]

It does come from the client, yeah—since this is intended to be used for local development I'm not sure there's a huge security concern, and I think the benefit of being able to mock this value might outweigh it.

In general, we don't recommend running a wrangler dev server exposed to the public internet, as it's not really designed for that purpose.

[godfat]

You're right and that makes sense. However, I still have concerns with regard to how GitLab might be using this. For now it's purely on CI, and it's probably not accessible from the public internet. However, this does increase the attack vector that if the client is compromised somehow, containing malicious code, it could expose some internal access.

Also, GitLab utilizes so called "Review Apps", which runs a GitLab instance for each merge requests (pull requests) requesting it. For now it does not run wrangler in any way, but if we need, we have to use wrangler because for that deployment process it cannot deploy to Cloudflare. These Review Apps are public accessible, which need to be protected.

Is it possible to provide an option to toggle this behaviour? If so, we GitLab can toggle it so that we never trust the client's headers. Actually, I think it should be an option to allow the client to mock it, not the other way around, if an option is provided. The default should be secure, and I am open to have options for anything reasonable like mocking the IP under trusted environments.

What do you think?

[Skye-31]

Even if the machine that wrangler is running on is open to the internet, wrangler will listen on 127.0.0.1 by default, not 0.0.0.0, so it won't be reachable unless you explicitly set it to be.

we have to use wrangler because for that deployment process it cannot deploy to Cloudflare

This change is also specific to wrangler dev, not deploying, so I'm not really sure what you mean by this

[godfat]

Even if the machine that wrangler is running on is open to the internet, wrangler will listen on 127.0.0.1 by default, not 0.0.0.0, so it won't be reachable unless you explicitly set it to be.

This is not about being reachable or not, it's about the client passing CF-Connecting-IP: 127.0.0.1 (as an example) header, to pretend that the client IP is 127.0.0.1 but in fact it's something else. The server trusting CF-Connecting-IP would think that is true, and allow the client to access internal resources which should normally be blocked with the actual client IP.

To mitigate this, the header from the client should be completely ignored and dropped.

This change is also specific to wrangler dev, not deploying, so I'm not really sure what you mean by this

I mean we have to run wrangler in some environments which should be secure, and not just for local development. We have to run wrangler because in those cases we cannot run them via Cloudflare. This means we kinda need to treat wrangler as a semi-production environment, which is also public facing.

[penalosa]

I mean we have to run wrangler in some environments which should be secure, and not just for local development. We have to run wrangler because in those cases we cannot run them via Cloudflare. This means we kinda need to treat wrangler as a semi-production environment, which is also public facing.

The Wrangler/miniflare security model is not designed for being exposed on the public internet—they're tools intended for local development and testing. There will be other security concerns, but off the top of my head Wrangler exposes a devtools connection that allows for arbitrary code execution.

We can potentially add an option for dropping CF-Connecting-IP, but I'd strongly recommend against running wrangler dev in production and relying on this value for security checks to production resources.

We have to run wrangler because in those cases we cannot run them via Cloudflare

Could you elaborate on this? Deploying the worker to Cloudflare sounds like the right solution for your use case.

[godfat]

There will be other security concerns, but off the top of my head Wrangler exposes a devtools connection that allows for arbitrary code execution.

Oh well, ok, this sounds a big no to begin with, so I would agree it'll be a moot point to secure other things with this in mind.

We can potentially add an option for dropping CF-Connecting-IP, but I'd strongly recommend against running wrangler dev in production and relying on this value for security checks to production resources.

To be fully clear, what I have in mind is not truly production. As mentioned above with the "Review Apps", it's scoped to each merge requests, which is considered part of development, but since they're deployed to GCP and it has some kind of admin access to the underneath sandbox environment, we have to protect it. At worst it can probably be exploited to destroy our sandbox environment, which doesn't really touch the actual production environment, but it's bad enough that it should be protected.

Anyway, with the following in mind:

There will be other security concerns, but off the top of my head Wrangler exposes a devtools connection that allows for arbitrary code execution.

At best if we still need to run it, it probably needs to sit behind another reverse proxy inside a private network, and from there we can replace CF-Connecting-IP with the actual client IP, and this option wouldn't really be needed either.

Could you elaborate on this? Deploying the worker to Cloudflare sounds like the right solution for your use case.

I am not very familiar with all the details, and I'll quote this comment: https://gitlab.com/gitlab-org/gitlab/-/issues/499983#note_2235810346

In short, the environment is completely controlled by helm and it's deployed to a k8s cluster. I do not know if we're able to integrate Cloudflare which is external to the k8s cluster.

We don't need this yet, but as we put more into the Cloudflare worker, we might need in the future, and that's why I would like to validate if it's possible even before we really need it. This is also more or less for running end-to-end tests, rather than deploying an actual application.

[godfat]

I have a follow up question and it would be nice if you can answer that. (It's the same from Cloudflare support channel in the GitLab Slack workspace, posting here for transparency)

While I think it's not ideal that we cannot use wrangler in a secure way, I see it's not designed to be used to run securely. My follow up question will be, is there any plans to make it secure? While I understand it's only designed to be used as a local development tool, as we run more complex stuffs on it, I think it's important to have a way to run it securely. It doesn't need to be feature full like it is running on Cloudflare, but for end-to-end tests I think it'll be valuable.