Skip to content

feat(web): relay --cert/--key for real TLS on one port - #3952

Open
paul-nechifor wants to merge 1 commit into
paul/feat/t12b-robot-pickerfrom
paul/feat/t12c-relay-tls
Open

feat(web): relay --cert/--key for real TLS on one port#3952
paul-nechifor wants to merge 1 commit into
paul/feat/t12b-robot-pickerfrom
paul/feat/t12c-relay-tls

Conversation

@paul-nechifor

@paul-nechifor paul-nechifor commented Sep 5, 2026

Copy link
Copy Markdown
Contributor
  • Adds --cert and --key options to run the relay with a supplied TLS certificate. This enables HTTPS access to the cockpit from other machines.
  • Serves HTTPS and robot data connections on the same port number when a certificate is supplied, simplifying network setup.
  • Updates browser and robot clients to verify the relay's supplied certificate. Adds --relay-ca so robots can trust certificates issued by a private certificate authority.

@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds operator-provided TLS certificates to the web relay and updates clients to verify those certificates. The TLS discovery response can advertise a localhost WebTransport address that does not reach the default IPv4-only QUIC listener.

Confidence Score: 4/5

Not safe to merge until TLS relay discovery consistently advertises an address that reaches the configured QUIC listener.

A reproduced relay discovery failure prevents the cockpit from connecting when localhost is selected but resolves to IPv6 before the relay's IPv4-only listener. The earlier listener-cleanup thread was manually resolved by paul-nechifor without explanation.

Files Needing Attention: web/relay/server.ts

T-Rex T-Rex Logs

What T-Rex did

  • Produced proofs for two P1 findings and linked them to the relevant review comments.
  • Executed the contract-validation proof with a localhost run and an IPv4 run; localhost showed HTTP 200 OK with the internal URL redacted, IPv6 direct connection was refused and WebTransport timed out, while the IPv4 run returned HTTP 200 OK and WebTransport ready; no repository files were changed and the only authored file was the reproduced TypeScript script artifact.
  • Prepared artifacts for the contract-validation proof, including the reproduction script and two log captures.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
web/relay/server.ts TLS relay discovery returns the request origin, which can advertise an unreachable IPv6 localhost address for the default IPv4-only listener.

Comments Outside Diff (1)

  1. General comment

    P1 TLS discovery advertises an IPv6-capable hostname for an IPv4-only default relay

    • Bug
      • With startRelay({ port: 0, cert, key }), fetching https://localhost:<port>/api/info returned 200 OK and advertised wtUrl: https://localhost:<port>. This environment resolves localhost to both 127.0.0.1 and ::1, while a direct ::1 connection was refused. A WebTransport connection using the advertised URL failed (timed out); the IPv4 comparison advertised 127.0.0.1 and connected successfully.
    • Cause
      • In TLS mode, /api/info returns url.origin, preserving the request hostname (localhost), but the default host remains 127.0.0.1, so neither the TCP nor QUIC listener is bound on ::1.
    • Fix
      • Advertise an IPv4-reachable origin for the default IPv4 listener (for example, derive the host from the configured bind host rather than request origin), or bind both TCP and QUIC listeners on IPv6 when advertising localhost.

    T-Rex Ran code and verified through T-Rex

Reviews (2): Last reviewed commit: "feat(web): relay --cert/--key for real T..." | Re-trigger Greptile

Comment thread web/relay/server.ts
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.65625% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/web/relay_bridge/wt_client.py 77.77% 1 Missing and 1 partial ⚠️
dimos/web/relay_bridge/test_wt_client.py 92.85% 1 Missing ⚠️
@@                       Coverage Diff                       @@
##           paul/feat/t12b-robot-picker    #3952      +/-   ##
===============================================================
+ Coverage                        78.01%   78.04%   +0.02%     
===============================================================
  Files                             1413     1413              
  Lines                           133568   133681     +113     
  Branches                         11572    11573       +1     
