Skip to content

feat(web): relay --auth-file with robot keys, viewer tokens, and the … - #3953

Open
paul-nechifor wants to merge 1 commit into
mainfrom
paul/feat/t12d-relay-auth
Open

feat(web): relay --auth-file with robot keys, viewer tokens, and the …#3953
paul-nechifor wants to merge 1 commit into
mainfrom
paul/feat/t12d-relay-auth

Conversation

@paul-nechifor

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

Copy link
Copy Markdown
Contributor
  • Adds optional relay authentication through --auth-file. Robots connect with keys tied to their IDs, and viewers connect with access tokens.
  • Adds a cockpit login form for entering a viewer token. The browser remembers it until the user logs out. (This should be changed later.)
  • Allows the relay to accept connections from other machines when TLS and authentication are configured. Access to the relay's statistics API also requires a viewer token.
  • Masks secret values in configuration output and excludes them from saved run and restart metadata.

@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Merge is not safe until authenticated runs can be restarted without losing required credentials.

Findings

  1. P1 Restart loses credentials

Summary

  • Loads validated robot keys and viewer tokens from an authentication file.
  • Requires TLS and authentication for protected non-loopback relay binding.
  • Propagates robot keys and viewer tokens through the cross-language hello protocol.
  • Adds Cockpit token login/logout behavior and protects relay statistics with viewer bearer authentication.
  • Masks configured secrets and removes them from persisted run metadata.
  • Adds unit, integration, and browser coverage for authentication flows.

Reviews (3) · Last reviewed commit: "feat(web): relay --auth-file with robot ..."

for key, value in self.config_overrides.items()
if _config_field_name(key) not in SECRET_CONFIG_FIELDS
}
self.original_argv = _without_secret_options(self.original_argv)

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 Restart loses credentials

This removes configured --relay-key, --dimos-api-key, and --unitree-aes-128-key values from the stored restart command. dimos restart then launches only that sanitized command, so an authenticated relay or hardware/cloud operation starts without its required credentials and fails. Preserve credentials through a secure restart mechanism, or reject restart requests for runs that require them before merging.

Knowledge Base Used: Core runtime lifecycle

Artifacts

Command output from the check

  • Observed output from the final executed validation, including successful pre-restart authentication and failed authentication after the actual restart path.

View artifacts

T-Rex Ran code and verified through T-Rex

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
dimos/e2e_tests/test_relay_auth_browser.py 19.04% 34 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3953      +/-   ##
==========================================
- Coverage   78.53%   78.50%   -0.03%     
==========================================
  Files        1457     1458       +1     
  Lines      137798   138012     +214     
  Branches    11842    11849       +7     
==========================================
+ Hits       108217   108351     +134     
- Misses      26283    26322      +39     
- Partials     3298     3339      +41     
Components Coverage Δ
Tests 94.54% <81.00%> (-0.05%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.79% <84.47%> (+0.01%) ⬆️
OS-ubuntu-latest 75.41% <84.47%> (+0.01%) ⬆️
Py-3.10 75.40% <84.47%> (+0.01%) ⬆️
Py-3.11 75.40% <84.47%> (+0.01%) ⬆️
Py-3.12 75.40% <84.47%> (+0.01%) ⬆️
Py-3.13 75.40% <84.47%> (+0.01%) ⬆️
Py-3.14 75.40% <84.47%> (+0.01%) ⬆️
Py-3.14t 75.40% <84.47%> (+0.01%) ⬆️
SelfHosted-Large 30.77% <23.74%> (-0.03%) ⬇️
SelfHosted-Linux 35.67% <19.63%> (-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/info.py 100.00% <100.00%> (ø)
dimos/cli/test_dimos.py 98.89% <100.00%> (+0.03%) ⬆️
dimos/core/global_config.py 88.97% <100.00%> (-3.77%) ⬇️
dimos/core/run_registry.py 89.23% <100.00%> (+1.84%) ⬆️
dimos/core/test_daemon.py 99.22% <100.00%> (+0.03%) ⬆️
dimos/web/relay_bridge/locate.py 72.09% <ø> (ø)
dimos/web/relay_bridge/module_test_support.py 98.08% <100.00%> (+0.02%) ⬆️
dimos/web/relay_bridge/protocol.py 100.00% <100.00%> (ø)
dimos/web/relay_bridge/relay_bridge_module.py 93.86% <100.00%> (-1.99%) ⬇️
dimos/web/relay_bridge/relay_process.py 82.08% <100.00%> (-1.44%) ⬇️
... and 8 more

... and 4 files 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.

@leshy
leshy force-pushed the paul/feat/t12d-relay-auth branch from c332da6 to 0af7149 Compare September 6, 2026 07:59
@paul-nechifor
paul-nechifor marked this pull request as ready for review September 8, 2026 05:53
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from 0af7149 to b8d9e06 Compare September 8, 2026 23:31
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from b8d9e06 to b473fe6 Compare September 9, 2026 00:28
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from b473fe6 to ad3ba7b Compare September 9, 2026 02:34
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from ad3ba7b to 6b9642a Compare September 9, 2026 22:54
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from 6b9642a to 97a6a7a Compare September 10, 2026 00:47
@paul-nechifor
paul-nechifor marked this pull request as draft September 10, 2026 01:08
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from 97a6a7a to b7f9915 Compare September 10, 2026 20:00
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from b7f9915 to dda5ffb Compare September 11, 2026 05:20
@leshy
leshy force-pushed the paul/feat/t12d-relay-auth branch from dda5ffb to b33169b Compare September 11, 2026 10:55
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from b33169b to 296b454 Compare September 11, 2026 18:14
@paul-nechifor
paul-nechifor marked this pull request as ready for review September 11, 2026 19:15
Base automatically changed from paul/feat/t12c-relay-tls to main September 11, 2026 21:44
…cockpit login

A relay that can be exposed: `--auth-file auth.json` maps robot ids to keys
and viewer names to tokens (static secrets, >= 16 chars, no secret twice,
constant-time compares, never logged). Robots and viewers present them in
the hello (`hello.token`, optional, <= 256 chars, no version bump); the
relay answers `auth_failed`, which is terminal on both clients. `/api/stats`
needs `Authorization: Bearer <viewer token>` and drops its CORS header once
auth is on. A non-loopback host binds with cert + key + auth together (or
`--unsafe-non-loopback` as before) and refuses `--serve-dir` there.

Bridge: `GlobalConfig.relay_key` (`RELAY_KEY`, mirrored on
`RelayBridgeConfig`, external relay only) rides `RelayClient.hello(token=)`;
an over-long token is refused before pydantic can quote it. `dimos
show-config` masks `relay_key` and `dimos_api_key`. `RelayProcess(auth_file=)`
joins the relay's read scope.

SDK: `connect({token})`, `TransportPhase.failed.code`. Cockpit: the token
form on `auth_failed` (stored in localStorage, page reload), "log out" in
the status bar.

Tests: deno auth/session/server (hello cases, stats bearer, mode gate),
fixtures with a token hello, pytest mirror + e2e + CLI masking, vitest
SDK/cockpit, and a Playwright login flow (added to the CI list).
@paul-nechifor
paul-nechifor force-pushed the paul/feat/t12d-relay-auth branch from 296b454 to 77f387a Compare September 11, 2026 21:44
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 11, 2026
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