Skip to content

feat: add clawctl open command - #84

Open
paulcam206 wants to merge 1 commit into
mainfrom
add-clawctl-open-command
Open

paulcam206 wants to merge 1 commit into
mainfrom
add-clawctl-open-command

Conversation

@paulcam206

Copy link
Copy Markdown
Collaborator

What Problem This Solves

The packaged clawctl surface has no safe command that opens the authenticated Control UI for its managed gateway.

User Impact

User impact: after setup and gateway startup, users can run clawctl open to open OpenClaw's verified one-time browser handoff without revealing the long-lived gateway token.

Why This Change Was Made

clawctl open requires completed setup and a positively observed running managed gateway, then delegates TLS, base-path, authentication, readiness, and one-time pairing resolution to packaged OpenClaw's dashboard --json flow. The launcher accepts only an absolute loopback HTTP(S) handoff, opens it through the Windows default browser, and never writes the authenticated URL to human output, JSON, or logs.

Gateway status output now directs users to clawctl open instead of recommending token disclosure. Missing or incomplete setup and non-running gateways fail without starting lifecycle work or opening a browser.

Evidence

Validated at head 7f46faf914121196e7550496bfe1eeecd61d7184:

  • .\scripts\Test-DotNetQuality.ps1 — passed with 0 warnings and 0 errors.
  • dotnet test .\OpenClaw.Gateway.MSIX.slnx --configuration Release --no-restore — 827 passed.
  • .\scripts\Test-NativeAotCli.Tests.ps1 — 18 NativeAOT scenarios passed.
  • .\scripts\Test-DocReferences.ps1 — 0 findings.
  • Independent pre-commit review findings were resolved: gateway output uses the one-time handoff, running gateways without a resolved port still receive guidance, and Program-level scenarios cover successful and non-running behavior without credential leakage.

Not run: signing and MSIX composition lanes, because this change does not modify package contents, manifest, signing inputs, or release policy.

Require completed setup and a running managed gateway before opening OpenClaw's verified one-time Control UI handoff in the default browser.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 9ff501e8-3075-418e-9816-146dedceca22
@clawsweeper

clawsweeper Bot commented Sep 19, 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 P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 19, 2026
@clawsweeper

clawsweeper Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 18, 2026, 9:51 PM ET / September 19, 2026, 01:51 UTC.

ClawSweeper review

What this changes

Adds clawctl open to obtain an authenticated Control UI handoff from packaged OpenClaw and open it in the Windows default browser without printing credentials.

Merge readiness

Blocked before merge - 6 items remain

This remains a useful, distinct addition absent from main. Two implementation defects need correction, and the new isolated-session-to-host browser boundary needs focused security proof.

Priority: P2
Reviewed head: 7f46faf914121196e7550496bfe1eeecd61d7184

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The feature has useful scope and substantial tests, but launch defects and missing proof for the new authority boundary prevent merge readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: Authority-chain proof required: the added tests substitute guest output and browser activation, so they do not prove that an unrelated loopback destination or replaced session is rejected before host browser I/O. Supply redacted final-effect evidence for those cases and the authorized handoff. The collaborator exemption still applies to unrelated contributor-proof requirements. 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 added tests substitute guest output and browser activation, so they do not prove that an unrelated loopback destination or replaced session is rejected before host browser I/O. Supply redacted final-effect evidence for those cases and the authorized handoff. The collaborator exemption still applies to unrelated contributor-proof requirements. 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 10 items Verified review scope and policy: Read the complete root, src, and tests AGENTS.md files, contributor guidance, and architecture documentation. No .agents/maintainer-notes directory exists. The introduced change is the pinned main-to-head delta across 13 files.
Still necessary on main and latest release: Main's command tree has no open command. The latest release is v2026.9.4-msix.2, targeting a99bb66; its command tree also lacks open. No verified replacement PR was established.
Explicit upstream dependency: The new handler executes packaged openclaw.mjs dashboard --json. release-policy.json identifies openclaw/openclaw and pins approvedCommit 3a9d69db306cd7f081e06254cb89c4bcc14a7107, making that upstream command contract directly applicable.
Findings 2 actionable findings [P2] Preserve native-runtime setup in the captured dashboard launch
[P2] Accept successful shell activation without a process handle
Security Needs attention Loopback validation does not establish managed-gateway identity: The host accepts any loopback HTTP(S) URL supplied through captured guest output. The prior gateway observation is not bound to the returned origin, and capture-time generation checks end before browser activation. Demonstrate rejection of a substituted destination and stale session before host navigation; this is an unresolved boundary concern, not a demonstrated exploit.

How this fits together

The Windows launcher manages OpenClaw inside an isolated agent account. This command retrieves a browser handoff from that account and passes it to the invoking user's Windows browser.

flowchart LR
 A[clawctl open] --> B[Check setup and managed gateway]
 B --> C[Run packaged dashboard in isolated session]
 C --> D[Capture authenticated handoff]
 D --> E[Validate browser destination]
 E --> F[Open host browser]
 F --> G[Report result without credentials]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: the added tests substitute guest output and browser activation, so they do not prove that an unrelated loopback destination or replaced session is rejected before host browser I/O. Supply redacted final-effect evidence for those cases and the authorized handoff. The collaborator exemption still applies to unrelated contributor-proof requirements. 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 native-runtime setup in the captured dashboard launch (P2) - When the dashboard loads a staged native dependency, this request resolves it from the immutable package instead of the agent-owned native root, where the isolated identity is allowed to load it. Existing OpenClaw launches supply the redirect environment, NodeOptionsSuffix, and NativeRootPath; this handler supplies none of them, and ExecuteCommandCaptureAsync also drops the latter two fields when serializing requests. Reuse the existing runtime composition and forward both fields so native loads and their lifetime lease work through this entry point.
  • Accept successful shell activation without a process handle (P2) - Windows can successfully hand a URL to an existing browser without returning a process handle. .NET then returns null from Process.Start, so this branch reports failure and makes clawctl open exit 1 even though the browser accepted the URL. Treat shell exceptions as failure, allow the null success case, and replace the test that currently requires null to throw.
  • Resolve security concern: Loopback validation does not establish managed-gateway identity - The host accepts any loopback HTTP(S) URL supplied through captured guest output. The prior gateway observation is not bound to the returned origin, and capture-time generation checks end before browser activation. Demonstrate rejection of a substituted destination and stale session before host navigation; this is an unresolved boundary concern, not a demonstrated exploit.
  • Resolve merge risk (P1) - An isolated-account-produced URL can select any host-loopback HTTP(S) destination; the evidence does not establish that an unrelated local service or replaced session is rejected before browser navigation.
  • Complete next step (P2) - Correct both launch defects and provide the scoped authority-boundary proof before merge; redact credentials and private endpoints from evidence.

Findings

  • [P2] Preserve native-runtime setup in the captured dashboard launch — src/OpenClaw.Launcher/Program.cs:649-652
  • [P2] Accept successful shell activation without a process handle — src/OpenClaw.Launcher/Program.cs:836-840
  • [medium] Loopback validation does not establish managed-gateway identity — src/OpenClaw.Launcher/Gateway/ControlUiHandoff.cs:26
Agent review details

Security

Needs attention: Credential output is kept private, but destination authorization across the isolated-account-to-host-browser boundary remains unproven.

Review metrics

Metric Value Why it matters
Production and test growth Production +318/-6; tests +453/-8 The command and captured-output adapter explain the growth, but duplicating request construction has already omitted existing launch fields.

Merge-risk options

Maintainer options:

  1. Prove the browser destination boundary (recommended)
    Establish that a substituted loopback destination and a replaced session cannot cause host browser navigation, while the authorized managed gateway still opens.
  2. Pause the host browser bridge
    Hold the new command until its cross-account destination authorization can be demonstrated.

Technical review

Best possible solution:

Keep upstream responsible for authentication, reuse the complete existing native-launch contract, and bind host browser delivery to the currently authorized managed gateway with final-effect proof.

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

Yes, source establishes the shell-activation defect: successful activation without a process handle reaches the new failure branch. Native-launch fields are also demonstrably omitted; no Windows runtime reproduction was performed.

Is this the best way to solve the issue?

The upstream handoff is the right owner for endpoint and authentication decisions, but this implementation needs the existing native-launch contract and correct Windows shell semantics before it is reliable.

Full review comments:

  • [P2] Preserve native-runtime setup in the captured dashboard launch — src/OpenClaw.Launcher/Program.cs:649-652
    When the dashboard loads a staged native dependency, this request resolves it from the immutable package instead of the agent-owned native root, where the isolated identity is allowed to load it. Existing OpenClaw launches supply the redirect environment, NodeOptionsSuffix, and NativeRootPath; this handler supplies none of them, and ExecuteCommandCaptureAsync also drops the latter two fields when serializing requests. Reuse the existing runtime composition and forward both fields so native loads and their lifetime lease work through this entry point.
    Confidence: 0.93
  • [P2] Accept successful shell activation without a process handle — src/OpenClaw.Launcher/Program.cs:836-840
    Windows can successfully hand a URL to an existing browser without returning a process handle. .NET then returns null from Process.Start, so this branch reports failure and makes clawctl open exit 1 even though the browser accepted the URL. Treat shell exceptions as failure, allow the null success case, and replace the test that currently requires null to throw.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 9c2106647ebb.

Labels

Label changes:

  • add P2: This is a bounded Control UI access improvement with actionable implementation defects, without evidence of an existing widespread outage.
  • add merge-risk: 🚨 security-boundary: The new bridge turns isolated-account output into host-browser navigation without demonstrated binding to the currently authorized gateway.
  • 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 added tests substitute guest output and browser activation, so they do not prove that an unrelated loopback destination or replaced session is rejected before host browser I/O. Supply redacted final-effect evidence for those cases and the authorized handoff. The collaborator exemption still applies to unrelated contributor-proof requirements. 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:

  • P2: This is a bounded Control UI access improvement with actionable implementation defects, without evidence of an existing widespread outage.
  • merge-risk: 🚨 security-boundary: The new bridge turns isolated-account output into host-browser navigation without demonstrated binding to the currently authorized gateway.
  • 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 added tests substitute guest output and browser activation, so they do not prove that an unrelated loopback destination or replaced session is rejected before host browser I/O. Supply redacted final-effect evidence for those cases and the authorized handoff. The collaborator exemption still applies to unrelated contributor-proof requirements. 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] Loopback validation does not establish managed-gateway identity — src/OpenClaw.Launcher/Gateway/ControlUiHandoff.cs:26
    The host accepts any loopback HTTP(S) URL supplied through captured guest output. The prior gateway observation is not bound to the returned origin, and capture-time generation checks end before browser activation. Demonstrate rejection of a substituted destination and stale session before host navigation; this is an unresolved boundary concern, not a demonstrated exploit.
    Confidence: 0.87

What I checked:

  • Verified review scope and policy: Read the complete root, src, and tests AGENTS.md files, contributor guidance, and architecture documentation. No .agents/maintainer-notes directory exists. The introduced change is the pinned main-to-head delta across 13 files. (AGENTS.md:1, 7f46faf91412)
  • Still necessary on main and latest release: Main's command tree has no open command. The latest release is v2026.9.4-msix.2, targeting a99bb66; its command tree also lacks open. No verified replacement PR was established. (src/OpenClaw.Launcher/ClawCtlCommandLine.cs:34, a99bb66d2162)
  • Explicit upstream dependency: The new handler executes packaged openclaw.mjs dashboard --json. release-policy.json identifies openclaw/openclaw and pins approvedCommit 3a9d69db306cd7f081e06254cb89c4bcc14a7107, making that upstream command contract directly applicable. (src/OpenClaw.Launcher/Program.cs:647, 7f46faf91412)
  • Pinned upstream supports the handoff: The pinned dashboard JSON path disables recovery, checks gateway and document readiness, and returns browserUrl from the existing one-time bootstrap owner. This supports delegating endpoint and authentication decisions upstream. (src/commands/dashboard.ts:64, 3a9d69db306c)
  • Native launch contract omitted: The new dashboard request uses only the basic runtime environment, unlike existing OpenClaw launches that provide native redirect variables, NodeOptionsSuffix, and NativeRootPath. The new capture executor also drops the latter two request fields. Merged fix: load packaged native addons from the isolated session #75 documents why native packages cannot load directly from WindowsApps under the isolated identity. (src/OpenClaw.Launcher/Program.cs:650, 7f46faf91412)
  • Successful shell activation can return null: At .NET v10.0.0, StartWithShellExecuteEx throws for shell errors but returns false when successful activation supplies no process handle; Process.Start(ProcessStartInfo) converts false to null. A null return therefore does not establish browser-launch failure. (src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs:1346, 60629d14374c)

Likely related people:

  • paulcam206: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • anna-dingler: 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.

  • Preserve native redirect variables, NodeOptionsSuffix, and NativeRootPath through captured execution, with focused regression coverage.
  • Correct the null-process shell success case and its test.
  • Add redacted final-effect proof for authorized delivery, an unrelated loopback destination, and session replacement before browser I/O.

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.

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

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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.

1 participant