===============================================================
+ Hits                            104207   104334     +127     
+ Misses                           26079    26068      -11     
+ Partials                          3282     3279       -3     
Components Coverage Δ
Tests 94.51% <99.00%> (+0.01%) ⬆️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.42% <97.65%> (+0.02%) ⬆️
OS-ubuntu-latest 74.92% <97.65%> (+0.03%) ⬆️
Py-3.10 74.91% <97.65%> (+0.02%) ⬆️
Py-3.11 74.91% <97.65%> (+0.02%) ⬆️
Py-3.12 74.91% <97.65%> (+0.02%) ⬆️
Py-3.13 74.91% <97.65%> (+0.02%) ⬆️
Py-3.14 74.91% <97.65%> (+0.02%) ⬆️
Py-3.14t 74.92% <97.65%> (+0.03%) ⬆️
SelfHosted-Large 30.64% <18.75%> (-0.02%) ⬇️
SelfHosted-Linux 35.62% <18.75%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/cli/commands/lifecycle.py 62.06% <ø> (+0.86%) ⬆️
dimos/cli/test_dimos.py 98.86% <100.00%> (+0.04%) ⬆️
dimos/core/global_config.py 88.70% <100.00%> (+0.18%) ⬆️
dimos/web/relay_bridge/_wt_session.py 93.02% <100.00%> (+0.53%) ⬆️
dimos/web/relay_bridge/locate.py 72.09% <100.00%> (ø)
dimos/web/relay_bridge/relay_bridge_module.py 93.83% <100.00%> (+0.03%) ⬆️
dimos/web/relay_bridge/relay_process.py 82.02% <100.00%> (+3.23%) ⬆️
dimos/web/relay_bridge/test_relay_bridge_e2e.py 95.62% <100.00%> (+0.39%) ⬆️
dimos/web/relay_bridge/test_relay_bridge_module.py 99.10% <100.00%> (+0.03%) ⬆️
dimos/web/relay_bridge/test_relay_process.py 100.00% <100.00%> (ø)
... and 2 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

With --cert/--key the relay terminates TLS itself: HTTPS on --port, QUIC on
the same port, /api/info advertises the origin the client dialed and no
certHash (the ready line omits it too). Without them nothing changes. The
SDK pins a hash only when one is advertised. The bridge verifies normally:
RelayClient.connect defaults the port to 443, GlobalConfig.relay_ca
(--relay-ca) is a PEM CA bundle used for both the /api/info fetch and
aioquic, and a TLS alert during the handshake is logged because aioquic
surfaces it as a bare ConnectionError. RelayProcess takes cert/key paths,
which also widen the child's --allow-read.

Verified with the deno relay tests (h2 discovery and a hash-pinned hello on
the shared port), vitest, pytest including a self-signed e2e, the browser
e2e, and a manual private-CA relay: the bridge and Firefox (with
network.http.http3.disable_when_third_party_roots_found=false) verify it;
Chrome's QUIC stack rejects user-added roots (QUIC_CERT_ROOT_NOT_KNOWN), so
Chrome needs a publicly trusted certificate.
@leshy
leshy force-pushed the paul/feat/t12c-relay-tls branch from f0d7460 to aeffeb6 Compare September 6, 2026 07:59
@paul-nechifor
paul-nechifor marked this pull request as ready for review September 8, 2026 01:05
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
Comment thread web/relay/server.ts
Comment on lines +283 to +285
const info = tls
? { wtUrl: url.origin, v: PROTOCOL_VERSION }
: { wtUrl, certHash: cert.certHashB64, v: PROTOCOL_VERSION };

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.

P1 TLS discovery selects localhost

If localhost resolves to both IPv4 and IPv6, requesting the TLS relay at https://localhost:<port>/api/info returns that localhost origin as wtUrl, even though the default HTTP and QUIC listeners bind only to 127.0.0.1. The browser can then try ::1, where no QUIC listener exists, so the cockpit's advertised WebTransport connection times out. Advertise the configured reachable host or bind both listeners on IPv6 before returning a localhost URL.

Knowledge Base Used: Web cockpit and relay transport

T-Rex Ran code and verified through T-Rex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants