fix(vnc): reject native-VNC grants for sessions that are not live

[anagnorisis2peripeteia]

What Problem This Solves

The native-VNC grant endpoint hands a caller usable desktop credentials for a session whose workspace
is already stopping or gone.

createNativeVNCGrant (in worker-application.ts) checked only that the session advertises
nativeVnc before minting — it never checked the session's lifecycle status. The browser-VNC path
for the same runtime-adapter workspace (InteractiveDesktopService.open) does two things this path
did not:

• rejects stopping / stopped / expired / failed before minting, and
• re-validates the session after the mint round trip (hasCurrentAccess), because the session can
  begin teardown while the grant is being created.

So a client could obtain a working native-VNC credential for a workspace being torn down, and a stop
racing the mint would leak a credential for a dead workspace.

The change

Extract the native-VNC path into a small NativeVNCService with injected deps (readSession,
mintGrant) so it is unit-testable, and give it the same guards as its browser-VNC sibling:

• a pre-mint live-status allowlist (ready / attached / detached), and
• a post-mint re-validation that fails closed (session authorization changed; retry) if — during the
  mint round trip — the session left a live state, control authorization was revoked, or its workspace
  identity moved (control-plane, workspace id, provider resource, or profile). This mirrors the
  browser-VNC sibling's hasCurrentAccess, which re-checks control and the full workspace identity
  after the mint, not just status. (providerResourceId in particular is rewritten by session
  reconciliation while a session stays live, so pinning it matters.)

worker-application.ts now just wires the service in; no behaviour change beyond the added guards.

Real production-endpoint proof (before → after)

The fix on the real HTTP endpoint: crabfleet running under wrangler dev (workerd) with a real
local D1 (SQLite), a real native-auth access token (bootstrap-owner identity), and real routing — not
a harness. POST /api/native/v1/native-vnc for a seeded native-VNC session whose status is
stopping:

BASE (origin/main @f4ef10b — inline createNativeVNCGrant, NO status guard)
  -> HTTP 500  {"error":"internal error"}
  Not refused: proceeds PAST to workspaceLifecycle().createNativeVNCGrant (the mint) and only
  fails at the runtime-adapter call — i.e. it would issue a credential for a stopping workspace.

HEAD (this PR)
  -> HTTP 409  {"error":"native VNC is unavailable while the session is stopping"}
  Refused at the pre-mint guard, before any mint.

409 (refused at the guard) vs 500 (reached the mint) on the production endpoint is the fix.
Reproducible transcript + seed: https://gist.github.com/anagnorisis2peripeteia/d4f681075a24801417f293aed70c4b12

Service-level before/after (same two interleavings)

Driven through the extracted NativeVNCService (base = the verbatim inline body from
worker-application.ts@f4ef10b L207-224, since worker-application.ts can't be imported under
node --test — its module graph pulls @cloudflare/containers, which is why the path was
extracted):

BASE — no status guard, no post-mint revalidation
STOPPING-session    -> GRANT MINTED (leak) minted=true
STOP-during-mint    -> GRANT MINTED (leak) minted=true

HEAD — pre-mint status guard + post-mint revalidation
STOPPING-session    -> rejected 409 "native VNC is unavailable while the session is stopping"
STOP-during-mint    -> rejected 409 "session authorization changed; retry"

That the guard actually sits on the production path (and isn't just a property of a new class the
route could bypass) is pinned the way the repo's own application-architecture.test.ts pins
composition — a source assertion that the createNativeVNCGrant route delegates to NativeVNCService
with readVisibleFresh + workspaceLifecycle().createNativeVNCGrant, and no longer mints inline.

Testing

pnpm check (tsc + oxlint + oxfmt), pnpm build, pnpm exec wrangler deploy --dry-run, pnpm test
all green (957 tests).

tests/native-vnc-service.test.ts covers: mints for a live session; rejects a stopping session
before minting; fails closed when the session begins teardown during the mint; rejects a
missing/uncontrollable session; rejects ineligible sessions (asserting the mint is never reached on
any rejection path); fails closed when control is revoked or the control-plane is re-bound during the
mint; when the provider resource or profile changes during the mint; and the production-wiring
assertion above. Mutation testing of the new service scores 87% (74/85). A concurrency lint of the
diff (a race/leak static pass) is clean.

Related: FINDING 1 of #88 (a small set of lifecycle concurrency findings; separate PRs to follow).

Review history: https://gist.github.com/2f7c872efcad9a64122db222f4a54e67

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 1:45 AM ET / 05:45 UTC.

Summary
The PR extracts native-VNC grant issuance into NativeVNCService, rejects non-live sessions before minting, and revalidates authorization and workspace identity after minting.

Reproducibility: yes. in source and supplied endpoint evidence: a native-VNC-capable session in stopping state, or a stop racing minting, exercises the missing lifecycle guard. The provided transcript demonstrates the HTTP route, but this review did not independently execute the reproduction.

Review metrics: 2 noteworthy metrics.

• Patch surface: 3 files affected; 360 added, 23 removed. The implementation is isolated to one worker service, its application wiring, and focused regression coverage.
• Regression coverage: 269 test lines added. The added tests exercise both the pre-mint and concurrent post-mint authorization paths.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

• Refresh the branch or merge result against current main and re-run pnpm check, pnpm test, and pnpm macos:test on the exact landing candidate.

Risk before merge

• [P1] The branch base is 6471f31a7c98d3b9355715d70993ee01aedca0c5 while current main is 753d9233abd3023fe6970a8dcde0e1e398c95d4a; refresh the exact merge result and re-run the repository-required gates before landing.

Maintainer options:

1. Decide the mitigation before merge
   Land the narrowly scoped live-status and post-mint identity guard if it remains clean against current main, retaining the focused race-regression coverage.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P2] No mechanical repair is identified; a maintainer should validate the exact current-main merge result and required gates before deciding whether to land the security fix.

Security
Cleared: The diff adds authorization and lifecycle validation without adding dependencies, secrets handling, workflows, or broader privilege paths.

Review details

Best possible solution:

Land the narrowly scoped live-status and post-mint identity guard if it remains clean against current main, retaining the focused race-regression coverage.

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

Yes in source and supplied endpoint evidence: a native-VNC-capable session in stopping state, or a stop racing minting, exercises the missing lifecycle guard. The provided transcript demonstrates the HTTP route, but this review did not independently execute the reproduction.

Is this the best way to solve the issue?

Yes. Matching the browser-VNC sibling’s pre-mint lifecycle check and post-mint access revalidation is a narrow, maintainable repair that preserves the existing native-VNC route contract for live sessions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 753d9233abd3.

Label changes

Label changes:

• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• remove rating: 🦞 diamond lobster: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

• P1: This patch addresses issuance of native desktop credentials during workspace teardown, a security-boundary race with real-user impact.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR provides a linked, reproducible real wrangler dev endpoint transcript showing the after-fix request is refused at the new guard; redact any private endpoint or token details in future proof updates.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR provides a linked, reproducible real wrangler dev endpoint transcript showing the after-fix request is refused at the new guard; redact any private endpoint or token details in future proof updates.

Evidence reviewed

What I checked:

• Focused authorization guard: The added service reads the visible session, requires control and native-VNC eligibility, allows only ready, attached, and detached lifecycle states before minting, then re-reads and compares authorization and workspace identity after the mint round trip. (src/worker/native-vnc-service.ts:32, 610383689a96)
• Production route wiring: The route is changed from inline grant minting to constructing NativeVNCService with readVisibleFresh and the runtime adapter’s native-VNC grant mint function, keeping the guard on the HTTP path. (src/worker/worker-application.ts:202, 610383689a96)
• Regression coverage: The new test suite covers live-session minting, pre-mint rejection for stopping sessions, post-mint teardown, revoked control, and workspace-identity changes during minting, including a source-level production-wiring assertion. (tests/native-vnc-service.test.ts:54, 610383689a96)
• Real endpoint proof: The PR body links a reproducible wrangler dev/local-D1 endpoint transcript showing a stopping session reaches the runtime mint path on the base revision but receives the intended HTTP 409 before minting on this branch; the PR is labeled proof: sufficient. (610383689a96)

Likely related people:

• anagnorisis2peripeteia: The supplied context identifies this contributor as author of the broader lifecycle-race report and this focused native-VNC repair; current-main ownership history was not available in the supplied review material. (role: related concurrency-report author and proposed implementation author; confidence: low; commits: 610383689a96; files: src/worker/native-vnc-service.ts, src/worker/worker-application.ts, tests/native-vnc-service.test.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.

Review history (4 earlier review cycles)

• reviewed 2026-07-17T14:25:06.905Z sha 1d98f2b :: needs real behavior proof before merge. :: none
• reviewed 2026-07-17T17:53:33.490Z sha 6103836 :: needs real behavior proof before merge. :: none
• reviewed 2026-07-17T21:22:44.909Z sha 6103836 :: needs maintainer review before merge. :: none
• reviewed 2026-07-17T22:04:11.994Z sha 6103836 :: needs maintainer review before merge. :: none

[anagnorisis2peripeteia]

@clawsweeper re-review

Added real production-endpoint proof: the fix runs under wrangler dev (workerd + real D1 + real native-auth) and rejects a stopping native-VNC session with HTTP 409 at the pre-mint guard, where base returns 500 (reaches the mint). Reproducible transcript + seed in the linked gist.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29616745279
• Updated: 2026-07-17T22:07:26.480Z