fix: secure WebVNC browser handoffs

[steipete]

Summary

• replace CLI-generated WebVNC username/password URL fragments with authenticated one-use handoff tickets
• consume tickets only from authorized portal sessions and remove them from the address bar before connecting
• reuse and focus one existing lease viewer through a selected-recipient cross-tab handshake
• handle browsers that reuse the current tab through hash-only navigation

Verification

• go test -race ./...
• go vet ./...
• npm run format:check --prefix worker
• npm run lint --prefix worker
• npm run check --prefix worker
• npm test --prefix worker
• npm run build --prefix worker
• npm run build:node --prefix worker
• autoreview: clean, no accepted/actionable findings

Live proof

• provider: local-container
• lease: cbx_a8378a283279
• isolated Node/PostgreSQL coordinator with registered desktop lease
• first webvnc --open --take-control: connected viewer; no username, password, or handoff remained in the Chrome URL
• repeated open: exactly one matching Chrome tab remained connected
• ticket probes: other lease 404, first correct redemption 200, replay 401
• test lease, bridge processes, local coordinator, database, proxy, and credential-bearing temporary files were removed after proof

[clawsweeper]

Codex review: found issues before merge. Reviewed July 9, 2026, 3:25 AM ET / 07:25 UTC.

Summary
The branch moves CLI WebVNC browser launches from username/password URL fragments to one-use handoff tickets, adds existing-tab reuse, updates docs, and extends CLI/Worker tests.

Reproducibility: not applicable. as a conventional bug reproduction: this is a security-hardening PR. Source inspection confirms current main still creates username/password fragments, and the PR body provides live WebVNC proof for the changed behavior.

Review metrics: 2 noteworthy metrics.

• Changed Surface: 8 files, +305/-58. The credential handoff spans CLI, Worker routing, portal browser script, docs, and tests.
• New Handoff Route: 1 API route added. The new /v1/leases/:id/webvnc/handoff route is the main security-sensitive merge surface.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Remove the CHANGELOG.md entry added by this PR.
• Get maintainer confirmation on accepting the existing handoff storage boundary while [security] WebVNC handoff persists share credentials as plaintext Durable Object records #969 remains open.

Risk before merge

• [P1] The PR expands the existing plaintext WebVNC handoff record path to CLI-generated credentials while [security] WebVNC handoff persists share credentials as plaintext Durable Object records #969 remains open; maintainers should explicitly accept that boundary or require a storage redesign before merge.
• [P1] The PR currently edits release-owned CHANGELOG.md; that line should be removed and release-note context kept in the PR body or commit metadata.

Maintainer options:

1. Accept Current Handoff Store (recommended)
   Merge after removing the CHANGELOG.md entry if maintainers accept using the existing one-use plaintext handoff store for this URL-fragment fix while [security] WebVNC handoff persists share credentials as plaintext Durable Object records #969 remains open.
2. Require Encrypted Handoffs First
   Hold the PR until the handoff implementation encrypts or otherwise avoids plaintext Durable Object credential records before CLI credentials use it.
3. Pause Until Storage Direction Settles
   Pause or close this PR if maintainers do not want to expand WebVNC handoff usage before deciding the broader credential-storage model.

Next step before merge

• Maintainer security review should decide whether expanding the existing plaintext handoff store to CLI credentials is acceptable; the changelog edit is a small contributor cleanup.

Maintainer decision needed

• Question: Should CLI-generated WebVNC credentials reuse the existing one-use Durable Object handoff store while [security] WebVNC handoff persists share credentials as plaintext Durable Object records #969 remains open?
• Rationale: The patch removes credentials from browser URLs, but it routes CLI credentials into the same plaintext handoff storage model that already has an open security issue, so automation should not infer the intended security boundary.
• Likely owner: steipete — This account owns the relevant WebVNC handoff and redaction history and authored the prior merged changes in this area.
• Options:
  • Accept Existing Handoff Store (recommended): Merge this URL-fragment hardening after the changelog cleanup and keep encrypted or non-plaintext handoff storage tracked separately.
  • Block On Storage Redesign: Require this PR to avoid plaintext Durable Object credential records before expanding the handoff path to CLI-generated credentials.

Security
Needs attention: The diff improves URL secrecy but expands an existing plaintext WebVNC handoff storage boundary that has an open security issue.

Review findings

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:15

Review details

Best possible solution:

Land the URL-fragment hardening after maintainer acceptance of the handoff storage boundary and removal of the release-owned CHANGELOG.md entry, while tracking plaintext-at-rest storage separately in #969.

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

Not applicable as a conventional bug reproduction: this is a security-hardening PR. Source inspection confirms current main still creates username/password fragments, and the PR body provides live WebVNC proof for the changed behavior.

Is this the best way to solve the issue?

Unclear pending maintainer security intent: one-use tickets are a narrow fix for URL fragments, but reusing plaintext Durable Object handoff records for CLI credentials should be explicitly accepted or redesigned first.

Full review comments:

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:15
  ClawSweeper's release-note policy keeps CHANGELOG.md release-owned for normal PRs. Please move this note to the PR body or commit context and leave release aggregation to the release process.
  Confidence: 0.9

Overall correctness: patch is correct
Overall confidence: 0.84

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 2a0120722173.

Label changes

Label changes:

• add P2: This is a security hardening for WebVNC credential handling with limited scope and live proof, not an emergency runtime outage.
• add merge-risk: 🚨 security-boundary: Merging changes how VNC credentials move through CLI, coordinator storage, portal sessions, and browser tabs.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live proof from a local-container WebVNC run with URL cleanup, single-tab reuse, and ticket probe outcomes.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live proof from a local-container WebVNC run with URL cleanup, single-tab reuse, and ticket probe outcomes.

Label justifications:

• P2: This is a security hardening for WebVNC credential handling with limited scope and live proof, not an emergency runtime outage.
• merge-risk: 🚨 security-boundary: Merging changes how VNC credentials move through CLI, coordinator storage, portal sessions, and browser tabs.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live proof from a local-container WebVNC run with URL cleanup, single-tab reuse, and ticket probe outcomes.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live proof from a local-container WebVNC run with URL cleanup, single-tab reuse, and ticket probe outcomes.

Evidence reviewed

Security concerns:

• [medium] Confirm plaintext handoff storage before expanding CLI use — worker/src/fleet.ts:7923
  The PR issues CLI-generated username/password handoffs through the existing WebVNCCredentialHandoffs store, whose records keep credential fields until redemption or expiry; maintainers should explicitly accept this boundary or require storage hardening first.
  Confidence: 0.78

What I checked:

• Repository policy read: AGENTS.md was read in full before review; no target-specific policy violation was found in the WebVNC implementation itself. (AGENTS.md:1, 2a0120722173)
• Current main still uses credential fragments: Current main builds the WebVNC portal URL directly from username/password before printing or opening the portal. (internal/cli/webvnc.go:315, 2a0120722173)
• PR adds CLI credential handoff creation: The PR head creates a coordinator handoff when username or password is present and only puts the returned ticket in the portal URL. (internal/cli/webvnc.go:4076, 3748a5169827)
• PR exposes API issuance and portal-only redemption: The new /v1/leases/:id/webvnc/handoff path issues handoffs for manageable leases, while ticket redemption is rejected outside the portal route. (worker/src/fleet.ts:7870, 3748a5169827)
• Existing handoff storage keeps credentials in DO records: Current main's handoff helper stores username and password fields until the one-use ticket is consumed or expires. (worker/src/webvnc-handoff.ts:29, 2a0120722173)
• Related open security issue: GitHub search found [security] WebVNC handoff persists share credentials as plaintext Durable Object records #969 open for plaintext WebVNC handoff credentials in Durable Object records, which overlaps the storage boundary this PR expands.

Likely related people:

• steipete: Prior merged WebVNC credential redaction, shared handoff, local bridge, and responsiveness commits all point to this account as the central owner for this behavior. (role: feature-history owner; confidence: high; commits: 2d589cd63c7c, 0abf47ae1267, ec67c93a7936; files: internal/cli/webvnc.go, worker/src/fleet.ts, worker/src/portal.ts)
• Vincent Koc: Recent Daytona coordinator-managed lease work touched the same Worker portal/fleet area, though not the specific WebVNC credential handoff boundary. (role: recent adjacent contributor; confidence: low; commits: caa79b1eb5c8; files: worker/src/fleet.ts, worker/src/portal.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works

• 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.