Skip to content

Latest commit

 

History

History
305 lines (239 loc) · 17.4 KB

File metadata and controls

305 lines (239 loc) · 17.4 KB
reconciled_at 2026-05-09 12:46:54 UTC
current_state closed
number 1867
repository openclaw/clawhub
type pull_request
title feat: Device Flow auth for headless environments
url openclaw/clawhub#1867
state_at_review open
item_created_at 2026-04-28 20:22:06 UTC
item_updated_at 2026-05-05 03:48:21 UTC
author LumenFromTheFuture
author_association NONE
labels
reviewed_at 2026-05-07 14:14:35 UTC
main_sha b8e5486f63e1a1901385695a5ce7a52a5c9d7797
pull_head_sha c77aa4028c497a610d940d3617ee780cda21557c
latest_release v0.1.0
latest_release_sha 8216c73c9bd4e7befe12081e3e38a8333fbce46b
fixed_release unknown
fixed_sha unknown
fixed_at unknown
fixed_pr_url unknown
fixed_pr_number unknown
fixed_pr_title unknown
fixed_pr_merged_at unknown
fixed_pr_sha unknown
fixed_pr_confidence unknown
fixed_pr_source unknown
review_policy 9f214239696c0cc2
review_model gpt-5.5
review_reasoning_effort high
review_sandbox danger-full-access
review_service_tier default
review_prompt_chars 71533
review_static_prompt_chars 33412
review_context_chars 36858
review_schema_chars 14081
review_additional_prompt_chars 0
review_context_elapsed_ms 4913
review_codex_elapsed_ms 195401
review_mode propose
review_status complete
local_checkout_access verified
item_snapshot_hash 37d6b39d8de95131f5f815f0b9ff8ce2a57ac82191f10ebcd097403a87f47562
close_comment_sha256 none
review_comment_sha256 none
review_comment_id unknown
review_comment_url unknown
decision keep_open
close_reason none
confidence high
action_taken kept_open
work_candidate manual_review
work_confidence high
work_priority high
work_status manual_review
work_reason_sha256 16bf94ec061a1f910d97400e22488b219cb891340365a1b87997b903b85f1c8b
work_prompt_sha256 none
work_cluster_refs
work_validation
bun run format:check
bun run lint
bun run test
bunx tsc -p packages/clawhub/tsconfig.json --noEmit
bunx convex dev --once
work_likely_files
packages/clawhub/src/cli.ts
packages/clawhub/src/cli/commands/auth.ts
packages/clawhub/src/deviceAuth.ts
packages/clawhub/src/deviceAuth.test.ts
packages/clawhub/src/schema/routes.ts
convex/http.ts
convex/tokens.ts
src/routes/cli/auth.tsx
src/routes/cli/-auth.test.tsx
docs/auth.md
CHANGELOG.md
item_category feature
reproduction_status source_reproducible
reproduction_confidence high
requires_new_feature true
requires_new_config_option true
requires_product_decision true
real_behavior_proof_status missing
real_behavior_proof_evidence_kind none
real_behavior_proof_needs_contributor_action true

Type: pull_request

URL: openclaw/clawhub#1867

Author: LumenFromTheFuture

Author association: NONE

Labels: none

Created at: Apr 28, 2026, 20:22 UTC

Updated at: May 5, 2026, 03:48 UTC

Reviewed against: b8e5486f63e1

Codex review: model gpt-5.5, reasoning high

Latest release at review time: v0.1.0 (8216c73c9bd4)

Fixed in: not determined

Decision

Keep open: kept open

Confidence: high

Action taken: kept_open

Summary

Keep open. Current main does not implement device-flow auth, and this PR is still a client-only slice that exposes a public login mode before the required server endpoints, verification UI, security controls, changelog, and real behavior proof exist.

What This Changes

Adds a --device login option to the ClawHub CLI, client-side device-code request/polling helpers, and unit tests for the helper.

Best Possible Solution

Land device flow as an end-to-end auth feature covering CLI behavior, backend device-code issuance and token exchange, an authenticated verification page, docs, changelog, and regression coverage.

Reproduction Assessment

Yes for the review findings from source inspection: the PR head exposes --device and posts to /cli/device/code and /cli/device/token, while current main has no matching routes or verification page. I did not run the flow live because the missing server surface is clear in source.

Solution Assessment

No. The client helper is a useful slice, but exposing the flag before backend issuance, polling exchange, verification UI, and security controls land is not the narrowest maintainable solution.

Review Findings

Overall correctness: patch is incorrect

Overall confidence: 0.92

Full review comments:

  • [P2] Add the device endpoints before exposing the flag: packages/clawhub/src/cli.ts:136
    • body: The new --device option sends users into a flow that posts to /cli/device/code and /cli/device/token, but the branch does not add matching HTTP routes or a verification page. Merging this as-is would expose a login mode that cannot complete against the shipped app.
    • confidence: 0.94
  • [P3] Validate device timing fields before polling: packages/clawhub/src/deviceAuth.ts:73-75
    • body: requestDeviceCode accepts responses where expires_in or interval are missing or non-numeric. cmdDeviceLogin then uses those values for display, timeout, and sleep math, so a malformed server response can produce NaN timing instead of a clear protocol error.
    • confidence: 0.84
  • [P3] Add the required changelog entry: packages/clawhub/src/cli.ts:136
    • body: This adds a user-facing CLI login mode, which is a feature under the repo policy, but the branch does not update CHANGELOG.md. Add an Unreleased entry so CLI release notes cover the new auth path.
    • confidence: 0.87

Security Review

Status: needs_attention

Summary: The diff introduces a new token login flow while leaving the server-side device-code issuance and exchange controls outside the PR.

Concerns:

  • [medium] Server-side device-flow controls are missing: packages/clawhub/src/cli.ts:136
    • body: The branch enables a CLI flow that depends on device-code issuance and token exchange, but the endpoints and verification UI where expiry, single-use codes, polling limits, user binding, replay resistance, rate limiting, and token scope would be enforced are outside this PR.
    • confidence: 0.88

Real Behavior Proof

Status: missing

Evidence kind: none

Needs contributor action: true

Summary: The PR body and comments show unit-test intent and a code-review fix, but no redacted terminal output, logs, screenshot, recording, or linked artifact demonstrating a real device login after the change.

Work Candidate

Candidate: manual_review

Confidence: high

Priority: high

Status: manual_review

Reason: Human maintainer/security review is needed because the remaining work is end-to-end token issuance design, not a narrow PR-branch repair.

Cluster refs:

Likely files:

  • packages/clawhub/src/cli.ts
  • packages/clawhub/src/cli/commands/auth.ts
  • packages/clawhub/src/deviceAuth.ts
  • packages/clawhub/src/deviceAuth.test.ts
  • packages/clawhub/src/schema/routes.ts
  • convex/http.ts
  • convex/tokens.ts
  • src/routes/cli/auth.tsx
  • src/routes/cli/-auth.test.tsx
  • docs/auth.md
  • CHANGELOG.md

Validation:

  • bun run format:check
  • bun run lint
  • bun run test
  • bunx tsc -p packages/clawhub/tsconfig.json --noEmit
  • bunx convex dev --once

Evidence

  • PR exposes device login: The PR head adds --device to both top-level login and nested auth login, making the new flow user-facing.
    • file: packages/clawhub/src/cli.ts:136
    • command: gh api repos/openclaw/clawhub/contents/packages/clawhub/src/cli.ts?ref=c77aa4028c497a610d940d3617ee780cda21557c --jq .content | base64 -d | nl -ba | sed -n '130,175p'
    • sha: c77aa4028c49
  • PR dispatches the new flow: cmdLoginFlow now routes options.device to cmdDeviceLogin, which requests a device code and later stores the returned access token via the existing login path.
  • PR calls new device endpoints: The helper posts to /cli/device/code for code issuance and /cli/device/token for polling, but those routes are not added by the PR.
    • file: packages/clawhub/src/deviceAuth.ts:46
    • command: gh api repos/openclaw/clawhub/contents/packages/clawhub/src/deviceAuth.ts?ref=c77aa4028c497a610d940d3617ee780cda21557c --jq .content | base64 -d | nl -ba | sed -n '35,140p'
    • sha: c77aa4028c49
  • Prior review issue addressed: The second PR commit parses the polling JSON response once and reuses it, resolving the earlier double body-read finding from Greptile and Vercel review comments.
  • Server-side requirements remain outside PR: The PR body explicitly says the server still needs POST /cli/device/code, POST /cli/device/token, and a verification page where users enter the code.
    • command: gh pr view 1867 --repo openclaw/clawhub --json body,files,commits,comments,reviews,closingIssuesReferences
    • sha: c77aa4028c49
  • Current main lacks device implementation: A targeted search on current main found no --device, /cli/device, device_code, verification_uri, authorization_pending, or slow_down implementation under the app, CLI, Convex, docs, or changelog surfaces.
    • command: rg -n "Device Flow|device flow|--device|device_code|authorization_pending|slow_down|verification_uri|/cli/device|clawhub login --device" CHANGELOG.md docs packages src convex
    • sha: b8e5486f63e1
  • Current main registers existing HTTP routes only: convex/http.ts registers the current API and legacy CLI routes, with no device-code issuance or token polling route.
  • Current route constants omit device routes: The schema route constants expose existing API and legacy CLI routes but no /cli/device/code or /cli/device/token route.
  • Current web auth is loopback-based: The existing browser auth helper builds /cli/auth URLs with a loopback redirect and binds a local callback server on 127.0.0.1; it is not a user-code verification flow.
  • Current CLI auth page creates a loopback token: The /cli/auth route consumes redirect_uri and state, creates an API token with api.tokens.create, and redirects the token back to the loopback URI.
  • Current docs describe token paste for headless auth: The auth docs still document headless login as creating a token in the web UI and running clawhub login --token clh_....
  • No changelog entry: The current changelog contains no device-flow or --device entry, and the PR diff does not modify CHANGELOG.md.
    • file: CHANGELOG.md:1
    • command: nl -ba CHANGELOG.md | sed -n '1,120p'; gh pr diff 1867 --repo openclaw/clawhub --patch
    • sha: b8e5486f63e1
  • No real behavior proof found: The PR body and comments mention unit tests and the double-read fix, but do not include after-fix terminal output, logs, screenshots, recordings, or linked artifacts showing a real device login completing.
    • command: gh pr view 1867 --repo openclaw/clawhub --json body,comments,reviews,statusCheckRollup
    • sha: c77aa4028c49
  • Feature history owners checked: Git history points the current CLI auth, browser auth, CLI auth route, token, and HTTP route surfaces primarily to Peter Steinberger and Vincent Koc, with recent adjacent CLI work by Patrick Erichsen.
    • command: git log --all --date=short --pretty=format:'%h %ad %an %s' -- packages/clawhub/src/cli/commands/auth.ts packages/clawhub/src/browserAuth.ts src/routes/cli/auth.tsx convex/tokens.ts docs/auth.md convex/http.ts packages/clawhub/src/cli.ts | sed -n '1,80p'; git blame -L 11,43 -- packages/clawhub/src/cli/commands/auth.ts
    • sha: b8e5486f63e1

Likely Related People

  • steipete: introduced adjacent auth behavior
    • reason: Git history shows Peter Steinberger introduced the CLI token auth and browser login flow and has recent auth/CLI API work touching the same token and HTTP route boundaries.
    • confidence: high
    • commits: 7dcd2432750aa8484889a4ca0011a429e09b4467, 57ef9ace63646c828d9c12d9e288eba9dd8e9856, a0133cd6c93bc1a62469c4cf92d885e948925291, e3de833359bff9168a9003b4394b4540494c579e, 5019f2a78abf4191ae06dcf481c3d616225dad74, 6925ec761c56ef1e1bb857c8cd9e5c1431b69f8e
    • files: packages/clawhub/src/cli/commands/auth.ts, packages/clawhub/src/browserAuth.ts, src/routes/cli/auth.tsx, convex/tokens.ts, convex/http.ts, docs/auth.md
  • vincentkoc: recent maintainer
    • reason: Current-main blame and recent history tie Vincent Koc to cmdLoginFlow, the /cli/auth route, token write paths, and surrounding CLI/API maintenance.
    • confidence: medium
    • commits: f53b49041a78582c4bd21ca163d347d8016fa34c, 9e15c5a6fa19, dfc0d540d8d0, 0fe234e
    • files: packages/clawhub/src/cli/commands/auth.ts, packages/clawhub/src/browserAuth.ts, src/routes/cli/auth.tsx, convex/tokens.ts, convex/http.ts
  • Patrick-Erichsen: recent adjacent CLI maintainer
    • reason: Recent history shows Patrick Erichsen working in the CLI command registry and adjacent command surfaces that a new auth flag would need to fit into, though the core auth logic trail is stronger for the other owners.
    • confidence: low
    • commits: 0c7607bd6408bc23fed12597da953fa8b99fb06f, 3618d29
    • files: packages/clawhub/src/cli.ts, docs/cli.md

Risks / Open Questions

  • Merging this client-only branch would expose a --device login mode that cannot complete against the shipped server.
  • Device-flow token issuance needs maintainer security design for code entropy, expiry, single-use semantics, polling limits, user binding, replay resistance, rate limiting, token scope, and auditability.
  • External PR proof is absent; unit tests alone do not show the changed behavior working in a real setup.

Close Comment

No close comment posted.

GitHub Snapshot

  • comments: 4
  • timeline events: 29
  • related items: 2
  • PR files: 4
  • PR commits: 2

Review Telemetry

  • prompt chars: 71533
  • static prompt chars: 33412
  • context chars: 36858
  • schema chars: 14081
  • additional prompt chars: 0
  • context collection ms: 4913
  • Codex review ms: 195401