fix(connection): Open Dashboard can send the token before the SSH forward owns the port

[SebTardif]

What Problem This Solves

Fixes: Open Dashboard can put the shared gateway token in the browser URL before the SSH forward owns the local port.

User Impact

User impact: the browser opens only after this app's ssh process owns the forward. If another process already has the port, the token is not sent.

Why This Change Was Made

The dashboard path now rejects an occupied forward port and waits with the same owned-listener check StartOwnedAsync uses. A failed wait shows the existing SSH tunnel toast and returns before the URL is built. SSH off still opens the dashboard directly. Other EnsureStarted callers are unchanged.

Evidence

Terminal output from the connection and tray contract runs on this branch.

EnsureSettingsOwnedForwardReadyAsync
Passed!  - Failed: 0, Passed: 2, Skipped: 0, Total: 2, Duration: 158 ms

Dashboard_AwaitsSettingsOwnedForwardBeforeTokenUrl
Dashboard_SurfacesSshTunnelConfigurationFailure
Passed!  - Failed: 0, Passed: 2, Skipped: 0, Total: 2, Duration: 61 ms

The occupied-port case returns false and does not report the forward as owned. The source contract requires OpenDashboardAsync to await EnsureDashboardSshForwardOwnedAsync before GatewayDashboardUrlBuilder.Build.

Change Type

• Bug fix
• Feature
• Refactor
• Docs or instructions
• Tests or validation
• Security hardening
• Chore or infrastructure

Scope

• Tray or WinUI UX
• Windows node capability
• Local MCP or winnode
• Gateway, connection, or pairing
• Setup or onboarding
• Permissions, privacy, or security
• Tests, CI, or docs

Required proof pools

• windows-winui-interactive: the dashboard open path changed. Not verified / blocked: the tray was not launched and no browser was opened.

Validation

• dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj --filter FullyQualifiedName~EnsureSettingsOwnedForwardReadyAsync: Passed 2, Failed 0.
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --filter "FullyQualifiedName~Dashboard_SurfacesSshTunnelConfigurationFailure|FullyQualifiedName~Dashboard_AwaitsSettingsOwnedForwardBeforeTokenUrl": Passed 2, Failed 0.
• dotnet build src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj -p:Platform=x64 on dc29fbb4 succeeded. One existing Win2D warning. ./build.ps1 was not run.

Real Behavior Proof

• Behavior or issue addressed: Open Dashboard opened a token URL as soon as ssh.exe was spawned, before the forward owned the port.
• Real environment tested: Windows, this worktree.
• Exact steps or command run after this patch: dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj --filter FullyQualifiedName~EnsureSettingsOwnedForwardReadyAsync
• Evidence after fix: The owned-forward helper returned false when the local port was already taken, and the dashboard method awaits that helper before building the URL.
• Observed result after fix: A foreign listener does not receive the dashboard token URL from this path.
• Screenshot or artifact links verified? No
• Not verified or blocked: ssh.exe was not spawned and a browser was not opened.
• What was not tested: A live SSH tunnel and a real browser launch.

Security Impact

• New permissions or capabilities? No
• Secrets or tokens handling changed? Yes
• New or changed network calls? No
• Command or tool execution surface changed? No
• Data access scope changed? No
• If any answer is Yes, explain the risk and mitigation: The shared token is appended to the dashboard URL only after the settings-owned ssh process owns the local forward. A failed wait stops the process and skips the URL.

Compatibility and Migration

• Backward compatible? Yes
• Config or environment changes? No
• Migration needed? No

Review Conversations

• I replied to or resolved every bot review conversation addressed by this PR.
• I left unresolved only conversations that still need maintainer judgment.

[clawsweeper]

🦞👀
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]

Codex review: needs real behavior proof before merge. Reviewed September 24, 2026, 2:16 PM ET / 18:16 UTC.

ClawSweeper review

What this changes

The branch makes Open Dashboard wait for an SSH forward owned by the app before opening a token-bearing browser URL, and adds connection and tray contract tests.

Merge readiness

⛔ Blocked before merge - 6 items remain

The dashboard race is present on current main, so this PR remains useful. Review found a concurrent tunnel-state regression, and the claimed token protection still needs proof at the browser handoff.

Priority: P1
Reviewed head: dc29fbb416a081b99c66f680c565b081591a17b3

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch targets a real security-relevant race, but a concurrent state defect and missing final-handoff proof limit merge confidence.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: Authority-chain proof required: The changed tray action waits on the production SSH service before launching a credential URL, but the supplied Windows evidence is focused tests and source contracts; the PR states that ssh.exe and the browser were not launched. Show an owned-forward browser launch and rejection before browser I/O for a foreign or replaced listener, with private details redacted. No stored-data contract changes. 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.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Authority-chain proof required: The changed tray action waits on the production SSH service before launching a credential URL, but the supplied Windows evidence is focused tests and source contracts; the PR states that ssh.exe and the browser were not launched. Show an owned-forward browser launch and rejection before browser I/O for a foreign or replaced listener, with private details redacted. No stored-data contract changes. 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.
Evidence reviewed	6 items	Introduced dashboard gate: The PR awaits the new forward check before resolving credentials, building the URL, and launching the browser. Concurrent state overwrite: A wait can fail after another operation replaces the captured process. StopIfCurrent then leaves the replacement alone, but the catch unconditionally marks the shared service Failed. Point-in-time ownership: The ownership wait returns after a matching listener snapshot; the dashboard later resolves the credential and launches the browser without pinning or rechecking that process at the final handoff.
Findings	1 actionable finding	[P2] Preserve a newer tunnel's state when an older wait fails
Security	Needs attention	Token URL can outlive the ownership check: A tunnel may stop or be replaced after the listener snapshot succeeds and before the browser opens the URL. The nearest foreign-listener case needs final-effect proof or a generation-bound guard before relying on this as a credential boundary.

How this fits together

The tray opens a Gateway dashboard using connection settings and a resolved credential. When SSH forwarding is enabled, the connection service starts a local forward; the tray then builds a URL and hands it to the browser.

flowchart LR
  A[Open Dashboard action] --> B[Connection settings]
  B --> C{SSH enabled?}
  C -->|Yes| D[Local SSH forward]
  D --> E[Listener ownership check]
  C -->|No| F[Credential and URL]
  E --> F
  F --> G[Browser launch]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: The changed tray action waits on the production SSH service before launching a credential URL, but the supplied Windows evidence is focused tests and source contracts; the PR states that ssh.exe and the browser were not launched. Show an owned-forward browser launch and rejection before browser I/O for a foreign or replaced listener, with private details redacted. No stored-data contract changes. 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.
• Preserve a newer tunnel's state when an older wait fails (P2) - Two dashboard attempts can overlap. If a newer attempt replaces the captured process while the older attempt waits, StopIfCurrent correctly leaves the replacement running, but this unconditional assignment marks that newer tunnel Failed. Guard the error update with the captured generation and process, and cover the overlap.
• Resolve security concern: Token URL can outlive the ownership check - A tunnel may stop or be replaced after the listener snapshot succeeds and before the browser opens the URL. The nearest foreign-listener case needs final-effect proof or a generation-bound guard before relying on this as a credential boundary.
• Resolve merge risk (P1) - Listener ownership is established before credential resolution and browser launch. A stopped or replaced forward could leave the token-bearing URL aimed at a different local listener; the final handoff and nearest foreign-listener case are unproven.
• Resolve merge risk (P1) - The repository-required full build, shared tests, and full tray tests have not been reported for this head.
• Complete next step (P2) - Guard the failure update against a replaced tunnel, provide redacted Windows proof of the owned and foreign-listener browser paths, and complete the repository-required validation.

Findings

• [P2] Preserve a newer tunnel's state when an older wait fails — src/OpenClaw.Connection/SshTunnelService.cs:622-626
• [medium] Token URL can outlive the ownership check — src/OpenClaw.Tray.WinUI/App.xaml.cs:3903

Agent review details

Security

Needs attention: The startup check improves token safety, but ownership at the final browser launch remains unproven.

Review metrics

Metric	Value	Why it matters
Code and test delta	production +88 lines, tests +70 lines	The production growth adds an ownership wait for the stated security fix; the tests cover occupied ports and source ordering, but no live browser handoff.

Merge-risk options

Maintainer options:

1. Prove and guard the handoff (recommended)
   Pin or revalidate tunnel ownership at the browser handoff and demonstrate that a foreign listener cannot receive the token-bearing URL.
2. Pause the dashboard change
   Hold this PR if a safe browser handoff cannot be established within its scope.

Technical review

Best possible solution:

Keep the dashboard credential handoff tied to the verified tunnel generation through browser launch, preserve a newer tunnel's state when an older wait fails, and show a redacted Windows run covering both an owned forward and a foreign listener.

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

Yes for the current-main defect: Open Dashboard builds its URL immediately after starting SSH, while listener ownership is checked later. The introduced concurrent-state defect is clear from the catch path; no live run was performed in this read-only review.

Is this the best way to solve the issue?

No, not yet. Waiting for an owned listener addresses startup ordering, but the shared state must remain generation-safe and the final browser handoff needs proof.

Full review comments:

• [P2] Preserve a newer tunnel's state when an older wait fails — src/OpenClaw.Connection/SshTunnelService.cs:622-626
  Two dashboard attempts can overlap. If a newer attempt replaces the captured process while the older attempt waits, StopIfCurrent correctly leaves the replacement running, but this unconditional assignment marks that newer tunnel Failed. Guard the error update with the captured generation and process, and cover the overlap.
  Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 273b0182745a.

Labels

Label changes:

• add P1: The change concerns exposure of a shared Gateway token during a real dashboard workflow.
• add merge-risk: 🚨 security-boundary: The patch uses a point-in-time local listener check to authorize a later token-bearing browser launch.
• add merge-risk: 🚨 availability: An older failed wait can mark a newer running tunnel Failed.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: The changed tray action waits on the production SSH service before launching a credential URL, but the supplied Windows evidence is focused tests and source contracts; the PR states that ssh.exe and the browser were not launched. Show an owned-forward browser launch and rejection before browser I/O for a foreign or replaced listener, with private details redacted. No stored-data contract changes. 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.

Label justifications:

• P1: The change concerns exposure of a shared Gateway token during a real dashboard workflow.
• merge-risk: 🚨 security-boundary: The patch uses a point-in-time local listener check to authorize a later token-bearing browser launch.
• merge-risk: 🚨 availability: An older failed wait can mark a newer running tunnel Failed.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: The changed tray action waits on the production SSH service before launching a credential URL, but the supplied Windows evidence is focused tests and source contracts; the PR states that ssh.exe and the browser were not launched. Show an owned-forward browser launch and rejection before browser I/O for a foreign or replaced listener, with private details redacted. No stored-data contract changes. 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.

Evidence

Security concerns:

• [medium] Token URL can outlive the ownership check — src/OpenClaw.Tray.WinUI/App.xaml.cs:3903
  A tunnel may stop or be replaced after the listener snapshot succeeds and before the browser opens the URL. The nearest foreign-listener case needs final-effect proof or a generation-bound guard before relying on this as a credential boundary.
  Confidence: 0.82

What I checked:

• Introduced dashboard gate: The PR awaits the new forward check before resolving credentials, building the URL, and launching the browser. (src/OpenClaw.Tray.WinUI/App.xaml.cs:3903, dc29fbb416a0)
• Concurrent state overwrite: A wait can fail after another operation replaces the captured process. StopIfCurrent then leaves the replacement alone, but the catch unconditionally marks the shared service Failed. (src/OpenClaw.Connection/SshTunnelService.cs:622, dc29fbb416a0)
• Point-in-time ownership: The ownership wait returns after a matching listener snapshot; the dashboard later resolves the credential and launches the browser without pinning or rechecking that process at the final handoff. (src/OpenClaw.Connection/SshTunnelService.cs:615, dc29fbb416a0)
• Current-main behavior: Before this PR, Open Dashboard started the settings tunnel and then built and launched the credential URL without waiting for listener ownership. (src/OpenClaw.Tray.WinUI/App.xaml.cs:3895, 273b0182745a)
• Proof and validation scope: The PR body reports passing focused tests and a WinUI project build, but explicitly says neither ssh.exe nor a browser was launched and that the full build was not run. (dc29fbb416a0)
• Area history: Local history identifies prior merged SSH credential-handoff, recovery, and saved-tunnel work; older blobs could not be fetched, so this supports routing rather than source-line introduction. (src/OpenClaw.Connection/SshTunnelService.cs, a51029ef6f77)

Likely related people:

• Scott Hanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Barbara Kudiess: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Guard failure-state updates by tunnel generation and add an overlapping-attempt regression test.
• Add final-effect proof for the nearest foreign listener and prove replacement of the checked tunnel prevents token-bearing browser I/O.
• Report the repository-required full build, shared tests, and full tray tests.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[karkarl]

Global triage: HOLD_FOR_AUTHOR. Take confidence 10%; recommendation confidence 97%; effort medium; risk high.

Two handoff races block this exact head:

1. Dashboard tunnel ownership is reduced to a point-in-time Boolean. The verified process and generation are discarded before credential resolution and browser launch, so a concurrent tunnel stop or replacement can put another localhost listener behind a page whose fragment contains the shared token.
2. When an older waiter detects replacement, StopIfCurrent preserves the newer process, but the catch path unconditionally writes LastError and Status=Failed, exposing a running replacement tunnel as failed.

Please carry a generation-bound ownership lease through browser launch, prevent replacement during that handoff, and guard failure-state updates by the captured process/generation. Add overlapping-attempt and replacement-before-launch regressions.

Run windows-winui-interactive with the isolated current-head app and prove real ssh.exe ownership before launch, no browser/network effect on port conflict, and no launch after ownership replacement. Required build, Shared, Tray, and rubber-duck closeout are also missing from the body.

The E2E setup refusals appear shared/baseline; the UI lane timed out and remains unresolved. CI Gate is derivative, so the required current-head proof is not green.