From 483e208d5e6ea3d32f9ad8d522b082a2cc55ef7a Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Wed, 2 Sep 2026 12:08:10 +0100 Subject: [PATCH 1/2] 20260902 - Route blah2's views through the support hostname Support could reach a node's interface but not the three views it exists to show: Passive Radar, Max-Hold and Controller. Those are served on other ports, and Cloudflare proxies a fixed set that does not include them, so the links were dead over the tunnel and an http:// link on an https:// page would have been blocked as mixed content anyway. No code change was needed on the node. blah2's own JS already switches on is_localhost: cross-origin to :3000 on the LAN, same-origin relative paths everywhere else. It only ever needed the paths routed. So the tunnel now carries three rules instead of one, and ordering is the whole thing: cloudflared takes the first rule whose hostname and path match, so a rule without a path matches everything and silently disables every rule below it. Those rules stay listed, still read correctly in the dashboard, and never fire; the only symptom is 404s that look like a dead origin. A hand-built tunnel in this account is configured exactly that way, which is what prompted the tests to simulate the matching rather than assert the shape of the list. blah2's API endpoints are named one by one rather than matched as ^/api, because retina-gui owns /api/mode, /api/fleet/peers and others on the same hostname. A blanket prefix would divert those to blah2 and break the interface over the tunnel while leaving it working on the LAN, which is the kind of fault nobody finds until support needs it. Naming them fails loudly if blah2 gains an endpoint instead. Reconciliation now compares the live ingress against what we would write. It costs one call per provisioned node, so it scales with opt-ins rather than fleet size, and it is the only thing that would ever notice a hand-edit or a reordering, given the failure is silent by nature. Verified end to end on a real node: the rules land in order, all paths stay Access-gated, and the three views load and pull data over the tunnel. Co-Authored-By: Claude Opus 5 --- mender-auto-accept/test_tunnel_sync.py | 102 +++++++++++++++++++++++++ mender-auto-accept/tunnel_sync.py | 69 +++++++++++++++-- 2 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 mender-auto-accept/test_tunnel_sync.py diff --git a/mender-auto-accept/test_tunnel_sync.py b/mender-auto-accept/test_tunnel_sync.py new file mode 100644 index 0000000..b64160f --- /dev/null +++ b/mender-auto-accept/test_tunnel_sync.py @@ -0,0 +1,102 @@ +"""Tests for the tunnel's ingress routing. + +These simulate cloudflared's own rule matching rather than asserting the shape +of the list, because the failure that matters is not a missing rule. It is a +rule that is present, reads correctly, and never fires because something above +it matched first. That produces 404s indistinguishable from a dead origin, and +it is how a hand-built tunnel in this account was misconfigured. +""" + +import re + +import pytest + +import tunnel_sync + +NODE = "ret4c844c20" +HOST = f"{NODE}.{tunnel_sync.REMOTE_ACCESS_DOMAIN}" + + +def serves(path, node_id=NODE, hostname=None): + """Which service answers `path`, under cloudflared's first-match rule. + + cloudflared walks ingress in order and takes the first entry whose hostname + and path both match. Anything with no path matches every path. + """ + hostname = hostname or f"{node_id}.{tunnel_sync.REMOTE_ACCESS_DOMAIN}" + for rule in tunnel_sync.build_ingress(node_id): + if "hostname" in rule and rule["hostname"] != hostname: + continue + if "path" in rule and not re.search(rule["path"], path): + continue + return rule["service"] + raise AssertionError("ingress has no catch-all") + + +# ── the three views support actually uses ──────────────────────── + +@pytest.mark.parametrize("path", [ + "/display/map/", # Passive Radar + "/display/maxhold/", # Max-hold + "/controller/", # Controller + "/lib/blah2.css", # assets those pages load + "/js/plot_map.js", +]) +def test_blah2_pages_reach_the_web_port(path): + assert serves(path) == tunnel_sync.BLAH2_WEB_SERVICE + + +@pytest.mark.parametrize("path", [ + "/api/timestamp", "/api/detection", "/api/map", + "/api/adsb2dd", "/api/config", + "/capture/toggle", "/stash/detection", +]) +def test_blah2_data_reaches_the_api_port(path): + """The pages are useless without these; they are what the JS calls once it + is same-origin, which it is whenever the host is not localhost.""" + assert serves(path) == tunnel_sync.BLAH2_API_SERVICE + + +# ── the collision that would break the interface ───────────────── + +@pytest.mark.parametrize("path", [ + "/api/mode", + "/api/mode/release-spectrum", + "/api/fleet/peers", + "/api/spectrum/ready", + "/api/sdrconnect/ready", +]) +def test_retina_gui_keeps_its_own_api(path): + """The reason blah2's endpoints are named one by one instead of matching + `^/api`. These belong to the GUI, share the hostname, and a blanket prefix + would divert them to blah2: broken over the tunnel, fine on the LAN, so + nobody would find it until support tried to use it.""" + assert serves(path) == tunnel_sync.REMOTE_ACCESS_SERVICE + + +@pytest.mark.parametrize("path", ["/", "/config", "/set-up", "/static/app.css"]) +def test_the_interface_still_answers_everything_else(path): + assert serves(path) == tunnel_sync.REMOTE_ACCESS_SERVICE + + +# ── ordering, which is the whole thing ─────────────────────────── + +def test_every_path_rule_precedes_the_catch_all(): + """A rule without a path matches everything. Put one above the path rules + and they are silently disabled while still looking correct.""" + rules = tunnel_sync.build_ingress(NODE) + first_pathless = next(i for i, r in enumerate(rules) + if "path" not in r and "hostname" in r) + last_pathed = max(i for i, r in enumerate(rules) if "path" in r) + assert last_pathed < first_pathless + + +def test_the_final_rule_is_a_catch_all(): + """cloudflared requires the list to end with a rule that matches anything.""" + last = tunnel_sync.build_ingress(NODE)[-1] + assert "hostname" not in last and last["service"] == "http_status:404" + + +def test_another_node_is_not_served_by_this_tunnel(): + for rule in tunnel_sync.build_ingress(NODE): + assert rule.get("hostname") in (HOST, None) diff --git a/mender-auto-accept/tunnel_sync.py b/mender-auto-accept/tunnel_sync.py index 6066b6c..d737cfb 100644 --- a/mender-auto-accept/tunnel_sync.py +++ b/mender-auto-accept/tunnel_sync.py @@ -84,6 +84,24 @@ #: port without touching the running GUI. REMOTE_ACCESS_SERVICE = os.environ.get("REMOTE_ACCESS_SERVICE", "http://localhost:80") +#: The other two things a node serves that support needs to see: blah2's web +#: assets and blah2's API. They are separate ports on the node, and both are +#: reached through the one support hostname rather than hostnames of their own. +BLAH2_WEB_SERVICE = os.environ.get("BLAH2_WEB_SERVICE", "http://localhost:49152") +BLAH2_API_SERVICE = os.environ.get("BLAH2_API_SERVICE", "http://localhost:3000") + +#: blah2's API endpoints, named one by one rather than matched as a whole +#: `^/api` prefix. retina-gui owns /api/mode, /api/fleet/peers, +#: /api/spectrum/ready and others on this same hostname, so a blanket prefix +#: would divert those to blah2 and break the interface over the tunnel while +#: leaving it working perfectly on the LAN. Naming them fails loudly when blah2 +#: gains an endpoint, which is far better than silently stealing a GUI route. +BLAH2_API_PATH = r"^/(api/(timestamp|detection|map|adsb2dd|config)|capture|stash)" + +#: blah2's web assets. The three views support actually uses live under +#: /display/ and /controller/; /lib and /js are what those pages load. +BLAH2_WEB_PATH = r"^/(display|controller|lib|js)" + CF_API = "https://api.cloudflare.com/client/v4" CF_TOKEN = os.environ.get("CLOUDFLARE_API_TOKEN") CF_ACCOUNT = os.environ.get("CLOUDFLARE_ACCOUNT_ID") @@ -315,6 +333,38 @@ def destroy_access_app(node_id): _cf("DELETE", f"/accounts/{CF_ACCOUNT}/access/apps/{app['id']}") +def build_ingress(node_id): + """The tunnel's complete ingress config, as a list of rules. + + Pure, and separate from the call that writes it, so what a node will serve + can be read and asserted without touching Cloudflare. + + **Order is the whole thing.** cloudflared takes the first rule whose + hostname and path both match, so every rule carrying a path must come + before the one that does not. A catch-all placed above them matches + everything and silently disables the rest: the rules are still listed, still + look right in the dashboard, and never fire. That is not hypothetical, it is + how a hand-built tunnel in this account was configured, and the only symptom + was 404s that looked like the origin was down. + """ + hostname = f"{node_id}.{REMOTE_ACCESS_DOMAIN}" + return [ + {"hostname": hostname, "path": BLAH2_API_PATH, "service": BLAH2_API_SERVICE}, + {"hostname": hostname, "path": BLAH2_WEB_PATH, "service": BLAH2_WEB_SERVICE}, + {"hostname": hostname, "service": REMOTE_ACCESS_SERVICE}, + {"service": "http_status:404"}, + ] + + +def read_ingress(tunnel_id): + """What Cloudflare currently holds for this tunnel, or [] if unreadable.""" + try: + cfg = _cf("GET", f"/accounts/{CF_ACCOUNT}/cfd_tunnel/{tunnel_id}/configurations") + except requests.RequestException: + return [] + return ((cfg or {}).get("config") or {}).get("ingress") or [] + + def ensure_tunnel(node_id): """Find or create this node's tunnel, and return (tunnel_id, token, aud). @@ -331,12 +381,8 @@ def ensure_tunnel(node_id): json={"name": node_id, "config_src": "cloudflare"}) tunnel_id = created["id"] - hostname = f"{node_id}.{REMOTE_ACCESS_DOMAIN}" _cf("PUT", f"/accounts/{CF_ACCOUNT}/cfd_tunnel/{tunnel_id}/configurations", - json={"config": {"ingress": [ - {"hostname": hostname, "service": REMOTE_ACCESS_SERVICE}, - {"service": "http_status:404"}, - ]}}) + json={"config": {"ingress": build_ingress(node_id)}}) # Before the DNS record, always. The moment that record resolves the # hostname serves this node's interface, so creating the policy afterwards @@ -525,6 +571,19 @@ def reconcile(wanted, state, tunnels, dns_records, access_apps): elif not dns.get("proxied"): repairs.append((node_id, "DNS record is not proxied")) + # Costs one call per *provisioned* node, so it scales with opt-ins + # rather than fleet size, which is why it is affordable where a + # per-device check would not be. + # + # Worth the call because ingress fails silently. A rule in the wrong + # order is still listed, still reads correctly to a human, and simply + # never fires; the only symptom is 404s that look like a dead origin. + # Nothing else in this script would ever notice, and a hand-edit in the + # dashboard is exactly how it happens. + if read_ingress(tunnel["id"]) != build_ingress(node_id): + repairs.append((node_id, "ingress does not match: the paths support " + "needs may be unrouted or shadowed")) + if not tunnel.get("connections"): # Not a repair. The node may simply be off, and re-provisioning # would not bring it back; only the node connecting does. From b93817b649fe1c7efff26cdecf18d74fd6fddc32 Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Wed, 2 Sep 2026 12:12:27 +0100 Subject: [PATCH 2/2] 20260902 - Group tunnel_sync with the other imports in its test CI's ruff failed on import ordering. Nothing declares tunnel_sync first-party, so ruff groups it with pytest rather than in a section of its own. Missed locally because I checked tunnel_sync.py by name instead of the directory, so the new test file was never linted. CI runs `ruff check .`. Co-Authored-By: Claude Opus 5 --- mender-auto-accept/test_tunnel_sync.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mender-auto-accept/test_tunnel_sync.py b/mender-auto-accept/test_tunnel_sync.py index b64160f..c9ed095 100644 --- a/mender-auto-accept/test_tunnel_sync.py +++ b/mender-auto-accept/test_tunnel_sync.py @@ -10,7 +10,6 @@ import re import pytest - import tunnel_sync NODE = "ret4c844c20"