Skip to content

fix(cloudxr): deliver the OOB control token in the URL fragment - #1318

Open
gareth-morgan-nv wants to merge 8 commits into
mainfrom
gmorgan/oob-hub-token-fragment
Open

gareth-morgan-nv wants to merge 8 commits into
mainfrom
gmorgan/oob-hub-token-fragment

Conversation

@gareth-morgan-nv

@gareth-morgan-nv gareth-morgan-nv commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The headset bookmark carried controlToken in the URL query, so the token was sent in the HTTP request target and was visible to the hosting server and proxy logs. It now travels in the URL fragment, which the browser never sends in a request or Referer.

Builds on #1316, which has merged (it uses that PR's token resolver and bookmark builder).

What changed

  • build_headset_bookmark_url and the startup banner put the token in the fragment: #<route>?controlToken=<urlencoded> (#?controlToken=… when no route is set). The token is omitted when OOB is disabled.
  • New helpers/oobControlToken.ts, called from index.tsx before React mounts (so before App canonicalizes the hash): reads the fragment token (fragment wins over a legacy ?controlToken= query), stores it in sessionStorage keyed by hub serverIP:port, and strips it from the address bar, keeping the route, other params and history state. A newly supplied token replaces a stored one; another endpoint's token is never reused. If sessionStorage is denied the token lasts for the current page only, with a console warning. App reads the token through getOobControlToken.
  • --host-client now refuses a cached bundle.js that predates fragment support (same check as the existing health-protocol marker), instead of silently failing to authenticate.
  • oob_teleop_control.rst documents the fragment bookmark, the remaining exposure and the older-client incompatibility.

Compatibility

A client build older than this change reads only ?controlToken= from the query and will not authenticate against a fragment bookmark. The hosted client is versioned, so ship this client before relying on generated bookmarks with a cached or pinned older build. A hand-built ?controlToken= link still works, but that URL has already been sent in the request.

Not covered

The fragment is still visible to the page's own JavaScript, adb command arguments and anyone who sees the URL.

Validation

  • jest, whole suite (130 passed), including helpers/oobControlToken.test.ts (11) and the real-hub controlChannel.test.ts
  • Playwright OobControlTokenTest.spec.js (3 passed): the real App.tsx (app-mock build) against a real OOBControlHub: fragment token -> authenticated register -> URL cleanup with route kept -> page reload -> authenticated register again, token absent from every page request and Referer; legacy query token; no token is rejected. Disabling captureOobControlToken() makes the first two fail.
  • Production npm run build: bundle.js contains the oobControlToken: marker, and the host preflight accepts it; an older bundle is accepted for ordinary hosted streaming and rejected for OOB, including hub-only mode
  • pytest in tests/python/core/cloudxr: test_oob_teleop_env.py, test_oob_teleop_adb.py, test_oob_teleop_hub.py, test_webclient.py, test_wss_static_client.py (212 passed)
  • SKIP=check-copyright-year pre-commit run --all-files
  • Not covered: host -> adb launch -> CDP browser -> hub end to end (needs the fake-adb tooling, not on main) and a real CloudXR stream
  • Full host -> adb launch -> CDP browser -> hub coverage of the fragment bookmark (including the URL passed through am start) will be added once test(cloudxr): reusable fake-adb mock (FakeAdb/mock_adb) #1147 (the fake-adb test double) merges.
  • Other Playwright specs: 4 failures (ControlPanelPositionTest x3, CloudXRComponentTest x1) reproduce identically on an unmodified main checkout in this environment

Test plan

  • CI green

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Headset connection links now carry control tokens in the URL fragment, and the web client supports both fragment-based links and legacy query-based links.
    • The control token remains available when a headset page is reloaded without a token in the URL.
    • The service exposes the active control token for session use.
  • Documentation

    • Updated setup guidance explains where to find control tokens, how to create headset links, and considerations for sharing token-bearing URLs.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between b0dbb4d and 9d3b091.

