Add a shell escape hatch for the IP allowlist - #35
Merged
DorwardTech merged 1 commit intoJul 25, 2026
Merged
Conversation
Every control over the allowlist lives on a page the allowlist guards, so getting it wrong locks you out of the tool you need to fix it. This is the same set of operations from a shell, with no HTTP request involved: php artisan zone3:ip-allowlist # status: enforcement and entries php artisan zone3:ip-allowlist off # the way back in php artisan zone3:ip-allowlist blocked # which addresses were rejected php artisan zone3:ip-allowlist allow 203.0.113.7 php artisan zone3:ip-allowlist forget 203.0.113.7 `off` leaves the entries in place so it can be reversed without retyping the list, verifies the change by reading it back through the same path the middleware uses rather than trusting that a write returned, and reports a cache flush failure as "written, run cache:clear" instead of a stack trace over a half-applied change. `blocked` is the diagnostic the 403 page cannot be: it lists the addresses the app actually turned away, grouped, from the audit log. Behind Cloudflare the address checked is CF-Connecting-IP, which is whichever protocol the browser used — so a household with IPv6 arrives from an address that looks nothing like the IPv4 a "what is my IP" page reports, and adding the IPv4 changes nothing. Being able to read the real one is the difference between fixing it and guessing. `allow` runs the same shape check as the admin form and additionally requires the address to parse, because a malformed entry would sit in the table looking valid and silently never match — the same lockout with no visible cause. A duplicate is a no-op rather than a unique-constraint 500 in a shell someone is using under pressure. Default action is `status`, so a half-remembered command reports rather than changes. 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:05
DorwardTech
merged commit Jul 25, 2026
b5e69c4
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.
Branched off mainline rather than stacked on #34, so it can merge and deploy on its own.
Why
Every control over the IP allowlist is on a page the allowlist guards. Getting it wrong locks you out of the tool you need to fix it, and there was no way back in short of editing the database by hand.
The command
Default action is
status, so a half-remembered command reports rather than changes.blockedis the part that mattersThe 403 page cannot say which address it saw, and that is nearly always the actual question.
Behind Cloudflare the address checked is
CF-Connecting-IP— whichever protocol the browser used. A household with IPv6 arrives from an address that looks nothing like the IPv4 a "what is my IP" page reports, so adding the IPv4 changes nothing and gives no clue why.blockedreads the real ones out of the audit log (security.ip_blockedalready records them), grouped, because a browser makes several requests per page and the raw list is one address repeated.Care taken in the failure paths
This runs when something is already wrong, so the unhappy paths are the ones that count:
offverifies rather than assumes. It reads the value back through the sameIpAllowlistpath the middleware uses, so success means the middleware will agree — not merely that a write returned.cache:clear", not a stack trace over a half-applied change. The DB write is what matters; the flush is what makes it take effect.offleaves the entries in place, so it is reversible without retyping the list. Nobody uses an emergency switch that costs them their configuration.allowrequires the address to parse, on top of the admin form's shape check. A malformed entry would sit in the table looking valid and silently never match — the same lockout with no visible cause.allowis a no-op, not a unique-constraint 500 in a shell someone is using under pressure.statusstates that an enforced-but-empty list allows everything. It reads like total lockout and is the opposite, and that sentence is worth having in front of someone who is already worried.Separately:
ClientIptrustsCF-Connecting-IPunconditionallyNot changed here, and flagged rather than fixed because changing IP resolution during a lockout is the wrong time.
TrustCloudflareIpmiddleware is careful — it only rewritesREMOTE_ADDRwhen the connection genuinely came from a Cloudflare range. ButClientIp::from(), which is whatEnforceIpAllowlistand the audit log actually call, prefers the rawCF-Connecting-IPheader with no such check:If the Coolify origin is reachable directly by IP — bypassing Cloudflare — anyone can send that header and choose the address the allowlist checks, and the address the audit log records. It costs a defence-in-depth layer (login and TOTP still stand) and it makes audit IPs attacker-controlled, which is the part I would want fixed. The fix is to have
ClientIpapply the same Cloudflare-range test the middleware already implements, but it should land on its own, with the origin firewall checked at the same time.Tests
19 covering:
offtaking effect as the middleware reads it, entries surviving it, the audit trail,blockedoutput and its empty case, add/remove, the duplicate no-op, four malformed addresses refused, missing arguments, and the enforced-but-empty warning.vendor/can't be installed in this environment (codeload.github.comis blocked by the proxy), so CI is the verification.Generated by Claude Code