Keep the health check reachable when the IP allowlist is enforced - #36
Merged
DorwardTech merged 2 commits intoJul 25, 2026
Merged
Conversation
Turning on IP enforcement took the site down rather than locking one person out of the admin. The container health-checks itself with `wget http://127.0.0.1:8080/up`. That is loopback, so there is no CF-Connecting-IP, so the allowlist judged the request as 127.0.0.1 — an address nobody thinks to put on an allowlist of their own home connection. It answered 403, six failures marked the container unhealthy, and the orchestrator killed a container that was working perfectly. The allowlist admin page has claimed "/up is always reachable" the whole time. The middleware never implemented it. This makes the page true. Exempting the route beats documenting "also add 127.0.0.1": that entry would have to stay on the list forever, and of every address that could be on an allowlist it is the one an attacker can assume is there — which matters because a request-supplied header decides which address gets checked. The exemption is one route wide, returns before the audit write so `blocked` stays a list of real rejections rather than one line every fifteen seconds, and the endpoint reports only whether the app, database and Redis answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014E51bA27LnFMK7v4YigZY1
The guest case proved nothing about this middleware. `auth` implements AuthenticatesRequests, which is in Laravel's $middlewarePriority, and priority sorting places it ahead of middleware merely appended to the web group — so a guest hitting /admin is redirected to /login before the allowlist runs, and the test asserted 403 against a 302. The code was right. Signed in, the allowlist is reached and does reject, which is the property worth pinning: the health exemption is one route wide and has not leaked. /login keeps the guest-facing half, where nothing is sorted ahead of the allowlist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014E51bA27LnFMK7v4YigZY1
DorwardTech
marked this pull request as ready for review
July 25, 2026 11:39
DorwardTech
merged commit Jul 25, 2026
6c1caa0
into
claude/zone3-darwin-internal-tool-YQKKN
2 checks passed
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.
What happened
Turning on IP enforcement took the site down, rather than locking one person out of the admin.
Dockerfile:120health-checks the container with:That is loopback, so there is no
CF-Connecting-IPheader, soClientIp::from()falls through toREMOTE_ADDRand the allowlist judges the request as 127.0.0.1 — not an address anyone thinks to put on an allowlist of their own home connection./upis a plain route inroutes/web.php, andEnforceIpAllowlistis appended to thewebgroup, so it answered 403. Six failures at 15s intervals marked the container unhealthy and the orchestrator killed a container that was working perfectly.The allowlist admin page has claimed this exemption the whole time.
resources/views/admin/ip-allowlist/index.blade.php:14:The middleware never implemented it. This makes the page true rather than changing the intended design.
Why exempt the route instead of documenting "also add 127.0.0.1"
That entry would have to stay on the list forever, and of every address that could be on an allowlist it is the one an attacker can assume is there. That matters more than it looks, because a request-supplied header decides which address gets checked — see below.
The exemption is deliberately one route wide, and returns before the audit write so
zone3:ip-allowlist blockedstays a list of real rejections rather than one line every fifteen seconds forever./upreports only whether the app, database and Redis answer, and was publicly reachable before enforcement existed.Tests
Six, including the exact regression: loopback with no CF header, enforcement on, 127.0.0.1 not on the list → 200. Plus that the exemption is one route wide (
/adminand/loginstill 403 from a non-listed address), that a listed address still gets through, and that a health check is not recorded as a blocked request.Related, and not fixed here — the origin firewall is the control that matters
While tracing how the request's IP is decided I followed it end to end, and the answer is that it is attacker-controlled unless the origin refuses non-Cloudflare traffic.
docker/nginx.confsetsreal_ip_header CF-Connecting-IPtogether withset_real_ip_from 172.16.0.0/12(and the other private ranges). Coolify's Traefik sits in front on the docker network, so every request reaches nginx from a private address that is on that list — which means nginx rewrites$remote_addrfrom theCF-Connecting-IPheader on every request, whether or not it came through Cloudflare.App\Support\Security\ClientIpthen prefers that same header directly, with no check at all:So if the host answers on its raw IP,
curl -H "CF-Connecting-IP: 198.51.100.42"picks both the address the allowlist checks and the address the audit log records.I am not fixing that in this PR, and specifically not with an app-layer range check, because that would look like a fix and would not be one: nginx has already rewritten
REMOTE_ADDRby the time PHP runs, and nginx genuinely cannot tell a Cloudflare-forwarded request from a direct one when Traefik is the peer in both cases. The decisive control is network-level — the host firewall accepting 80/443 only from Cloudflare's published ranges, or a Cloudflare Tunnel so the origin has no public port at all.Worth checking that before relying on the allowlist for anything. Login and TOTP are unaffected either way, so this is a lost defence layer rather than a way in — but attacker-controlled audit IPs is the part I would want closed.
Generated by Claude Code