Skip to content

Commit 4da24fc

Browse files
committed
domains/spf: fall through to tunnels.json when DB has no tunnel rows
Operators who bootstrap the relay sidecar by hand-editing `/etc/mailcue-sidecar/tunnels.json` (or via env vars) never populate the `tunnels` table. `_list_active_tunnel_hosts` would then return an empty list and `_build_spf_expected` would fall back to the single-host default (`v=spf1 mx a:<hostname> ~all`) even when the live apex DNS correctly authorised every relay (`a:relay-us... a:relay-de... -all`). Result: every page load showed a phantom drift on the apex SPF record forever, on any deployment that didn't go through the API path. Fix: when the DB has no enabled tunnels, read `tunnels.json` directly and extract the enabled `host` fields. This keeps the API path as the preferred source of truth (DB wins when populated) but lets externally managed sidecars surface the right SPF guidance without forcing the operator to mirror their JSON into the API. The reader is defensive: missing file, unparseable JSON, schema mismatch all return an empty list. SPF generation must never depend on the sidecar's filesystem being available. Also: scrub example.com placeholders into tunnel/README.md and the new test fixtures (the previous draft used a contributor's real production domain as illustrative data).
1 parent 9944f89 commit 4da24fc

3 files changed

Lines changed: 175 additions & 19 deletions

File tree

backend/app/domains/service.py

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import asyncio
66
import contextlib
7+
import json
78
import logging
89
import re
910
import subprocess
@@ -462,12 +463,65 @@ def _reload() -> None:
462463
# ── High-level API ───────────────────────────────────────────────
463464

464465

465-
async def _list_active_tunnel_hosts(db: AsyncSession) -> list[str]:
466-
"""Return the deduped, ordered list of enabled tunnel ``endpoint_host``s.
466+
def _read_tunnel_hosts_from_json(path: str | Path) -> list[str]:
467+
"""Read enabled relay hosts from the sidecar's ``tunnels.json``.
468+
469+
The sidecar (``mailcue-relay-sidecar``) treats this file as its single
470+
source of truth — it loads it on boot and reloads on inotify. Any
471+
relay listed here is one that outbound mail might actually egress
472+
through, so the apex SPF must authorise it.
467473
468-
These are the relay hostnames that an outbound MAIL FROM uses, so they
469-
are exactly the ``a:`` mechanisms the apex SPF record must authorise.
470-
Empty list when no tunnels are configured (single-host deployment).
474+
Returns an empty list when the file is missing, unreadable, or its
475+
schema doesn't match. Never raises — SPF generation must not depend
476+
on the sidecar's filesystem being available.
477+
"""
478+
try:
479+
raw = Path(path).read_text(encoding="utf-8")
480+
except (OSError, FileNotFoundError):
481+
return []
482+
try:
483+
data = json.loads(raw)
484+
except json.JSONDecodeError:
485+
logger.warning("Could not parse tunnels.json at %s — invalid JSON.", path)
486+
return []
487+
entries = data.get("tunnels") if isinstance(data, dict) else None
488+
if not isinstance(entries, list):
489+
return []
490+
seen: set[str] = set()
491+
hosts: list[str] = []
492+
for entry in entries:
493+
if not isinstance(entry, dict):
494+
continue
495+
# ``enabled`` defaults to True so a hand-written file that omits
496+
# the field is treated as live (matching the sidecar's behaviour).
497+
if entry.get("enabled", True) is False:
498+
continue
499+
host_value = entry.get("host")
500+
if not isinstance(host_value, str):
501+
continue
502+
host = host_value.strip().rstrip(".").lower()
503+
if host and host not in seen:
504+
seen.add(host)
505+
hosts.append(host)
506+
return hosts
507+
508+
509+
async def _list_active_tunnel_hosts(db: AsyncSession) -> list[str]:
510+
"""Return the deduped, ordered list of relay hostnames outbound mail
511+
may egress through.
512+
513+
These are the relay hostnames an outbound MAIL FROM uses, so they are
514+
exactly the ``a:`` mechanisms the apex SPF record must authorise.
515+
Source-of-truth precedence:
516+
517+
1. The ``tunnels`` DB table — populated when an admin configures
518+
tunnels via the API.
519+
2. ``/etc/mailcue-sidecar/tunnels.json`` — populated by hand or by
520+
older deployments that bootstrap the sidecar config out-of-band.
521+
Read directly so externally-managed sidecars still get accurate
522+
SPF guidance without forcing operators to mirror their config
523+
into the DB.
524+
3. Empty list — single-host deployment with no relays.
471525
"""
472526
stmt = select(Tunnel).where(Tunnel.enabled.is_(True)).order_by(Tunnel.name)
473527
result = await db.execute(stmt)
@@ -478,7 +532,9 @@ async def _list_active_tunnel_hosts(db: AsyncSession) -> list[str]:
478532
if host and host not in seen:
479533
seen.add(host)
480534
hosts.append(host)
481-
return hosts
535+
if hosts:
536+
return hosts
537+
return await asyncio.to_thread(_read_tunnel_hosts_from_json, settings.tunnels_config_path)
482538

483539

484540
def _build_spf_expected(hostname: str, tunnel_hosts: list[str]) -> str:

backend/tests/test_domains.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
_join_txt_rdata,
2929
_normalize_dkim_txt,
3030
_parse_zonefile_txt,
31+
_read_tunnel_hosts_from_json,
3132
)
3233
from app.tunnels.models import Tunnel
3334

@@ -498,6 +499,105 @@ async def test_spf_expected_includes_every_enabled_tunnel(
498499
assert body["has_drift"] is False
499500

500501

502+
# ── Tests: tunnels.json fallback for externally-managed sidecars ─
503+
504+
505+
def test_read_tunnel_hosts_from_json_returns_enabled_hosts(tmp_path: Any) -> None:
506+
"""``tunnels.json`` is the sidecar's source of truth — operators who
507+
bootstrap it directly (rather than via the API) must still get an
508+
SPF that authorises every relay."""
509+
import json as _json
510+
511+
cfg = tmp_path / "tunnels.json"
512+
cfg.write_text(
513+
_json.dumps(
514+
{
515+
"version": 1,
516+
"tunnels": [
517+
{"name": "us", "host": "Relay-US.EXAMPLE.com.", "enabled": True},
518+
{"name": "de", "host": "relay-de.example.com", "enabled": True},
519+
{"name": "old", "host": "relay-old.example.com", "enabled": False},
520+
# Duplicate host across entries — must dedupe.
521+
{"name": "us-2", "host": "relay-us.example.com"},
522+
],
523+
}
524+
)
525+
)
526+
hosts = _read_tunnel_hosts_from_json(cfg)
527+
# Order preserved, dot-normalised + lowercased, disabled dropped, duplicate dropped.
528+
assert hosts == ["relay-us.example.com", "relay-de.example.com"]
529+
530+
531+
def test_read_tunnel_hosts_from_json_handles_missing_and_garbage(tmp_path: Any) -> None:
532+
"""Must never raise: SPF generation has to keep working even when the
533+
sidecar isn't deployed or its config is mid-rewrite."""
534+
assert _read_tunnel_hosts_from_json(tmp_path / "does-not-exist.json") == []
535+
536+
bad = tmp_path / "tunnels.json"
537+
bad.write_text("not json {")
538+
assert _read_tunnel_hosts_from_json(bad) == []
539+
540+
# Schema-mismatched JSON (top-level list, no ``tunnels`` key, etc).
541+
bad.write_text("[1, 2, 3]")
542+
assert _read_tunnel_hosts_from_json(bad) == []
543+
bad.write_text('{"tunnels": "not-a-list"}')
544+
assert _read_tunnel_hosts_from_json(bad) == []
545+
546+
547+
async def test_spf_falls_through_to_tunnels_json_when_db_empty(
548+
client: AsyncClient,
549+
seed_domain: Domain,
550+
monkeypatch: pytest.MonkeyPatch,
551+
_engine_and_session: Any,
552+
tmp_path: Any,
553+
) -> None:
554+
"""The actual user-reported regression: a deployment that bootstrapped
555+
the sidecar by hand (no API calls) had zero ``Tunnel`` rows, so SPF
556+
fell back to the single-host default even though the live DNS listed
557+
multiple relays. With the JSON fallback in place, the expected SPF
558+
must reflect the relays declared in ``tunnels.json``."""
559+
import json as _json
560+
561+
cfg = tmp_path / "tunnels.json"
562+
cfg.write_text(
563+
_json.dumps(
564+
{
565+
"version": 1,
566+
"tunnels": [
567+
{"name": "us", "host": "relay-us.example.com", "enabled": True},
568+
{"name": "de", "host": "relay-de.example.com", "enabled": True},
569+
],
570+
}
571+
)
572+
)
573+
574+
from app.config import settings as _settings
575+
576+
monkeypatch.setattr(_settings, "tunnels_config_path", str(cfg), raising=False)
577+
578+
expected_spf = "v=spf1 mx a:relay-us.example.com a:relay-de.example.com -all"
579+
plan = _build_full_plan(
580+
domain=seed_domain.name,
581+
hostname="mail.example.com",
582+
selector=seed_domain.dkim_selector,
583+
dkim_value=seed_domain.dkim_public_key_txt or "",
584+
spf_value=expected_spf,
585+
)
586+
_pin_hostname(monkeypatch)
587+
_install_resolver_stub(monkeypatch, plan)
588+
589+
resp = await client.get(f"/api/v1/domains/{seed_domain.name}/dns-state")
590+
assert resp.status_code == 200, resp.text
591+
body = resp.json()
592+
spf_rec = next(
593+
r
594+
for r in body["records"]
595+
if r["record_type"] == "TXT" and r["hostname"] == seed_domain.name
596+
)
597+
assert spf_rec["expected_value"] == expected_spf
598+
assert spf_rec["drift"] is False
599+
600+
501601
# ── Tests: MTA-STS id is operator-chosen ─────────────────────────
502602

503603

tunnel/README.md

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,15 @@ Append (replace with the FQDN you set as rDNS for **this** VPS — see
133133

134134
```ini
135135
[Service]
136-
Environment=MAILCUE_EDGE_HELO_HOSTNAME=relay-us.olib.email
136+
Environment=MAILCUE_EDGE_HELO_HOSTNAME=relay-us.example.com
137137
```
138138

139139
Then `sudo systemctl restart mailcue-relay-edge`.
140140

141141
You can also set it in `/etc/mailcue-edge/config.toml`:
142142

143143
```toml
144-
helo_hostname = "relay-us.olib.email"
144+
helo_hostname = "relay-us.example.com"
145145
```
146146

147147
---
@@ -154,15 +154,15 @@ relay through needs **all five** of the following set up before it can
154154
deliver mail. Skipping any one of them produces a 5xx rejection that
155155
looks like a tunnel bug but isn't.
156156

157-
For each VPS hostname (e.g. `relay-us.olib.email`):
157+
For each VPS hostname (e.g. `relay-us.example.com`):
158158

159159
| # | Direction | What | Where set | Verify |
160160
|---|---|---|---|---|
161-
| 1 | **Forward (A)**: hostname → IPv4 | Your DNS provider | `dig +short A relay-us.olib.email` |
162-
| 2 | **Forward (AAAA)**: hostname → IPv6 (if the VPS has IPv6 — most do) | Your DNS provider | `dig +short AAAA relay-us.olib.email` |
163-
| 3 | **Reverse (PTR) v4**: IPv4 → hostname | OVH (or other VPS provider) manager | `dig +short -x 51.81.202.4` |
164-
| 4 | **Reverse (PTR) v6**: IPv6 → hostname | OVH manager (separate row from v4!) | `dig +short -x 2604:2dc0:202:300::24de` |
165-
| 5 | **SPF** for the apex domain (`olib.email`) covering both IP families of every relay | Your DNS provider | `dig +short TXT olib.email` |
161+
| 1 | **Forward (A)**: hostname → IPv4 | Your DNS provider | `dig +short A relay-us.example.com` |
162+
| 2 | **Forward (AAAA)**: hostname → IPv6 (if the VPS has IPv6 — most do) | Your DNS provider | `dig +short AAAA relay-us.example.com` |
163+
| 3 | **Reverse (PTR) v4**: IPv4 → hostname | OVH (or other VPS provider) manager | `dig +short -x 192.0.2.4` |
164+
| 4 | **Reverse (PTR) v6**: IPv6 → hostname | OVH manager (separate row from v4!) | `dig +short -x 2001:db8::24de` |
165+
| 5 | **SPF** for the apex domain (`example.com`) covering both IP families of every relay | Your DNS provider | `dig +short TXT example.com` |
166166

167167
Two recurring traps:
168168

@@ -171,14 +171,14 @@ Two recurring traps:
171171
rDNS unset or pointing at a generic `vps-…` domain. Gmail will use
172172
whichever family the VPS picks for the outbound connection (often v6
173173
if available) and reject if PTR doesn't match.
174-
- **SPF is set on the relay hostname (`relay-us.olib.email`) but not
175-
on the apex (`olib.email`).** Gmail's SPF check runs against the
174+
- **SPF is set on the relay hostname (`relay-us.example.com`) but not
175+
on the apex (`example.com`).** Gmail's SPF check runs against the
176176
envelope `MAIL FROM` domain. Real Mailcue traffic will use
177-
`From: user@olib.email`, so the apex must authorize *every* relay
177+
`From: user@example.com`, so the apex must authorize *every* relay
178178
IP. Use the `a:` mechanism — it covers both A and AAAA records:
179179

180180
```text
181-
olib.email. IN TXT "v=spf1 mx a:relay-us.olib.email a:relay-de.olib.email -all"
181+
example.com. IN TXT "v=spf1 mx a:relay-us.example.com a:relay-de.example.com -all"
182182
```
183183

184184
This automatically authorizes all IPs of all named relay hosts; you
@@ -349,7 +349,7 @@ Common causes (read these in order):
349349
Your apex SPF doesn't authorize the IP that actually carried the
350350
outbound connection. Re-check that the SPF record covers *both*
351351
IPv4 and IPv6 of the relay (use the `a:` mechanism, see "DNS
352-
prerequisites" above). Then verify with `dig +short TXT olib.email`.
352+
prerequisites" above). Then verify with `dig +short TXT example.com`.
353353
- **`550 5.7.1 ... does not meet IPv6 sending guidelines regarding
354354
PTR records`** — the IPv6 PTR for the source IP is missing or wrong.
355355
Set it in OVH manager (separate row from the IPv4 PTR), then re-test.

0 commit comments

Comments
 (0)