Skip to content

fix(connection): open the dashboard on the tunnel, not the saved URL - #1484

Open
SebTardif wants to merge 16 commits into
openclaw:mainfrom
SebTardif:fix/f020-dashboard-tunnel
Open

SebTardif wants to merge 16 commits into
openclaw:mainfrom
SebTardif:fix/f020-dashboard-tunnel

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What Problem This Solves

Open dashboard for an SSH gateway built the browser URL from the saved gateway URL and appended the shared token. The node socket is ws://localhost on the tunnel. A cleartext saved URL carried the token to the remote host.

Why

IsStrongCredentialAllowed treated every SSH record as allowed, so the managed-gateway check was skipped.

User Impact

The dashboard opens on localhost at the forwarded port and keeps the saved path, query, and fragment. The token is appended only to that tunnel URL. A down or different tunnel is refused. A browser failure uses the existing retry dialog and does not log the address.

Evidence

Current head: 18bebb666e67e0edbe236f18fe339bfdd3357f72. It is the same tree as e8f4d2c10ff8f7c74aae24225b4f8d3d242e95c6. UI passed on a6f5e2ed. This tip retriggers CI after ConcurrentCreationConvergesOnOneCredential hit a file lock on run 37848102774.

Dashboard launches share http://127.0.0.1:47831 for the life of the tray. An unused link expires after two minutes. The listener stays up. After the socket to the forward connects, ownership is checked again before any dashboard bytes are copied. The upstream scheme follows the saved destination, including HTTPS and WSS. The live tray trace below is parent 60b1cc0e. It was not repeated on this head.

The one-use page no longer sends the browser to the SSH forward. The follow-up address stays on the handoff listener, and that listener proxies to the forward only while ownership still holds. The live tray trace below is parent 60b1cc0ee03d04c2389ce820ce10922b4025e2e0, whose first response still named the tunnel address. That live tray was not repeated on this head.

The one-use page now keeps the gateway and tunnel that issued the link. If the active gateway changes before the browser requests the page, the token is not released.

An SSH dashboard link issued by the tray, the saved-row action, or local MCP is now http://127.0.0.1:<port>/d/<nonce> and does not contain the token. The first request writes the real dashboard address only when the owned listener still matches. A second request is HTTP 404 and does not contain the token. After the browser follows that page, its address bar can still show the tunnel URL.

Current-head tray process 18376. ssh.exe 21220 was its child and listened on 127.0.0.1:45678. winnode --list-tools included app.dashboard.url. The call returned http://127.0.0.1:63604/d/<nonce> with hasTokenQuery false, usesSharedGatewayToken true, credentialSource record.SharedGatewayToken, and no token in that URL. The first request returned HTTP 200 and a location.replace page. The second request to the same URL returned HTTP 404 with an empty body and no token. After that ssh.exe stopped, nothing listened on 45678, and the next call returned Dashboard blocked because the SSH tunnel is not up with no URL. A foreign process then listened on 45678, and the next call returned the same error with no URL. The browser was not opened. Switching the active gateway was not done on this live tray. The first 200 response body contains the tunnel address, so a browser that follows it can still open that port.

The tray, the saved-row action, and local MCP check the owned listener again immediately before the browser launch or before the link is returned. If that recheck fails, the browser is not opened and the link is not returned. A link that was already returned can still be opened later. The live trace below is parent 1d9bbac0e6d18288ecafe6ce4a50552185904778.

Isolated current-head tray, process 18504. The active saved gateway URL was wss://gateway.example/mount/?view=compact#section. Its SSH forward was local port 45678 to the Ubuntu-24.04 gateway port 18789. ssh.exe process 30580 was a child of that tray. winnode --list-tools included app.dashboard.url. winnode --command app.dashboard.url returned:

credentialSource record.SharedGatewayToken
usesSharedGatewayToken True
hasTokenQuery True
scheme https
host localhost
port 45678
path /mount
query view=compact
fragment keys section,token
token length 48
saved host present False

curl.exe to http://127.0.0.1:45678/mount/?view=compact returned HTTP 200. The same path over https returned no status. The tray log line was SSH tunnel started for root at local port 45678 to remote port 18789. Stopping ssh.exe 30580 left nothing listening on 45678. The next app.dashboard.url call returned Dashboard blocked because the SSH tunnel is not up. and no URL.

On head 1d9bbac0e6d18288ecafe6ce4a50552185904778, wss://gateway.example/mount/?view=compact#section with local port 45678 resolves to wss://localhost:45678/mount/?view=compact#section. A plain ws://gateway.example:18789 still resolves to ws://localhost:45678. ./build.ps1 exit 0 on d94a9f770c0f1dbf2d5d757aeeae5b3b0e6e4de0. Shared 4263 passed, 33 skipped, 4296 total. Tray 4001 passed, 0 failed. The saved-row menu and a browser window were not opened on this head.

Maintainer repair

Normal integration of main 42c562f91a42ae0251ac971c44f6ce0f8028e88d preserves the original author commits and branch. Repair 99ea745923d5d6edf113a8d2bf979f5db85615b1 changes only the pinned dashboard credential helper (10 added / 9 removed production lines) and two focused test files.

Paired SSH dashboard HTTP authentication now uses the existing shared-first InteractiveGatewayCredentialResolver.TryResolveRecord, not the device-first WebSocket resolver. Before/after pin checks, existing authorization semantics, owned-listener and endpoint binding, and the final shared-token equality gate remain. Device/bootstrap credentials never enter dashboard URLs. WebSocket precedence is unchanged. The HTTP browser URL builder uses a #token fragment, not a token query.

Required proof pools

  • windows-winui-interactive: local MCP app.dashboard.url on head 1d9bbac0, with the owned SSH listener up and then stopped. Head d94a9f77 rechecks that listener immediately before launch or before returning the link. The saved-row menu and a browser window were not opened.

Validation

Validated source tree: beae2a9b7790c4819df441690c6ba6e489dec1a6, repair head 99ea745923d5d6edf113a8d2bf979f5db85615b1.

Native Windows ARM64, SDK 10.0.400. Isolated tray data, worktree-specific OPENCLAW_REPO_ROOT, E2E flags off. Each test project was built first so --no-restore could not silently no-op.

Command Passed Skipped Failed
.\build.ps1 All projects N/A 0
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore 4096 35 0
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore 3073 0 0
dotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restore 812 5 0
dotnet test .\tests\OpenClaw.WinNode.Cli.Tests\OpenClaw.WinNode.Cli.Tests.csproj --no-restore 127 0 0

TRX result entries confirm these counts. Focused Connection resolver/gate/endpoint/credential tests: 48 passed. New tray/saved-row/MCP wiring guards: red 1 failed / 3 passed before the repair; green 4 passed after it. The behavioral cases use a real isolated persisted DeviceIdentity and production credential resolvers with fabricated tokens, including paired shared-token selection, device/bootstrap exclusion, authorization rejection, and changed-pin/shared-token mismatch controls.

Focused full-source rubber-duck review and parent inspection of the exact repair passed with no blockers. Structured autoreview could not start: python .\.agents\skills\autoreview\scripts\autoreview --mode commit --commit 99ea745923d5d6edf113a8d2bf979f5db85615b1 --engine codex (using the approved Codex 0.156.1 executable) exited 1 before engine invocation. Its bundle scanner rejected plainly synthetic token fixtures and source-field expressions with refusing to include secret-like content in review bundle; clean or redact commit diff before running autoreview. No scanner was weakened, no source-altering review mirror or alternate engine was used, and no clean autoreview result is claimed. After inspecting these false positives and the full repair, the parent authorized publication as a reviewable maintainer update, not merge/ship approval. The outstanding structured-review and live-proof limitations remain visible.

Real behavior proof

  • Behavior or issue addressed: An SSH dashboard link keeps the saved path, query, and fragment, and moves only the host and port onto the owned local forward. The owned-listener check runs again immediately before the browser starts or before the link is returned. When that check fails, the browser stays closed and no link is returned.
  • Real environment tested: Windows 11. The live tray trace is head 1d9bbac0e6d18288ecafe6ce4a50552185904778, process 18504, isolated data directory, Ubuntu-24.04 gateway on port 18789, ssh.exe 30580 parented by the tray. The recheck is head d94a9f770c0f1dbf2d5d757aeeae5b3b0e6e4de0.
  • Exact steps or command run after this patch: Launch the isolated tray. Run winnode --list-tools. Run winnode --command app.dashboard.url --params {}. Request http://127.0.0.1:45678/mount/?view=compact. Stop the tray's ssh.exe. Call app.dashboard.url again.
  • Evidence after fix: terminal output from the current-head tray:
HAS_DASHBOARD True
credentialSource record.SharedGatewayToken
usesSharedGatewayToken True
scheme https
host localhost
port 45678
path /mount
query view=compact
fragment keys section,token
token length 48
saved host present False
HTTP 200
after stop: Dashboard blocked because the SSH tunnel is not up.
  • Observed result after fix: The link used localhost port 45678, path /mount, query view=compact, and fragment fields section and token. The saved host gateway.example was absent. The token length was 48 and it was a fragment field. usesSharedGatewayToken was true. The plain HTTP forward returned 200. After the owned ssh.exe exited, the port was closed and the next call returned the tunnel-down error with no URL. On the new head, a failed ownership recheck returns before the browser delegate runs.
  • What was not tested: The saved-row menu and a browser window were not clicked on this head. Switching the active gateway was not done on the live tray. The first HTTP 200 body contains the tunnel address, so a browser that follows it can still open that port after the response. Bootstrap and changed-pin refusals were not repeated.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 23, 2026
@clawsweeper

clawsweeper Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge.

What this changes

The branch routes SSH dashboard launches through one-use local links and an HTTP and WebSocket proxy that checks tunnel ownership.

Example: Open the dashboard for wss://gateway.example/mount/?view=compact#section through local SSH port 45678

  • Before: The browser link uses the saved gateway address.
  • After: The issued link uses http://127.0.0.1:47831/d/&lt;nonce>, then opens the mounted dashboard through the local proxy.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) Earlier runtime traces support the routing idea, but the unchanged proxy has blocking correctness and security defects.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🧂 unranked krab (1/6) Security review found an item that needs attention.

Product

Kind: Bug fix · Worth it: Yes
User problem: Opening an SSH Gateway dashboard uses the saved remote address instead of the protected tunnel and can expose its shared credential.
Reason: Correcting SSH dashboard routing is valuable, and the recorded reviewer direction confirms the existing HTTP credential contract. That value does not justify the proxy regressions.

Merge readiness

⛔ Blocked before merge - 9 items remain

This PR addresses a real dashboard-routing defect, but all six previously reported proxy defects remain on the unchanged source tree. Current main does not implement the requested repair.

Priority: P1
Reviewed head: 18bebb666e67e0edbe236f18fe339bfdd3357f72

Before merge

  • Add real behavior proof - Authority-chain proof required: the supplied Windows tray/MCP traces exercise earlier redirects, not the current HTTP and WebSocket proxy. Show authenticated current-head dashboard loading and rejection before credential or content I/O for a replaced accepted backend and an older Gateway A tab after opening Gateway B, using browser diagnostics, redacted logs or live output. The inspected screenshots do not exercise this path, and preservation of existing browser preferences is unverified. Redact credentials and private endpoints. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • [P1] Preserve the upstream asset content type (P1) - Every proxied response is marked text/html, including the Gateway's JavaScript modules and stylesheets. Browsers reject the module MIME type, preventing dashboard initialization. Preserve the upstream content type when reconstructing the response. This prior finding remains unchanged.
  • [P1] Keep each browser session bound to its issuing gateway (P1) - Delivering Gateway B's link overwrites _active, while Gateway A's existing tab retains the same proxy endpoint and A credential. When A reconnects, the proxy forwards that credential to B and checks B's ownership callback. Bind subsequent browser traffic to the issuing Gateway rather than the latest delivered handoff. This prior finding remains unchanged.
  • [P1] Verify the connected backend rather than its current listener (P1) - A foreign process can accept the backend connection, release its listener while retaining that socket, and allow the legitimate SSH listener to return before _owned() runs. The check then succeeds although HTTP bytes or authentication frames reach the foreign socket. Verify the accepted connection's process and generation before forwarding. This prior finding remains unchanged.
  • [P1] Preserve the saved TLS identity on both upstream transports (P1) - For wss://gateway.example/mount/ with a certificate valid for gateway.example, endpoint substitution changes the upstream identity to localhost. HTTP accepts every certificate here, while ClientWebSocket uses normal validation and rejects the hostname mismatch. Retain and validate the saved TLS identity consistently on both transports. This prior finding remains unchanged.
  • [P1] Forward the Gateway's browser security headers (P1) - The Gateway supplies CSP, framing protection and other browser safeguards, but this response reconstruction discards them. Opening the dashboard through this proxy therefore removes protections provided by the existing endpoint. Preserve applicable end-to-end security headers. This prior finding remains unchanged.
  • [P2] Preserve existing dashboard preferences across the origin change (P2) - An existing user's dashboard moves from its saved Gateway origin to http://127.0.0.1:47831, and the injected Gateway identity changes too. Control UI preferences reside in origin-scoped storage with Gateway-derived keys, so the new dashboard cannot load the existing profile. Preserve the established identity or provide a compatible transfer. This prior finding remains unchanged.
  • Resolve security concern: Separate Gateways share browser storage authority - Beyond wrong-backend routing, separately administered Gateways now serve executable pages under the same browser origin and gatewayUrl scope. A page served by one Gateway can access the shared origin's browser state, removing the previous separation between Gateway profiles.
  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

  • [P1] [P1] Preserve the upstream asset content type — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:220-224
  • [P1] [P1] Keep each browser session bound to its issuing gateway — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:166-168
  • [P1] [P1] Verify the connected backend rather than its current listener — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:194-195
  • [high] Separate Gateways share browser storage authority — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:181

Tests

  • Missing end-to-end proof: The current proxy lacks authenticated browser and WebSocket loading, replaced-backend and older-tab rejection, and upgrade preference-preservation proof; no base-fail/head-pass browser regression is established.
Agent review details

How this fits together

Windows Companion opens Gateway dashboards from tray actions, saved gateway rows, and local MCP commands. The changed path selects browser credentials and forwards dashboard traffic through the SSH tunnel.

flowchart TD
 A[Tray, saved gateway, or MCP action] --> B[Resolve gateway and shared credential]
 B --> C[Check owned SSH listener]
 C --> D[One-use local link]
 D --> E[Browser dashboard]
 E --> F[HTTP and WebSocket proxy]
 F --> G[SSH forward]
 G --> H[Gateway]
Loading

Technical review

Best possible solution:

Use one guarded forwarding owner that preserves per-Gateway browser identity, existing preferences, validated TLS and accepted-socket ownership through final I/O.

Do we have a high-confidence way to reproduce the issue?

Yes, current source establishes the saved-address routing defect and concrete failures in the introduced proxy. No runtime reproduction was executed during this read-only review.

Is this the best way to solve the issue?

No, the current proxy breaks browser delivery and Gateway isolation; the related guarded-forward implementation offers a narrower direction, subject to its separate port-reuse choice.

Full review comments:

  • [P1] [P1] Preserve the upstream asset content type — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:220-224
    Every proxied response is marked text/html, including the Gateway's JavaScript modules and stylesheets. Browsers reject the module MIME type, preventing dashboard initialization. Preserve the upstream content type when reconstructing the response. This prior finding remains unchanged.
    Confidence: 0.99
  • [P1] [P1] Keep each browser session bound to its issuing gateway — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:166-168
    Delivering Gateway B's link overwrites _active, while Gateway A's existing tab retains the same proxy endpoint and A credential. When A reconnects, the proxy forwards that credential to B and checks B's ownership callback. Bind subsequent browser traffic to the issuing Gateway rather than the latest delivered handoff. This prior finding remains unchanged.
    Confidence: 0.98
  • [P1] [P1] Verify the connected backend rather than its current listener — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:194-195
    A foreign process can accept the backend connection, release its listener while retaining that socket, and allow the legitimate SSH listener to return before _owned() runs. The check then succeeds although HTTP bytes or authentication frames reach the foreign socket. Verify the accepted connection's process and generation before forwarding. This prior finding remains unchanged.
    Confidence: 0.96
  • [P1] [P1] Preserve the saved TLS identity on both upstream transports — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:206-208
    For wss://gateway.example/mount/ with a certificate valid for gateway.example, endpoint substitution changes the upstream identity to localhost. HTTP accepts every certificate here, while ClientWebSocket uses normal validation and rejects the hostname mismatch. Retain and validate the saved TLS identity consistently on both transports. This prior finding remains unchanged.
    Confidence: 0.98
  • [P1] [P1] Forward the Gateway's browser security headers — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:219-224
    The Gateway supplies CSP, framing protection and other browser safeguards, but this response reconstruction discards them. Opening the dashboard through this proxy therefore removes protections provided by the existing endpoint. Preserve applicable end-to-end security headers. This prior finding remains unchanged.
    Confidence: 0.99
  • [P2] [P2] Preserve existing dashboard preferences across the origin change — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:181-186
    An existing user's dashboard moves from its saved Gateway origin to http://127.0.0.1:47831, and the injected Gateway identity changes too. Control UI preferences reside in origin-scoped storage with Gateway-derived keys, so the new dashboard cannot load the existing profile. Preserve the established identity or provide a compatible transfer. This prior finding remains unchanged.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 037c17dcb581.

Provenance checked

Testing

Proof path: in-process harness.

Security

Needs attention: The proxy changes credential isolation and weakens existing transport and browser safeguards; no unrelated supply-chain changes were found.

Evidence

Security concerns:

  • [high] Separate Gateways share browser storage authority — src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:181
    Beyond wrong-backend routing, separately administered Gateways now serve executable pages under the same browser origin and gatewayUrl scope. A page served by one Gateway can access the shared origin's browser state, removing the previous separation between Gateway profiles.
    Confidence: 0.98

What I checked:

  • Current main still builds dashboard links from the saved endpoint: The current-main app.dashboard.url handler uses TryResolveChatCredentials and GatewayDashboardUrlBuilder without the introduced SSH dashboard substitution. Changes between the pinned base and fetched main do not touch the reviewed dashboard paths. (src/OpenClaw.Tray.WinUI/App.CapabilityHandlers.cs:239, 037c17dcb581)
  • Prior findings remain on an identical source tree: The comparison with the previous completed review is empty. The full durable review comment was retrieved, and the six findings were checked against current source. Captured context sourceRevision: fc52e6b8a04da52559db8d3100b8a39c78126b7618d1131d39cba594c42a0463. (src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:166, 18bebb666e67)
  • Proxy routing and response reconstruction: Delivering a link overwrites the global active destination. Later requests use that destination, while HTTP forwarding discards upstream headers and assigns text/html to every response. (src/OpenClaw.Tray.WinUI/Services/DashboardCredentialHandoff.cs:222, 18bebb666e67)
  • Listener inspection does not identify an accepted connection: IsOwnedListenerReadyAsync checks the configured listener, SSH process and lifecycle generation. It accepts no connected socket, so the proxy's post-connect check cannot identify the process that accepted its backend connection. (src/OpenClaw.Connection/SshTunnelService.cs:484, 18bebb666e67)
  • Affirmative dependency signal and asset contract: The branch proxies OpenClaw Control UI and injects its gatewayUrl parameter, directly consuming its browser contract. Gateway source serves JavaScript as application/javascript and stylesheets as text/css. (src/gateway/control-ui-static.ts:30, eb377ac59e6c)
  • Existing browser security safeguards: Control UI responses set framing protection, Content-Security-Policy, X-Content-Type-Options, Referrer-Policy and Permissions-Policy. The introduced proxy does not preserve them. (src/gateway/control-ui.ts:198, eb377ac59e6c)

Review metrics

Metric Value Why it matters
Production and test growth production +977/-57 lines; tests +465/-0 lines The routing repair adds a substantial browser transport path whose compatibility and authorization behavior are not established.

Labels

Label changes:

No label changes.

Label justifications:

  • P1: The change affects SSH dashboard availability and shared Gateway credential handling.
  • merge-risk: 🚨 compatibility: The proxy changes asset delivery, TLS identity and persisted browser preference scope.
  • merge-risk: 🚨 security-boundary: The proxy can forward credentials to a different Gateway or accepted backend and removes existing browser safeguards.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦐 gold shrimp and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (18 earlier review cycles; latest 8 shown)
  • reviewed 2026-10-08T17:04:37.796Z sha 1d9bbac :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-08T17:33:26.540Z sha d94a9f7 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-08T18:21:34.578Z sha e52d75a :: needs real behavior proof before merge. :: [P1] [P1] Bind the deferred callback to the gateway that issued the link
  • reviewed 2026-10-08T19:11:29.570Z sha 60b1cc0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-08T19:46:32.831Z sha 60b1cc0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-08T20:53:48.726Z sha e136209 :: needs real behavior proof before merge. :: [P1] [P1] Retain ownership of the browser origin after handoff | [P1] [P1] Bind forwarding to the connected SSH backend | [P2] [P2] Preserve TLS on the upstream dashboard connection | [P1] [P1] Preserve browser settings across dashboard launches
  • reviewed 2026-10-08T21:25:26.954Z sha e8f4d2c :: needs real behavior proof before merge. :: [P1] [P1] Preserve the upstream asset content type | [P1] [P1] Keep each browser session bound to its issuing gateway | [P1] [P1] Verify the connected backend rather than its current listener | [P1] [P1] Preserve the saved TLS identity on both upstream transports | [P1] [P1] Forward the Gateway's browser security headers | [P2] [P2] Preserve existing dashboard preferences across the origin change
  • reviewed 2026-10-08T21:54:47.596Z sha a6f5e2e :: needs real behavior proof before merge. :: [P1] [P1] Preserve the upstream asset content type | [P1] [P1] Keep each browser session bound to its issuing gateway | [P1] [P1] Verify the connected backend rather than its current listener | [P1] [P1] Preserve the saved TLS identity on both upstream transports | [P1] [P1] Forward the Gateway's browser security headers | [P2] [P2] Preserve existing dashboard preferences across the origin change

Reviewed October 8, 2026, 6:10 PM ET / 22:10 UTC (Revision 19).

Require the SSH server port and a verified listener before a dashboard URL can carry a shared token, and read the token from the same gateway record as the tunnel.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Sep 23, 2026
@karkarl

karkarl commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Global triage: HOLD_FOR_AUTHOR. Take confidence 25%; recommendation confidence 93%; effort moderate; risk high.

Reviewed exact head acb5744a9336. Tunnel/listener binding is good hardening, but the head appends pinned.SharedGatewayToken whenever non-empty, dropping the credential-precedence gate from the earlier commit. A device-token-paired SSH gateway can expose its stored shared token in the browser URL. Restore the source/bootstrap gate, stop hardcoding shared-token credential source in MCP output, and surface pin mismatch or async failures. Add windows-wsl-gateway-e2e, windows-winui-interactive, and winnode/MCP invocation proof for app.dashboard.url.

…ecord

An SSH dashboard was always appending the shared token and labeling that as the MCP source. The URL now follows the pinned record's credential source, and a pin mismatch returns no token URL.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@shanselman

Copy link
Copy Markdown
Collaborator

Thanks, Seb. The new pin checks and truthful credential-source reporting are useful. We independently reviewed the acb5744a to 51c2586a007010f9cd1038060815698dca62372a delta with Opus 5.5 and GPT-5.6 Sol, then checked the source.

One correction to the earlier precedence feedback: operator WebSocket authentication and browser dashboard authentication use different resolvers. Keeping a device token ahead of a shared token for an operator connection does not mean a browser can authenticate with that device token.

The new TryResolvePinnedDashboardCredential calls CredentialResolver.ResolveOperator. For a normal paired SSH record containing both an operator device token and a shared token, it selects the device token. DashboardCredentialGate correctly refuses to put that device token in the browser link, but the result is now a tokenless dashboard link. Direct dashboard/chat paths still use the shared-first HTTP contract. Both reviewers independently identified this paired-SSH automatic-authentication regression.

The small correction is already available in this delta: use InteractiveGatewayCredentialResolver.TryResolveRecord for the pinned browser credential, retaining the existing authorization callback, listener ownership, before/after pin checks, and final shared-token equality gate. Do not put device or bootstrap tokens into the dashboard link, and do not change WebSocket precedence.

Please add a regression that actually invokes the resolver with both a stored device token and a shared token, rather than supplying a preselected source to DashboardCredentialGate. Cover the tray, saved-row, and app.dashboard.url paths against the same owned SSH record, then collect the current-head browser/MCP proof.

This is not a request for a new authentication framework or a general token-link redesign. The separate active-record-switch observation already exists at the prior reviewed head, and no specific closed architecture-ledger responsibility was established as newly violated; neither is being added as a new delta blocker.

@shanselman shanselman added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 23, 2026
shanselman and others added 2 commits September 24, 2026 01:44
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ec217af-d09a-475f-8c7c-1153efc34eae
Resolve the pinned dashboard credential with the existing shared-first HTTP resolver while preserving authorization, pin checks, and the final token gate. Add stored-identity regression coverage and tray, saved-row, and MCP wiring guards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5ec217af-d09a-475f-8c7c-1153efc34eae
@shanselman shanselman removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 24, 2026
…nnel

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>

# Conflicts:
#	src/OpenClaw.Connection/InteractiveGatewayCredentialResolver.cs
#	src/OpenClaw.Tray.WinUI/App.xaml.cs
#	src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs
#	tests/OpenClaw.Tray.Tests/AppRefactorContractTests.cs
…the shared authorizer

The merge left a direct managed-port check on the pinned dashboard path.
HTTP surfaces now ask InteractiveGatewayEndpointAuthorizer, which still
applies that check for a non-native gateway and adds native ownership inspection.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
The UI job aborted after five minutes of host inactivity during markdown and workspace layout tests. Those tests are not part of this dashboard change.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
An SSH dashboard address such as wss://host/mount/?view=compact#section
now opens on localhost at the forwarded port and keeps the path, query,
and fragment. Browser launch goes through GatewayDashboardLauncher, so
a failure shows the existing retry dialog and the token URL is not logged.

The mounted-address resolver case passed. build.ps1 exit 0. Shared
4263 passed, 33 skipped. Tray 4000 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Oct 7, 2026
The listener check that builds a dashboard link can be stale by the time
the browser starts. The tray, the saved-row action, and local MCP check
the owned listener again immediately before launch or before returning
the link. A failed recheck does not open the browser and does not return
the URL.

./build.ps1 passed. Shared 4263 passed, 33 skipped, 4296 total. Tray
4001 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
The ownership recheck still returned a reusable token fragment aimed at
the tunnel port. SSH dashboard opens and local MCP now issue a loopback
page with no credential. That page writes the token only after a fresh
ownership check, and a second request does not.

./build.ps1 passed. Shared 4263 passed, 33 skipped, 4296 total. Tray
4002 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
…d it

The one-use page checked whichever gateway was active and treated a
gateway with no SSH tunnel as ready. Switching away from the SSH
gateway before the browser requested the page could release the first
gateway's token. The check now requires the active record to still be
the issued gateway and its tunnel.

./build.ps1 passed. Shared 4263 passed, 33 skipped, 4296 total. Tray
4003 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
The one-use page sent the browser to the SSH forward with the token
in the address. A process that binds that port after the response can
read the fragment. The page now stays on the handoff listener and
proxies to the forward only while ownership still holds.

./build.ps1 passed. Shared 4263 passed, 33 skipped, 4296 total. Tray
4003 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Oct 8, 2026
The handoff listener used a new port and closed after two minutes, so a
reload lost the Control UI profile and freed that origin. It also sent
bytes to the forward before checking that the SSH process still owned
the port, and it downgraded loopback TLS to plain HTTP. Launches now
share 127.0.0.1:47831 for the life of the tray. Unused links expire.
Connected sockets are checked again before any dashboard bytes move,
and the upstream scheme follows the saved destination.

./build.ps1 passed. Shared 4263 passed, 33 skipped, 4296 total. Tray
4003 passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
The UI job on run 37845784723 aborted because the test host exited.
The tests named at the crash were markdown list layout and workspace
session layout. Tray, Setup and connect E2E, and the other product
checks passed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
ConcurrentCreationConvergesOnOneCredential failed on run 37848102774
because api-credential.dpapi was in use by another process. UI passed
on this tree. The dashboard handoff does not touch that store.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>

This branch has not been deployed

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

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants