Repository navigation
fix(server): admit the reached IP on a wildcard bind; isolate the origin tests from the environment - #121
Merged
Conversation
…gin tests from the environment Both CodeRabbit functional-correctness findings from PR #120, accepted. Wildcard bind: the origin allow-list excludes wildcard addresses and the Host check runs on loopback binds only, so a UI served from a wildcard bind (start(host="0.0.0.0")) received 403 on every write: the browser sends Origin on same-origin POSTs, and the served address (http://192.168.1.10:8080) was not in _allowed_origins(). The operator's only recourse was to list the machine's address in JULIA_METAMANIFOLD_ALLOWED_ORIGINS, which the README documents for split deployments but which makes the plain wildcard-served UI unusable for writes. The guard now admits, on a wildcard bind, an origin that names the request's own IP: an http origin whose host is an IP literal (IPv4 or bracketed IPv6, never a name), on the bound port, whose host-and-port equals the Host header. A browser sends such an Origin only for a page it loaded from exactly that address, so the request is same-origin by construction; a DNS-rebinding page arrives with the attacker's domain as both Host and Origin, and a name is not an IP literal, so it is refused as before. Hostnames and proxied ports must still be listed in JULIA_METAMANIFOLD_ALLOWED_ORIGINS. Test isolation: the baseline origin assertions in test_server_security.jl assumed JULIA_METAMANIFOLD_ALLOWED_ORIGINS was unset; the suite failed for a developer running it with the variable set (a supported deployment setting). It is now cleared before the testset and restored in the finally. Tests: a new wildcard-bind testset covers the admitted IP (GET with CORS echo, preflight 204, a POST that reaches the route), the admitted IPv6 literal, other addresses, ports and schemes refused, the rebinding shape refused, and a listed hostname required without the variable. Verified locally both with the variable unset (86/86 in the file, 2636 passed in the full suite) and set to "http://localhost:3000, http://192.168.1.10:8080" (still 86/86; the same run on main fails 4 tests). Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Owner
Author
|
@coderabbitai review |
|
13 tasks
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
1 participant
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 this changes
Both of CodeRabbit's functional-correctness findings from #120, accepted and fixed.
1. Wildcard-bind writes (Major). On a wildcard bind (
start(host="0.0.0.0")) the origin allow-list excludes the wildcard address and the Host check runs on loopback binds only, so a UI served at e.g.http://192.168.1.10:8080received403on every write: browsers sendOriginon same-origin POSTs, and the served address was not in_allowed_origins(). The guard now admits, on a wildcard bind only, an origin that names the request's own IP: anhttp://origin whose host is an IP literal (IPv4 or bracketed IPv6, never a name), on the bound port, whose host-and-port equals theHostheader. A browser sends such anOriginonly for a page it loaded from exactly that address, so the request is same-origin by construction. A DNS-rebinding page arrives with the attacker's domain as bothHostandOrigin; a name is not an IP literal, so it is refused exactly as before. Hostnames (e.g.http://lab-box:8080) and proxied ports still have to be listed inJULIA_METAMANIFOLD_ALLOWED_ORIGINS.2. Test isolation (Minor). The baseline origin assertions in
test_server_security.jlassumedJULIA_METAMANIFOLD_ALLOWED_ORIGINSwas unset; the suite failed for a developer running it with the variable set (a supported deployment setting). It is now cleared before the testset and restored in thefinally.Tests
A new "Wildcard bind" testset covers: the served IP admitted (GET with CORS echo, preflight 204, a POST that reaches the route and fails there with 400, not 403), an IPv6 literal admitted, other addresses/ports/schemes refused, the DNS-rebinding shape (domain matching its own Host) refused, and a listed hostname working only with the variable set.
Verified locally, both with
JULIA_METAMANIFOLD_ALLOWED_ORIGINSunset (86/86 in this file; 2636 passed, 0 failed in the full unit suite) and set tohttp://localhost:3000, http://192.168.1.10:8080(still 86/86; the same run onmainfails 4 tests).How to run the tests
Versions tested against
Julia 1.13.1 (local; CI installs the pinned 1.12.5), R 4.6.1 (conda-forge; lockfile pins 4.5.0). No frontend change.
Notes
Hostgenerally): a rebinding page matches its own Host too, so only the IP-literal form is safe without new configuration.Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe vibe@mistral.ai