fix(appliance): keep the dashboard off globally-routable addresses (#1021) - #1036
Merged
Conversation
Routine sync, 32 commits. Five conflicts, all resolved against the code rather than by preferring a side: - pithead / control_upgrade: UNION. The appliance refusal (v2) and the cosign precondition (#1023, develop) are independent preconditions and both belong. Appliance is checked FIRST: an appliance cannot take a tarball upgrade whatever the host holds, so that is the informative answer. - docker-compose.yml / caddy: UNION. v2 added the appliance TLS mount, develop added the config-loaded healthcheck (#904). - Makefile / test-patch-coverage: took develop. #1000 replaces the bare diff-cover call with scripts/patch-coverage.sh, which closes the vacuous pass where diff-cover exits 0 on "No lines with coverage information". - web/server.py: UNION. v2 added the backup handlers (#908), develop added _RECORDABLE_WORKER_STATUSES and the change_type parameter (#1014). Verified no duplicate definition results and that the new parameter defaults, so existing 4-arg callers still work. - docs/dev/testing-strategy.md: kept develop's new #972 row AND v2's wording of the double-outage row. The wording was decided by the code, not by branch preference: data_service readmits on monerod alone and test_readmit_ignores_tari_state_entirely pins it. Gates on the merge result: make lint 0 errors (docs voice + operator strings clean), stack 2333/0, dashboard 1818/0 at 96.78% coverage, frontend 375/0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…1021) The appliance built its Caddy site list from every address `hostname -I` reports. On any network that passes IPv6 through, a SLAAC/DHCPv6 global unicast address landed there exactly like a LAN one — and render_derived rebuilds it every boot, so the control panel was republished on a globally-routable address on every restart, with nothing but the operator's router between it and the internet. Confirmed live on the physical bench: the generated Caddyfile carried `https://2605:59c8:...` and the dashboard answered on it. Two changes, because the obvious one is not a boundary. Filtering the site list is necessary but NOT sufficient. Caddy runs host-networked and opens ONE WILDCARD listener — `ss -lnt` on the appliance showed `*:443`, not per-address sockets — and it matches on Host content, never on which interface a connection arrived over. So trimming the list alone leaves the socket open on the public address; a client only has to send a Host header naming an address that IS still listed. `bind` is what actually closes it. So: skip public addresses (via the existing is_public_ip, which already classifies 2000::/3 and the v4 private ranges) unless the new dashboard.expose_public_ip opts back in, AND emit a `bind` carrying the surviving literal addresses plus loopback. Literal addresses only — a NAME would be resolved by Caddy, and the mDNS name resolves to every address the box holds, including the one being excluded. The onion vhost gets its own bind for the same reason: a second site with no bind re-opens a wildcard on :80. Nothing supported regresses. The documented way to reach the dashboard off-LAN is the onion service, explicitly "rather than forwarding a port"; the one cross-host flow (xvb.standby.source) is documented as onion or LAN address only. Verified on the physical appliance, before and after: listeners *:443 and *:80 -> 192.168.1.202, 10.89.0.1, 172.28.0.1, fd1c:… ULA, 127.0.0.1, ::1 — no global-v6 socket at all LAN no creds / creds / mDNS 401/200/401 -> 401/200/401 (unchanged) global IPv6 served -> connection refused Eight unit tests over the real address set from that box. One of them exists because the bench caught what the tests did not: `$(...)` strips trailing newlines, so a newline inside the helper glued `basic_auth {` onto the bind line and produced a Caddyfile Caddy cannot parse. The shape is now pinned so that cannot come back. dashboard.expose_public_ip is added to config.reference.json — the reference must stay a complete superset of every path pithead reads or the control gate false-rejects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…or (#1021) A new config.reference.json leaf with no LOGICAL_GROUPS prefix falls into "Other" in the config editor, and the frontend suite pins exactly that: "every config.reference.json leaf path resolves to a REAL logical group, not Other". Adding dashboard.expose_public_ip without this broke it. Caught by running the frontend suite on the integration branch — the #1021 commit was gated on `make lint` + tests/stack only, which do not cover the config editor's grouping. That is the gap, not the key: any config addition needs the frontend suite too. It belongs in "Dashboard & access" beside dashboard.host, which is the setting it qualifies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…vhost to it (#1021) Three defects in the first cut of this fix, all found by adversarial review before merge. Two would have shipped the bug the fix exists to close, or a new outage. 1. The bind was DERIVED FROM the site list and gated on the same condition as the site filter (is_appliance && dashboard.host unset). An operator who PINS dashboard.host — documented and supported, and the most deliberately-configured boxes there are — got a single-host site list and NO bind at all: the wildcard listener, and the entire exposure, back again. Caddy matches on Host content, never on arrival interface, so a client on the global address only had to send `Host: <the pinned LAN address>`. The bind now comes from the BOX's own addresses via `hostname -I` + is_public_ip, independent of the site list, because the two answer different questions: which Host values Caddy matches, versus which sockets it opens. The original tests never set DASHBOARD_HOST, which is why they passed. 2. Loopback was documented as "always added" but was appended inside the guarded path, so a box reporting no usable non-public address returned before reaching it — no bind at all, silently back to a wildcard. It is appended outside the loop now. 3. The onion vhost's bind was unconditional while the LAN vhost's was not. A site block with no bind asks Caddy for a WILDCARD listener, so the two must agree: with dashboard.secure:false and the onion enabled — documented, and explicitly exempted from the insecure-transport warning — the file would carry a wildcard :80 and a specific NETWORK_PREFIX.1:80, both claiming the port. Caddy fails to start, taking the dashboard AND the onion down together. The onion now binds exactly when the LAN vhost does. Five new cases cover precisely what was missed: a pinned dashboard.host still binds and still excludes the global address; a box holding only a public address still binds loopback; and the insecure+onion combination binds in both blocks or neither, on appliance and DIY. make lint 0 errors · tests/stack 2347 passed, 0 failed · frontend 375 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A config key shipped without documentation. The standing bar is that a change lands with its docs; this one added dashboard.expose_public_ip to config.reference.json and said nothing about it anywhere an operator would look. Names what the default protects against and why the onion service is the supported way in, so nobody flips it to true expecting it to be the normal remote-access route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6 tasks
…undle signing trap Updating a bench entirely over SSH is proven on the physical appliance — bundle copied, installed, rebooted onto the new slot, /data and its synced chain untouched. That was not written down anywhere, so the next person would reach for a USB stick. The trap that costs the time is signing: mkbundle.sh --dev mints a NEW throwaway chain every run, so a --dev bundle only installs on the machine built from that same run. Anywhere else RAUC refuses with 'self-signed certificate', which reads like a broken bundle rather than the wrong key. Documents naming the original chain instead, and checking the fingerprint against the target's keyring before copying half a gigabyte to it. Repeats the variant-flip warning at the point of use, since --yes skips the guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ly-dashboard # Conflicts: # pithead
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.
Closes #1021. Stacked on #1031 (the sync merge) — merge that first.
The appliance built its Caddy site list from every address
hostname -Ireports. On any network passing IPv6 through, a SLAAC/DHCPv6 global unicast address landed there exactly like a LAN one, andrender_derivedrebuilds the list every boot — so the control panel was republished on a globally-routable address on every restart, with nothing but the operator's router between it and the internet.Confirmed live on the physical bench, not just from source: the generated Caddyfile carried
https://2001:db8::1and the dashboard answered on it.Why this needs two changes
Filtering the site list is necessary and not sufficient. Caddy runs host-networked and opens one wildcard listener —
ss -lnton the appliance showed*:443, not per-address sockets — and it matches onHostcontent, never on which interface the connection arrived over. Trimming the list alone leaves the socket open on the public address; a client only has to send aHostheader naming an address that is still listed.bindis what actually closes it.is_public_ip(already classifies2000::/3and the v4 private ranges — it was only used for an unenforced warning), unless the newdashboard.expose_public_ipopts back in.bindwith the surviving literal addresses plus loopback. Literal only: a name would be resolved by Caddy, and the mDNS name resolves to every address the box holds — including the one being excluded.bind; a second site with no bind re-opens a wildcard on:80.Verified on the physical appliance, before → after
*:443,*:80192.168.1.10,10.89.0.1,172.28.0.1,fd1c:…ULA,127.0.0.1,::1— no global-v6 socketNothing supported regresses
docs/configuration.mddocuments the off-LAN route as the onion service, explicitly "rather than forwarding a port", and the one cross-host flow (xvb.standby.source) is documented as onion-or-LAN-address only.Tests
Eight unit tests over the real address set from that box (LAN v4, two podman bridges, a global v6, a ULA), covering both the default and the opt-in.
One of them exists because the bench caught what the tests did not:
$(...)strips trailing newlines, so a newline inside the helper gluedbasic_auth {onto the bind line and produced a Caddyfile Caddy cannot parse — a config that would have taken the dashboard down. The shape is now pinned so it cannot come back. That gap (nothing validates the generated Caddyfile against real Caddy at any tier) is worth closing separately.make lint0 errors ·tests/stack2342 passed, 0 failed.dashboard.expose_public_ipis added toconfig.reference.json— the reference must stay a complete superset of every path pithead reads, or the control gate false-rejects.