📒 Files selected for processing: 12
  • deps/cloudxr/webxr_client/helpers/oobControlToken.test.ts
  • deps/cloudxr/webxr_client/helpers/oobControlToken.ts
  • deps/cloudxr/webxr_client/src/App.tsx
  • deps/cloudxr/webxr_client/src/index.tsx
  • deps/cloudxr/webxr_client/tests/playwright/OobControlTokenTest.spec.js
  • docs/source/references/oob_teleop_control.rst
  • src/python/isaaccapture/cloudxr/oob_teleop_env.py
  • src/python/isaaccapture/cloudxr/service/_service.py
  • src/python/isaaccapture/cloudxr/wss.py
  • tests/python/core/cloudxr/test_launcher.py
  • tests/python/core/cloudxr/test_oob_teleop_env.py
  • tests/python/core/cloudxr/test_service.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

The service resolves and retains the OOB control token, then passes it to the WSS proxy. Headset bookmark URLs place the token in the URL fragment. The web client captures fragment or legacy query tokens before React mounts, removes them from the address bar, and retrieves the token for the OOB control channel. Bundle checks can require fragment-token support. Unit, service, and Playwright tests cover token handling and authentication.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant startApp
  participant captureOobControlToken
  participant sessionStorage
  participant App
  participant OOBControlChannel
  participant OOBControlHub
  Browser->>startApp: open bookmark with token in URL
  startApp->>captureOobControlToken: capture token before React mounts
  captureOobControlToken->>sessionStorage: store token for endpoint
  captureOobControlToken->>Browser: remove token parameters from URL
  App->>OOBControlChannel: provide token from getOobControlToken
  OOBControlChannel->>OOBControlHub: authenticate headset connection
Loading

Possibly related PRs

  • NVIDIA/IsaacCapture#1316: Adds the OOB hub token-resolution flow that this PR passes through the service and web-client bookmark.
  • NVIDIA/IsaacCapture#1147: Adds test tooling for OOB teleoperation command flows, which relates to the token-flow testing described for this PR.
Merge Risk: ⚪ Minimal · up to 9d3b0

No identified issue blocks the fragment-token change from merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 55.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 11 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: moving the OOB control token from the URL query to the URL fragment.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 11 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacCapture/preview/pr-1318/

Built from commit 9d3b091. It may take a minute for the updated preview to appear.

Base automatically changed from gmorgan/oob-hub-require-token to main October 23, 2026 23:25
gareth-morgan-nv and others added 8 commits October 23, 2026 19:27
The headset bookmark put controlToken in the URL query, so it was sent in
the request target and visible to the hosting server and proxy logs. The
bookmark and startup banner now put it in the fragment
(#<route>?controlToken=...), which the browser never sends.

The web client lifts the token from the fragment (or a legacy query) before
React canonicalizes the hash, stores it in sessionStorage keyed by hub
host:port so reloads keep working, and strips it from the address bar. A
newly supplied token replaces a stored one; one endpoint's token is never
reused for another. When sessionStorage is denied the token is kept for the
current page only.

--host-client now also refuses a cached bundle.js that predates fragment
support instead of silently failing to authenticate.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…tored one

When sessionStorage rejects the write of a newly supplied token, the old
stored value must not shadow the fresh in-memory one.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…or any OOB host-client

Add a Playwright spec that drives the real App (app-mock build) against a
real OOBControlHub: fragment token -> authenticated register -> URL cleanup
-> reload -> authenticated register again, with the token absent from page
requests and Referer; a legacy query token; and rejection without a token.

The --host-client bundle check for fragment support now applies whenever
OOB is set up (including hub-only mode), independent of the health-probe
requirement, so an older cached bundle cannot silently fail to authenticate.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…ght calls

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Embedded owners bypass the CLI banner, the only place the generated token
was printed, so they had no way to authenticate hub requests or build a
headset URL. Add a control_token property returning the value the hub
enforces (None without setup_oob); the token is still never logged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
… hub-only accessor

Hub-only mode delivers no bookmark, so operators need the token: document
that the banner prints it on stdout (service.log under service start) and
that CONTROL_TOKEN avoids the lookup. Run the accessor/hub agreement test
with and without TELEOP_OOB_HUB_ONLY.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…o the manual URL step

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
@gareth-morgan-nv
gareth-morgan-nv force-pushed the gmorgan/oob-hub-token-fragment branch from 1028e67 to 9d3b091 Compare October 23, 2026 23:27
Comment thread tests/python/core/cloudxr/test_service.py

This branch was successfully deployed

1 active deployment
dev — 9d3b091f Deployed Oct 23, 2026 by gareth-morgan-nv via publish-wheel #5996
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants