fix(onboarding): say what a domain buys, earn the tick, and write the hardening guide (#2691, #2692)

[obasilakis]

Summary

Two linked onboarding tickets. Rebased onto dev after #2715 landed (squash cf74f5001); the branch now carries only this PR's own commits.

Walked live on a fresh droplet, nobody in the room could say why an operator would add a domain, what happens when they do, or whether the setting affects routing at all. This answers that in the copy, and stops the step claiming success for something nothing has checked.

What the setting actually does

Traced end to end and now written down (requirements/infrastructure.md → PROV-016):

• It is the address Trinity hands out — Telegram, WhatsApp and VoIP callbacks, Slack's OAuth return, public chat links, workspace links, file downloads. VoIP fails outright without it; the rest fall back to an address nobody outside can use.
• On a --provision host it authorises the web server in front to obtain a certificate for that one name.
• It re-points every Telegram webhook and WhatsApp binding immediately, which was stated nowhere.
• It does not create a DNS record, and Trinity issues no certificate itself.

The tick is earned, not typed

The ticket's AC#4 suggests warning when "the hostname does not resolve to this instance". Both reviews landed on the same objection independently, so it is worth recording rather than quietly reinterpreting: a DNS lookup is wrong at the moment it would run. The step's own copy tells the operator to save before DNS settles, and the card's next step is "put your domain on Cloudflare" — a proxied record resolves to Cloudflare, not the droplet. The warning would fire hardest on the posture the product recommends. Same for a reserved IP, a load balancer, or an AAAA-only record.

So the requirement survives and the instrument changed. Caddy's on-demand-TLS gate is already called during a real handshake for the saved name, and that call arriving is proof of the whole chain: DNS resolves, traffic reaches this box, SNI matches, a certificate follows — true through any proxy in between. An authorised ask latches <iso>|<host>; is_public_url_reached() compares that host to the one in force. Until it flips, the step and Settings both say saved, waiting for the first visit.

• The host is stored rather than the row cleared on save. A stale row (restored backup, direct edit, any writer that isn't the PUT) describes a name that no longer matches and reads as not-reached — and there's no interleaving where a handshake landing mid-save is wiped by the save that provoked it.
• Only Caddy's own ask latches. The same route is reachable from the public internet (Caddy proxies to the frontend, nginx forwards /api/) and the domain is published in every webhook URL, so an unguarded latch was forgeable with one curl. Fails closed: an unrecognised caller is still answered, it just records nothing.
• Step completion is unchanged — still posture-based — so no established install re-opens the overlay. The tick is a claim about the connection; the step is a claim about the operator's part.

Two live defects found while tracing

• A domain with any non-ASCII character could never obtain a certificate. SNI is ASCII so Caddy asks about the A-label, while the operator saves the name as they read it; the gate compared them without IDNA and refused its own configured name — silently, forever, on every visitor's page load, with the UI reporting the domain as set. Both sides now go through the existing canonical_host policy leaf.
• The settings PUT accepted any string for this key. htp://typo.com stored cleanly and the back-fill then re-pointed every live Telegram and WhatsApp binding at an address that answers nothing. Refused now, before the write, with a named error. Plain http:// stays legal — the managed fleet advertises exactly that behind a tunnel — and clearing stays legal.

Copy

Benefit and prerequisite are readable without opening the disclosure. The sentence claiming the proxy "picks up the name" is gone — it never did. The webhook side effect is stated in Settings, where the setting is owned, and left out of first-run where nobody has channels yet. Settings → General takes the same two lines from the same module, so the explanation can't depend on which surface you arrived through. The https? regex that let http:// complete the step is now https-only.

One stance change worth flagging: the card called the tunnel "optional, and dismissing it here is a fine answer", which contradicts #2692's "keep a real instance off the open internet". It now reads optional-but-recommended and names what you accept by skipping. Skippable stays skippable — that's the overlay's Skip control and required: false, not the copy's job.

#2692 — the guide

docs/user-docs/guides/deploying/hardening.md: bare IP → domain → tunnel, or a private network instead, each stage with a verification step. It does not restate the Cloudflare dashboard steps (public-access.md owns those) and covers the marketplace-specific parts that page doesn't: TUNNEL_TOKEN plus start.sh --hosted rather than a compose profile, the ufw --force reset on re-provision, and the fact that over a tailnet you reach Caddy on 80/443 but not container ports. Tailscale is framed honestly — operator access, no Trinity integration, and not a substitute for the tunnel, since a VPN breaks every inbound integration (the recorded 2026-09-01 decision). Linked from the deploy index, DEPLOYMENT.md → Security Recommendations, and the marketplace listing.

Docs-site note: I initially reported the deployment pages as unpublished. Wrong — they publish at /getting-started/deploying/…, and the docs site remaps that folder. The real defect is that they're missing from the site's page registry and so are invisible to site search; filed as abilityai/trinity-docs#1, along with two stale sync-skill routing tables that would have skipped this new page.

Tests

• Backend: new test_2691_public_url_reachability.py — 14 tests: the A-label round trip, that canonicalisation doesn't widen the allowlist, the forgery guard (a forwarded request is answered but records nothing), write-once-per-process, never-raises, the host-scoped read including the stale-stamp and cleared-setting cases, and the save guard driven through the real handler rather than asserted against source text.
• Updated two existing exec-slice helpers that my change outgrew, and tightened one that used a fixed byte window reaching past the function it asserts on.
• Frontend: hardeningGuide.spec.js 36 tests, full suite 2717 passing. One failure (portalAgentsAtCentre, a date-locale assertion) fails identically on the untouched branch — verified by stashing.
• check:tokens, both ratchets, and vite build clean.

Test plan

• pytest tests/unit/test_2691_public_url_reachability.py tests/unit/test_2380_*.py
• On a droplet: save a domain before its DNS exists → Settings reads saved, waiting; add the record, load the site → tick.
• curl https://<ip>/api/public/tls-allowed?domain=<saved> → 200, and the tick does not appear.

Not fixed here

Clearing the Public URL leaves the gate authorising a host baked into PUBLIC_CHAT_URL in the environment, so Settings reads "not configured" while certificates are still issued. Pre-existing; the guide now says so.

Fixes #2691
Fixes #2692

🤖 Generated with Claude Code

https://claude.ai/code/session_01CtMP6kk2GP9oKaEzGwA8H3

[vybe]

merge-train note — #2715 landed on dev as squash cf74f5001 and its branch is deleted, so this PR was retargeted to dev. It now reads CONFLICTING because the branch still carries #2715's original commits alongside the squash. A rebase onto dev (dropping the already-landed commits) should leave only this PR's own delta; it is then a candidate for the next train.

[vybe]

merge-train 2026-09-14: not on this train — two findings need your intent, not a mechanical push. Lane C: /validate-pr READY, /cso --diff READY (one LOW advisory), /review NEEDS-FIX. Both blocking findings were verified against the PR ref by hand. Rides the next train once fixed.

1. The Settings → General tick regresses on every non-Caddy install — src/frontend/src/views/Settings.vue:327-346. The tick now renders only when sessionsStore.publicUrlReached is set, and the only writer of that latch is the provisioned Caddyfile's on-demand-TLS ask (start.sh:348). The first-run step is correctly scoped (firstRunSteps.js requires marketplaceInstall); this block is not gated on install source at all. So a plain docker-compose.prod.yml install behind its own proxy, a Cloudflare Tunnel install (ingress → frontend, Caddy never in the path), and the tailnet-HTTP posture the hardening guide itself calls "finished" all drop from today's unconditional tick (dev Settings.vue:323) to "saved, waiting for the first visit to confirm it resolves here" — permanently, and the sentence is false there. Gate the waiting branch on the provisioned shape (hardeningGuideEligible / install source) and keep the pre-PR tick for everyone else, or make the copy honest ("Trinity cannot confirm reachability from here"). Which one is your design call.

2. provision_caddyfile overwrites the live Caddyfile before validating it — scripts/deploy/start.sh:345 (cat > /etc/caddy/Caddyfile) then :387 (caddy validate). On a failed validate the running process keeps its in-memory config, but the on-disk file is now invalid with systemctl enable caddy in force, so the next reboot or restart takes the site down — the exact outcome the message ("leaving the running config alone"), hardening.md, and .env.example all claim is prevented. Reachable: provision_private_cidrs only charset-filters, so 1.2.3.4/99 passes to remote_ip and fails at validate. Render to Caddyfile.new, validate that path, mv -f on success; the ordering test only asserts source text, so add one that proves the previous file survives.

Riders for the same push (small):

• src/backend/routers/public.py:148-151 — _is_caddy_ask trusts a caller-supplied Host; any agent container (curl -H 'Host: 127.0.0.1' http://backend:8000/api/public/tls-allowed?domain=<saved>) latches the tick. Advisory only (no cert/auth/data impact), but the docstring says "fails closed". Also require request.client.host ∈ loopback, or word it "unforgeable from the public front door".
• tests/unit/test_2691_public_url_reachability.py:331 — ss.db.get_setting_value = lambda … on the shared db instance is not undone by the importlib.reload(ss) at :348 (reload rebinds the module name to the same instance); every later db.get_setting_value in the process returns "". Latent cross-test flake — use monkeypatch.setattr(ss.db, …) and drop the reload.
• src/backend/services/settings_service.py:91-93 — comment says the stamp is "cleared when the saved host changes"; nothing clears it (by design — PROV-016, the PUT handler, and the latch docstring all say compare-on-read). One-line fix so nobody "restores" the missing clear.

The IDNA compare fix, the PUT validation, the source-address remote_ip block, and the guide itself are sound — 204 tests green on the PR tree, no SSRF surface (the "probe" is passive), no secrets or internal hosts in the docs.

[vybe]

merge-train: not on today's train — rides the next one once these two are answered.

Both blockers from the 2026-09-14 review are unchanged on the branch; nothing has been pushed since 15:25Z that day.

1. The tick sentence is false on most installs. Settings.vue:327 gates the tick on publicUrlCurrent && sessionsStore.publicUrlReached, and the only writer of that latch is _latch_public_url_reached, reached solely from Caddy's ask in scripts/deploy/start.sh:349. The block is not gated on install source, so every compose, tunnel, or self-proxied install reads "saved, waiting for the first visit to confirm it resolves here" forever, about a domain that is working fine. This is the one that needs your intent rather than a mechanical fix: either gate the block on hardeningGuideEligible, or reword it so it is true for an install that will never receive a Caddy ask.

2. An invalid Caddyfile is left on disk. start.sh:345 writes /etc/caddy/Caddyfile and then validates it; on failure it returns 1 with the broken file in place and systemctl enable caddy in force, so the next reboot takes the site down — the opposite of the message the user just read about leaving the running config alone. It is reachable, because provision_private_cidrs only charset-filters and 1.2.3.4/99 passes. Render to .new, validate that, then mv -f.

Riders, cheap to fold into the same push: _is_caddy_ask trusts a caller-supplied Host, so an agent container can curl -H 'Host: 127.0.0.1' …/tls-allowed?domain=<saved> and latch it — not reachable from the internet since nginx adds XFF, but the docstring claims it "fails closed", which is stronger than what the code does. Note the obvious fix of checking request.client.host against loopback is probably wrong: Caddy dials the published port, so the backend sees the bridge gateway. The honest correction may be the wording. Also settings_service.py:93 says the latch is "cleared when the saved host changes" and nothing clears it (compare-on-read by design), and test_2691:331 assigns onto the shared db instance while reloading the module, which is a latent cross-test leak that passes in isolation today.

The rest is sound and validated: no new disclosure on the unauthenticated surface, route ordering fine, the IDNA canonicalisation and PUT validation are right, and the hardening guide uses placeholder domains throughout.

[vybe]

merge-train: a commit was pushed to this branch (3f3ca5a5e) — [validated for the 2026-09-15 train]. Nothing else was touched.

What and why

Lane C came back clean on the security surface — /cso --diff found no SSRF (zero new outbound requests in the diff), no new unauthenticated route, no disclosure, placeholders only, no credential-shaped setting, no require_role("admin"). Six security-relevant behaviours are mutation-proven.

The one gap was coverage, not correctness. The tick's client half is correctly wired end to end, but the store action that parses it had no assertion of any kind: deleting both stores/sessions.js:164 and :195 left the full 2930-test frontend suite green, with publicUrlReached permanently false, the https-domain-reached copy dead, and the tick this PR exists to earn never appearing.

The commit extends the existing describe('the store fails closed') harness at hardeningGuide.spec.js:455-499, which already pins three sibling flags on this exact code path, with the fourth field.

One addition beyond the obvious, and why it was needed

The four in-pattern assertions alone did not fully bite. Deleting the catch-path reset (:195) still left the suite green — every test in that block starts from a fresh store where the closed value equals the state default, so it cannot tell a reset from an initial value. So there is one extra test that earns the tick and then fails a forced re-read. That is the real path, not a contrivance: FirstRunOverlay.vue:378 and all three Settings.vue save handlers re-read with force, which is precisely when a just-saved domain's tick resolves — and therefore when a failing backend must revoke it.

Verified: dropping the success assignment → 2 red; dropping the catch reset → 1 red; baseline 130 files / 2931 tests pass.

Deliberately NOT done

FirstRunOverlay.vue:217's binding is left uncovered. vitest.config.js is environment: 'node' with no DOM, so driving it needs a mount harness — your call, not a mechanical fix. A toContain pin over the SFC text would only restate the source, which is the shape that got another PR ejected from this same train.

Two follow-ups for you (no action needed to merge)

1. The whole-internet refusal is a two-literal denylist, not a prefix-length property. Running the real function from scripts/deploy/start.sh:283-285: 0.0.0.0/0 → refused ✅, ::/0 → refused ✅, but 1.2.3.4/0, 0.0.0.0/1 128.0.0.0/1, 0/0, ::0/0 and 0.0.0.0/00 all pass. Each is rendered into @private remote_ip. Operator-supplied .env on their own box, so a foot-gun rather than an attack — but PROV-017 and .env.example both present the refusal as a property, and this is the repo's own feat: machine identities for admin/ops APIs — service credentials that survive enforced 2FA #2323 lesson ("an allowlist, not two named enemies") recurring in a new subsystem. Refusing prefix length 0 outright, or allowlisting RFC1918/RFC6598/fc00::/7, would make the docs true.
2. The body under-describes the diff for a lane-C review. It frames docs: step-by-step hardening for a marketplace install — Cloudflare Tunnel and Tailscale VPN #2692 as the guide, but the diff also ships PRIVATE_NETWORK_CIDRS, a --caddy-only provisioning phase and a new plain-HTTP serving path — 157 lines of security-relevant shell. Documented correctly in requirements/infrastructure.md (PROV-017), but a reviewer reading only the body would not know to look.

On the two earlier train rejections

Both are resolved on the current head, and the second one was written against a stale fetch: b3e54489b (09:33Z) addressed all six findings 7 minutes before the 09:40Z rejection was posted. All six re-verified individually. Apologies for the round-trip — the train will re-fetch before writing an ejection from now on.

[vybe]

merge-train: validated on #2820 (batch of 6, full suite green). Lane C — /validate-pr + /review + /cso --diff. Security surface clean: no SSRF (zero new outbound requests), no new unauthenticated route, no disclosure, verified state unforgeable from the public internet (two independent forwarding-header layers), placeholders only, no credential-shaped setting, no require_role("admin"). Six security behaviours mutation-proven. All six prior merge-train findings confirmed resolved by b3e5448. One mechanical commit pushed to this branch (3f3ca5a) covering the earned-tick store seam; two follow-ups (the prefix-length denylist, the under-described body) noted in the comment above.