Skip to content

feat(hosting): install provenance + a first-run hardening guide gated on it (#2380) - #2431

Merged
vybe merged 2 commits into
devfrom
feature/2380-install-provenance-hardening
Sep 2, 2026
Merged

vybe merged 2 commits into
devfrom
feature/2380-install-provenance-hardening

Conversation

@obasilakis

Copy link
Copy Markdown
Contributor

Summary

Records how an instance was installed, and shows a first-run HTTPS/VPN hardening guide on marketplace installs and nowhere else.

A marketplace droplet is the one install where Trinity knows at boot that it is on a public IPv4 with no domain, with zero network configuration, and an operator who has read no deployment docs.

The obvious predicate is unusable, and that is the whole design. Measured across all 16 managed instances: every one serves plain HTTP with no DOMAIN, no HTTPS_ENABLED, on a 100.x Tailscale CGNAT address — structurally identical to an unhardened droplet, and already correct, because HTTP over a WireGuard tunnel is encrypted transport (HOST-010). A "no TLS configured → warn" rule fires permanently on every paying client. No environmental signal separates the two cases. The install channel does, and it is knowable only at provisioning time.

Three properties that are security, not hygiene

An unrecognised marker records nothing — not even unknown, which would combine with write-once to freeze a typo permanently. An absent row already reads unknown and lets a corrected marker land next boot.

Honest state by construction

Nothing probes a socket or reads a certificate — TLS terminates outside the backend, so no in-process check can. install_tls_posture is pure string parsing of the URL the instance advertises, resolved through the canonical get_public_chat_url() so it cannot disagree with public links and webhook registration about what this instance is called.

The card says "advertises", never "secure"; https-ip reads as working-but-upgradeable rather than broken (LE IP certs went GA 2026-01-15, DO's own 1-Click rules ship Caddy with them); and it states plainly that Trinity issues no certificate itself — nothing in the tree reads public_chat_url and reconfigures a proxy.

Review found four defects in the first cut, all fixed here

  1. The https-ip copy asserted "a real, browser-trusted Let's Encrypt certificate … works today" on the evidence that a string starts with https:// — a direct AC violation, and the spec required the offending words. Copy rewritten; the honesty guard widened from grepping secure|verified|validated to rejecting connection-property claims generally, with a regression test that feeds it the old sentence and requires ≥4 hits.
  2. The domain path promised "a normal 90-day certificate then replaces the short-lived IP one". Verified false — public_chat_url is a display/webhook-base setting. Now attributes the effect to whatever terminates TLS.
  3. The card rendered for non-admins, whose only action dead-ends (general is adminOnly, so the button lands them on the default tab) and whose copy discloses the box's network posture. Admin-gated, matching AdminEmailNudge.vue.
  4. It did not retire in-session: savePublicUrl never force-refreshed the flags, so the card survived the action it asked for until a hard reload.

Also: posture resolution diverged from the canonical URL resolver; write-once rested on a check-then-act; config imports sat outside the fail-safe guards; and two doc claims of mine overstated (the parity test is prod↔hosted only, and retirement is server state, not verified fact — an admin can type any domain and suppress it, which is accepted and now written down rather than implied otherwise).

Scope note

Nothing in the OSS tree writes the marker — #2281's Packer snapshot does. Until it lands, provenance reads unknown everywhere and the guide renders nowhere. That is PROV-004's contract working, and it is what makes shipping this half first safe.

Test plan

  • pytest tests/unit/test_2380_install_provenance.py — 118 passed. Mutation-tested: removing write-once, recording unknown on a bogus marker, reintroducing the env fallback, and dropping either router guard are each caught.
  • pytest tests/unit/test_2217_canary_status.py test_2280_hosted_compose_parity.py — 147 passed with the above
  • Broader sweep (-k "settings or database or seed or version or ent435 …") — 754 passed; the single failure (test_736_a2a_outbound_properties) reproduces identically on clean dev and is environmental (local 3.11 vs the 3.13 image pin)
  • npm run test:unit — 1528 passed; npm run build and npm run check:tokens clean; new component contributes zero raw colors
  • Manual: set TRINITY_INSTALL_SOURCE=do-marketplace, boot, confirm the row records once, the card renders for an admin only, and a second boot with a different marker leaves it unchanged

Fixes #2380

🤖 Generated with Claude Code

…only where it belongs (#2380)

A marketplace droplet is the one install where Trinity knows, at boot, that it
is on a public IPv4 with no domain, with zero network configuration, and an
operator who has read no deployment docs. It is therefore the one install that
should be prompted to add a real name or a VPN. This records how an instance was
installed and gates that prompt on it.

The obvious predicate is unusable, which is the whole design. Measured across
all 16 managed instances: every one serves plain HTTP with no DOMAIN, no
HTTPS_ENABLED, on a 100.x Tailscale CGNAT address — structurally identical to an
unhardened droplet, and already correct, because HTTP over a WireGuard tunnel is
encrypted transport (HOST-010). A "no TLS configured, so warn" rule fires
permanently on every paying client. No environmental signal separates the two
cases; the install channel does, and it is knowable only at provisioning time.

TRINITY_INSTALL_SOURCE (.env) is read once at boot and recorded into
system_settings by database._record_install_source{,_engine}, on both backends.
An env var rather than /etc/trinity/*, which the issue also offered: config.py
reads zero files today and a marker file needs a read-only bind mount added to
all three compose files — the packaging class this codebase has shipped
repeatedly (#1039, #1056, #1707), where the value never reaches the container and
the feature is silently inert forever. A boot recorder rather than a migration,
for #2381's reason: a migration answers once and records itself, so it can never
reach an instance provisioned before it, nor one whose marker was corrected
afterwards.

Three properties are security rather than hygiene:

- Write-once. Provenance is a fact about an installation *event*. If a later
  .env edit could rewrite it, it would answer "what does this box currently
  claim" instead, and the marketplace gate would be self-assertable by anyone who
  can edit a file. Enforced at the PRIMARY KEY on both arms (INSERT OR IGNORE /
  ON CONFLICT DO NOTHING) rather than by the SELECT that precedes it, which is a
  separate statement and therefore a check-then-act.
- No env fallback on read. get_install_source reads the row only. Without this,
  write-once is decorative — an unrecorded install could be talked into a
  marketplace verdict just by exporting the variable.
- Blocked on PUT *and* DELETE, and refused at the db sink as well. The DELETE
  block is not symmetry: because the recorder is write-once, a delete is
  precisely the move that unlocks a rewrite. The sink guard follows
  db/settings.py's own stated rule — boundary AND sink, because the generic
  catch-all can write any key, which is how the same door was found open by
  #506, #1609, ent#12, #1644, ent#14 and ent#346 in turn.

An unrecognised marker records nothing — not the value, and not `unknown`
either, which would combine with write-once to freeze a typo permanently. An
absent row already reads `unknown`, so leaving it absent costs nothing and lets a
corrected marker land on the next boot. Absent, empty, unrecognised and
unreadable all resolve to `unknown`, never toward a marketplace value: the
failure direction is always to hide the guide.

Surfaced on GET /api/settings/feature-flags (install_source, the server-resolved
marketplace_install gate, install_tls_posture) — no new endpoint, per the AC.
marketplace_install is resolved server-side so the browser holds no second copy
of which channels count. /api/version carries install_source too, threaded into
_build_version_payload as a parameter because that function is exec-sliced by its
own tests and must stay stdlib-only (#1443's constraint on `edition`).

Honest state by construction. Nothing probes a socket or reads a certificate —
TLS terminates outside the backend, so no in-process check can — and the posture
is derived by pure string parsing of the URL the instance *advertises*, resolved
through the canonical get_public_chat_url() so it cannot disagree with public
links and the webhook registrations about what this instance is called. The card
says "advertises", never "secure", and https-ip reads as working-but-upgradeable
rather than broken: LE IP certificates went GA 2026-01-15 and DigitalOcean's own
1-Click rules ship Caddy with them, so a droplet can boot on genuinely trusted
HTTPS — just on a ~6-day renewal at an unmemorable address. The card is also
explicit that Trinity issues no certificate itself; nothing in the tree reads
public_chat_url and reconfigures a proxy.

The card is admin-gated (its only remediation is an adminOnly settings tab, and
its copy discloses the box's network posture to every user of it), renders only
after the flags resolve, and retires when a domain is configured — with
Settings.vue force-refreshing the flags on save so it clears in-session rather
than on the next hard reload.

Nothing in the OSS tree writes the marker; #2281's Packer snapshot does. Until
then provenance reads `unknown` everywhere and the guide renders nowhere, which
is the contract working and is what makes shipping this half first safe.

test_2217_canary_status.py's hand-rolled settings_service stub gains the three
new reads it now has to answer.

Fixes #2380
Comment thread src/frontend/tests/unit/hardeningGuide.spec.js Dismissed
Comment thread src/frontend/tests/unit/hardeningGuide.spec.js Dismissed
@vybe

vybe commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Unblock status, @obasilakis:

CodeQL — both alerts were js/incomplete-multi-character-sanitization in src/frontend/tests/unit/hardeningGuide.spec.js (test-only). Dismissed as used in tests; the check should go green on the next analysis of this PR.

regression diff — this one is real and needs a fix. 4 new failures under HEAD that don't fail under BASE in any seed:

  • test_187_jwt_revocation::test_minted_token_carries_jti
  • test_voice_auth.TestVoiceWebSocketAuth::test_admin_bypasses_ownership
  • test_voice_auth.TestVoiceWebSocketAuth::test_other_user_rejected_4003
  • test_voice_auth.TestVoiceWebSocketAuth::test_owner_passes_auth_gate

The failure counts differ per head seed (6/3/2 vs base's 2/2/2), so this looks order-dependent — likely one of the PR's new tests leaking state (settings/env/module) into the JWT + voice-auth suites rather than a straight assertion break. Worth running the failing tests locally right after your new test modules to reproduce.

…config module

`TestMarkerNormalisation` called `importlib.reload(config)` to exercise the real
`.strip().lower()` at import. Reload mutates the shared module object in place,
and `config.py` mints a random `SECRET_KEY` at import whenever the env var is
unset — which it is under the unit suite. Modules that bound
`from config import SECRET_KEY` at their own import kept the old value while
anything reading `config.SECRET_KEY` per request got the new one, so a JWT signed
on one side failed verification on the other. Order-dependent, hence the varying
failure counts across seeds in the regression diff:

    test_187_jwt_revocation::test_minted_token_carries_jti
    test_voice_auth::TestVoiceWebSocketAuth::test_owner_passes_auth_gate
    test_voice_auth::TestVoiceWebSocketAuth::test_other_user_rejected_4003
    test_voice_auth::TestVoiceWebSocketAuth::test_admin_bypasses_ownership

This is the #1895 divergence, and the conftest hook written for it structurally
cannot catch this shape: it restores `sys.modules["config"]` to the same object a
reload has already mutated.

The intent behind the reload was right — the `marker` fixture rebinds
`cfg.TRINITY_INSTALL_SOURCE` directly and so proves nothing about the real boot
path — so it is kept, and only the mechanism changes: `config.py` is executed
under a throwaway module name that is never registered in `sys.modules`. That
runs the identical module-level statement with none of the reach. Pinning
`SECRET_KEY` across a reload, or snapshotting and restoring the module dict,
would both have worked, but each leaves a cleanup step that can be forgotten and
would need revisiting the day config grows another import-time-derived value;
isolation has nothing to restore.

Verified: the four tests pass when run directly after the new module, and a full
unit run with the new module forced first has a failure set identical to the
branch parent's (23 pre-existing IPv6-mapped-address failures local to this
machine, unchanged on both sides).
@obasilakis

Copy link
Copy Markdown
Contributor Author

Fixed in 412256d.

Root cause. TestMarkerNormalisation called importlib.reload(config). Reload mutates the shared module object in place, and config.py mints a random SECRET_KEY at import whenever the env var is unset — which it is under the unit suite. Modules that bound from config import SECRET_KEY at their own import kept the old value while anything reading config.SECRET_KEY per request got the new one, so a JWT signed on one side failed verification on the other. That is exactly the #1895 divergence, and the pytest_collectstart hook written for it structurally cannot catch this shape: it restores sys.modules["config"] to the same object a reload has already mutated.

Reproduced deterministically before the fix, which also explains the varying counts:

pytest tests/unit/test_2380_install_provenance.py \
       tests/unit/test_187_jwt_revocation.py \
       tests/unit/test_voice_auth.py -p no:randomly
→ 4 failed  (your four, exactly)

Fix. The intent behind the reload was right — the marker fixture rebinds cfg.TRINITY_INSTALL_SOURCE directly and so proves nothing about the real boot path — so that is kept and only the mechanism changes: config.py is now executed under a throwaway module name never registered in sys.modules, running the identical module-level statement with none of the reach. Pinning SECRET_KEY across the reload, or snapshot/restore of the module dict, would both have worked; each leaves a cleanup step that can be forgotten and would need revisiting the day config grows another import-time-derived value. Isolation has nothing to restore. Same command now: 143 passed.

Verification. Full unit suite with the new module forced to run first (the ordering most likely to expose leakage from it), against the branch parent as baseline:

failures
base 13a3de8c^ 23
head + new module first 23

diff of the two FAILED sets is empty — identical, zero new failures. The 23 are pre-existing IPv6-mapped-address failures local to this machine (test_736_a2a_outbound_edges, test_ent14_registry_url_ssrf, test_mcp_validator::TestCgnatSsrfGuard, …), red on both sides and untouched by this branch.

CodeQL — thanks, and confirmed independently: the replace() calls are only in hardeningGuide.spec.js (comment-stripping so the spec can assert on prose). hardeningGuide.js and HardeningGuide.vue contain no replace() and no sanitization, so there is no production twin of the flagged pattern.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: settings-row-only (no schema change on either track), write-once enforced by PK, both catch-all routes 422 the key after the admin gate, TRINITY_INSTALL_SOURCE wired into all three compose files + .env.example (hosted parity test green), 28/28 checks green incl. six-seed pytest + regression diff + frontend-e2e, CodeQL alerts test-only. Locally: 147 + 525 backend tests and 31 vitest pass on the head. Approving.

@vybe
vybe merged commit 0b1d5bb into dev Sep 2, 2026
29 checks passed
obasilakis added a commit that referenced this pull request Sep 4, 2026
…ket asked for (#2380)

Four first-run surfaces landed on the Dashboard from four separate issues, none
aware of the others: the hardening guide (#2380), the front desk (ent#319), the
activation checklist (ent#238) and — since ent#437 folded the #2381 sign-in-email
ask in with the usage-sharing consent — the finish-setup card. On a fresh
marketplace install every predicate can be true at once, which put roughly 520px
of chrome and four dismiss buttons above the product the operator installed
Trinity for. ent#437 already collapsed what would have been a fifth card into an
existing one; this caps what renders at one.

Three changes, one shape:

1. The second hardening path is a Cloudflare Tunnel, not a VPN. #2380 recorded
   that swap on 2026-09-01 — a VPN reaches the same posture but breaks every
   inbound integration, since Telegram, WhatsApp, VoIP, public agent links,
   x402, inbound A2A and webhook triggers all call us, and only Slack survives
   on Socket Mode. PR #2431 merged a day later carrying the pre-decision copy,
   so AC #4 was never met. The copy now states the prerequisite (the domain has
   to be on Cloudflare), says outright that the tunnel needs the name and is
   therefore the second step rather than a different one, and names the half
   Trinity cannot perform: TUNNEL_TOKEN reaches .env and the tunnel starts
   under a compose profile, from the host. It deliberately carries no button,
   because no button here could finish the job.

2. The card face collapses to one action. Title, badge, one sentence, and
   `Add a domain` — the only step collectable in-app, and the completion
   condition that retires the card. Posture detail, both paths and the
   stacking sentence move behind a native <details>: keyboard-accessible, no
   JS, no state, and not a primitive the catalog covers. A first login should
   be actionable at a glance, not two columns of prose.

3. At most one card renders, ever. The four now sit in a `.onboarding-stack`
   wrapper under a single rule — `> * ~ * { display: none }`. Every card is
   v-if'd, so "the first element child" already means "the highest-priority
   card that wants to speak"; the gate needs no predicate lifted into
   Dashboard.vue, leaves each card's visibility where it lives today (its own
   store, its own localStorage dismissal), and makes dismissing the top card
   reveal the next one for free. DOM order is priority order.

That also settles the spacing defect underneath all of this. Every card owned
`mt-3` and nothing owned the gap below, so the last visible card's border sat
flush against the pane — 0px in Timeline and worst in Grid, where a white card
meets a white full-bleed surface. Each card now owns `mt-3 mb-3`; the wrapper
carries no margin of its own so it collapses to a zero-height div when all four
are silent. With only one card visible the "which element owns the gap"
question does not arise.

Verified in a browser against a live stack, with the four visibility predicates
forced true: exactly one card paints in Timeline, Grid and List, in both themes;
dismissing walks the whole stack in priority order — hardening guide, front
desk, activation checklist, finish setup, then nothing — with a 12px gap under
whichever card is showing and no phantom gap once all four are silent. The
collapsed face carries exactly two buttons (`Add a domain` and dismiss), the
disclosure is closed on load, its summary takes focus and toggles on Enter, and
the rendered copy contains cloudflared / the tunnel profile / TUNNEL_TOKEN / the
host-side step and no occurrence of Tailscale, VPN or WireGuard.

Tests: onboardingStack.spec.js pins the rule, the priority order and both
margins; hardeningGuide.spec.js gains a guard that fails if Tailscale or VPN
returns, plus assertions for the tunnel prerequisite, the host-side step and the
one-action face. Those copy assertions strip the SFC's HTML comments — which
record the VPN the card no longer offers — to a FIXPOINT rather than in one
pass: a single `replace(/<!--[\s\S]*?-->/g, '')` leaves a live `<!--` behind on
nested input, which CodeQL flags as js/incomplete-multi-character-sanitization.
Nothing untrusted reaches it (it reads a checked-in file), but the loop is both
the rule's prescribed fix and the more correct strip. 84 files / 1835 tests
pass; check:tokens clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3FfTmxVyfmLNWtAXxMSQ1
vybe pushed a commit that referenced this pull request Sep 4, 2026
…ket asked for (#2380) (#2531)

Four first-run surfaces landed on the Dashboard from four separate issues, none
aware of the others: the hardening guide (#2380), the front desk (ent#319), the
activation checklist (ent#238) and — since ent#437 folded the #2381 sign-in-email
ask in with the usage-sharing consent — the finish-setup card. On a fresh
marketplace install every predicate can be true at once, which put roughly 520px
of chrome and four dismiss buttons above the product the operator installed
Trinity for. ent#437 already collapsed what would have been a fifth card into an
existing one; this caps what renders at one.

Three changes, one shape:

1. The second hardening path is a Cloudflare Tunnel, not a VPN. #2380 recorded
   that swap on 2026-09-01 — a VPN reaches the same posture but breaks every
   inbound integration, since Telegram, WhatsApp, VoIP, public agent links,
   x402, inbound A2A and webhook triggers all call us, and only Slack survives
   on Socket Mode. PR #2431 merged a day later carrying the pre-decision copy,
   so AC #4 was never met. The copy now states the prerequisite (the domain has
   to be on Cloudflare), says outright that the tunnel needs the name and is
   therefore the second step rather than a different one, and names the half
   Trinity cannot perform: TUNNEL_TOKEN reaches .env and the tunnel starts
   under a compose profile, from the host. It deliberately carries no button,
   because no button here could finish the job.

2. The card face collapses to one action. Title, badge, one sentence, and
   `Add a domain` — the only step collectable in-app, and the completion
   condition that retires the card. Posture detail, both paths and the
   stacking sentence move behind a native <details>: keyboard-accessible, no
   JS, no state, and not a primitive the catalog covers. A first login should
   be actionable at a glance, not two columns of prose.

3. At most one card renders, ever. The four now sit in a `.onboarding-stack`
   wrapper under a single rule — `> * ~ * { display: none }`. Every card is
   v-if'd, so "the first element child" already means "the highest-priority
   card that wants to speak"; the gate needs no predicate lifted into
   Dashboard.vue, leaves each card's visibility where it lives today (its own
   store, its own localStorage dismissal), and makes dismissing the top card
   reveal the next one for free. DOM order is priority order.

That also settles the spacing defect underneath all of this. Every card owned
`mt-3` and nothing owned the gap below, so the last visible card's border sat
flush against the pane — 0px in Timeline and worst in Grid, where a white card
meets a white full-bleed surface. Each card now owns `mt-3 mb-3`; the wrapper
carries no margin of its own so it collapses to a zero-height div when all four
are silent. With only one card visible the "which element owns the gap"
question does not arise.

Verified in a browser against a live stack, with the four visibility predicates
forced true: exactly one card paints in Timeline, Grid and List, in both themes;
dismissing walks the whole stack in priority order — hardening guide, front
desk, activation checklist, finish setup, then nothing — with a 12px gap under
whichever card is showing and no phantom gap once all four are silent. The
collapsed face carries exactly two buttons (`Add a domain` and dismiss), the
disclosure is closed on load, its summary takes focus and toggles on Enter, and
the rendered copy contains cloudflared / the tunnel profile / TUNNEL_TOKEN / the
host-side step and no occurrence of Tailscale, VPN or WireGuard.

Tests: onboardingStack.spec.js pins the rule, the priority order and both
margins; hardeningGuide.spec.js gains a guard that fails if Tailscale or VPN
returns, plus assertions for the tunnel prerequisite, the host-side step and the
one-action face. Those copy assertions strip the SFC's HTML comments — which
record the VPN the card no longer offers — to a FIXPOINT rather than in one
pass: a single `replace(/<!--[\s\S]*?-->/g, '')` leaves a live `<!--` behind on
nested input, which CodeQL flags as js/incomplete-multi-character-sanitization.
Nothing untrusted reaches it (it reads a checked-in file), but the loop is both
the rule's prescribed fix and the more correct strip. 84 files / 1835 tests
pass; check:tokens clean.


Claude-Session: https://claude.ai/code/session_01G3FfTmxVyfmLNWtAXxMSQ1

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants