Add admin.allowed_hosts to let /admin accept LAN Host headers#1
Merged
Conversation
/admin's DNS-rebinding guard only ever accepted Host values derived from api.bind plus the loopback aliases. With api.bind = "0.0.0.0:PORT" (needed for any non-loopback deployment), the literal string "0.0.0.0:PORT" is never a real browser Host header, so /admin is unreachable by LAN IP or hostname with no way to opt in. Adds a new [admin] config section with an allowed_hosts list, additive to the existing api.bind/loopback allowlist. Same Host-header exact-match gate as before, just an operator-controlled list of extra values.
- tests/admin_ui.rs: the valid-config-save assertion appends "page_size = 7\n" directly onto base_cfg assuming [api] is the last open table; my [admin] section landed after [api] and broke that. Moved [admin] before [api] in base_cfg. - config.rs: clippy (derivable_impls) flagged the manual Default impl for AdminCfg since it's just Vec::new(); derive it instead. Verified locally: cargo fmt --check, cargo clippy --all-targets -- -D warnings, and cargo test (63 tests) all pass clean.
Review fixes on top of the allowed_hosts feature: - config.rs: reject allowed_hosts entries that are empty or contain '/' or whitespace. Such a value can never equal a real Host header, so the misconfiguration would otherwise present as the same 403 the operator is trying to escape, with no signal. The check runs on both config paths (startup load and the admin editor validate through the same fn). IPv6 literals like "[::1]:8080" stay legal, pinned by test. - docs/SPEC.md §3 claims "all defaults shown"; add the [admin] section so that stays true (marked post-v1 like the /admin surface in §10). - docs/MANUAL.md: turn the "see the admin page above" cross-reference into the anchor link used elsewhere (the section is below, not above); document the new validation rule; state the exposure plainly (a listed name admits everyone on the network to every operation, CSRF token included; it is not authentication); drop an em dash per the repo prose convention. - config.rs: collapse the [admin] template comment to one line to match every other option in DEFAULT_CONFIG_TOML. Gates: cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test (64 tests) all green; the reject path also exercised against the real binary (a scheme-bearing entry fails startup with the new message).
No behavior change. Drop the rationale comments (validate()'s checks carry none), reshape the validation test after peer_id_validated, revert the AdminState field doc to its original line, and cut the MANUAL additions down to the delta: the trust-model and loopback details they repeated already live in the adjacent paragraphs and linked section.
splch
approved these changes
Jul 6, 2026
splch
left a comment
splch
left a comment
Owner
There was a problem hiding this comment.
nice addition! i ran into this issue earlier today as well
The rebuilt site shipped with SPEC's planning estimate (~6,300) in the stats strip and the graveyard card; src/ is 8,490 lines as of 0.3.0, same src-only count the old site's 8,447 used.
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
2 participants
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.
Problem
/admin's DNS-rebinding guard builds its accepted-Hostallowlist fromapi.bindplus the loopback aliases (127.0.0.1/localhost/[::1]). Thatworks when
api.bindis the default127.0.0.1:PORT, but for anynon-loopback deployment
api.bindhas to be a wildcard address like0.0.0.0:PORT— and the literal string"0.0.0.0:PORT"is never a realbrowser
Hostheader. So with a wildcard bind,/adminreturns403 host not allowedfor every possible client, with no way to opt in.Hit this running a node on a LAN container (
api.bind = "0.0.0.0:8080") —/adminwas reachable over HTTP for search, but unreachable for anythingrequiring the Host check, from any real client on the network.
Fix
New
[admin]config section:Additive to the existing
api.bind/loopback allowlist — same exact(case-insensitive)
Host-header match as before, just operator-extensible.Follows the existing config module's conventions (
deny_unknown_fields,derived
Default, documented in the commentedDEFAULT_CONFIG_TOMLtemplate, covered in
docs/MANUAL.md).Testing
cargo fmt --check,cargo clippy --all-targets -- -D warnings, andcargo test(63 tests, all existing suites + a new config test and anextension to
tests/admin_ui.rs) all pass.(
api.bind = "0.0.0.0:8080"), confirmed/adminreturned403beforethe config change and
200(all 7 operation forms rendering) after,hitting it from a separate host on the network by both hostname and IP.