feat(auth): add device authorization login

[miguel-heygen]

Summary

heygen auth login --device now supports attended remote-terminal sign-in without copying an API key or relying on a loopback browser callback. The existing interactive picker, browser OAuth, and non-interactive API-key behavior remain unchanged.

Security and persistence

• Device login is explicit and refuses CI or non-TTY execution.
• RFC 8628 polling waits before the first request, handles pending and slowdown responses, and has bounded time and response sizes.
• Verification URLs must be HTTPS or loopback HTTP, and server errors never echo the device bearer secret.
• /v3/users/me must accept the minted access token before anything is written locally.
• Verification and persistence failures revoke both minted tokens.
• The verified OAuth session, friendly identity, and API-key replacement are committed atomically while preserving foreign credential fields.
• DEV endpoints, client ID, resource, clock, and polling are injectable for exact-SHA certification.

Test plan

• make test — all Go packages passed
• make lint with golangci-lint v2.11.4 — 0 issues

End-to-end DEV certification is pending the unmerged EF endpoint and Pacific consent UI.

---

[somanshreddy]

Independent review — LGTM. Traced the RFC 8628 device flow with a security lens; it's well-built.

Poll loop is correct. PollDeviceToken: ""→return token, authorization_pending→continue, slow_down→delay += 5s, default→terminal DeviceAuthorizationError. Matches RFC 8628 §3.5. Interval floors to 5s when the server sends ≤0; delay caps at maxDevicePollDelay (60s); IssuedAt is captured before the request (accurate token expiry).

Expiry is conservative and correct. deadline = Now() + min(expiresIn, maxDeviceLifetime), and the loop bails with expired_token when Now()+delay would cross the deadline — so it never polls past expiry. Context-cancelable Sleep.

Security posture is solid. Verification URI restricted to HTTPS (loopback HTTP only for tests) — blocks a malicious authz server from phishing the user to http://. Errors are redacted (decodeRedactedDeviceError drops error_description, so a buggy/hostile server can't inject content into CLI output). device_code/user_code are documented as never logged or in errors. Responses are LimitReader(64KB)-bounded. Token persisted 0600 in a 0700 dir, preserving a co-located api_key.

Parity with the HF-CLI companion (#2836) holds on the load-bearing bits: slow_down +5s, HTTPS-only verification URI, redacted errors, secret hygiene. Nothing blocking.

— Somu

[james-russo-rames-d-jusso]

🟢 LGTM from my side — this is a careful, well-scoped RFC 8628 implementation. The credential-minting boundary invariants the PR body promises hold up under a trace: identity verification gates persistence, both minted tokens are revoked on any verification-or-write failure via a deduped access+refresh sweep, the credential file goes through writeCredentialsFile's temp+rename atomic write, and the jsonCredentials.extra bag preserves foreign top-level and per-sub-object keys so a hyperframes-written OAuth block round-trips unmolested. Bounded reads (64 KiB on device/token, 1 MiB on /v3/users/me), redacted 400/401 handling that drops error_description on principle, and a fixed-vocabulary analytics mapper on the RFC 6749 §5.2 error codes — all in the right places.

Leaving this as a comment — Miguel/team should merge; concerns below are worth logging but don't gate.

Peer scan

Somu (somanshreddy) posted an independent COMMENTED review at HEAD 1933258 about four minutes before mine (review). They traced the poll loop, expiry math, URI scheme restriction, error redaction, and file mode — all correct at HEAD. I'm not re-deriving those; they check out. Somu also stated "Parity with the HF-CLI companion (#2836) holds on the load-bearing bits." My cross-repo grep against heygen-com/hyperframes#2836 surfaces two implementation asymmetries Somu's review didn't reach into; neither is load-bearing security, but they're worth logging.

Hardest look — the credential-minting-boundary checklist

BREADTH — Scope on the device-authorization request is oauth.DefaultScopes = "openid profile email" (internal/auth/oauth/oauth.go:25), which is appropriately narrow. Resource is ctx.configProvider.BaseURL() (default https://api.heygen.com) trimmed of trailing /. The client does not verify that the server-returned scope matches what was requested — an IdP that returns a broader scope is silently accepted and persisted. Standard practice for OAuth clients, and the server is authoritative for scope grants regardless; flagging for completeness, not as a defect.

ALLOWLIST — Production endpoints are compile-time constants (internal/auth/oauth/oauth.go:32-36). The injection surface for DEV certification is the HEYGEN_OAUTH_CLIENT_ID / HEYGEN_OAUTH_DEVICE_URL / HEYGEN_OAUTH_TOKEN_URL / HEYGEN_OAUTH_REVOKE_URL env vars, read unconditionally in deviceLoginConfigFromEnvironment (cmd/heygen/auth_login.go:544-560) — no --dev gate, no printed warning when any of the four is set. Threat model is bounded (an attacker who can set env vars in a user's shell has broader local access already, and the returned verification_uri goes through safeVerificationURI), but a one-line stderr warning of the form "using non-default OAuth endpoints from HEYGEN_OAUTH_… env" whenever any of the four is non-empty would (a) make a mis-set env var visible to the user during audit and (b) give ops a printed breadcrumb if someone runs heygen auth login --device inside a compromised shell. Non-blocking; a defensive nicety.

ASYMMETRY (client-side vs. server-side) — This is the one bit I'd like to flag more clearly:

• PollDeviceToken clamps intervalSeconds <= 0 → 5 (internal/auth/oauth/device.go:112-114) but does not enforce a minimum for positive values. A server (or a compromised endpoint) sending interval: 1 produces one-second polling until slow_down bumps it. RequestDeviceAuthorization has the same shape at line 90-92.
• The companion hyperframes-CLI PR (heygen-com/hyperframes#2836) explicitly declares MIN_DEVICE_POLL_SECONDS = 5 and applies Math.max(Math.ceil(interval), MIN_DEVICE_POLL_SECONDS) on the same field. So the same misconfigured IdP produces 5-second polling on the TypeScript CLI and 1-second polling on the Go CLI.

Not a spec violation on either side — RFC 8628 §3.5 requires the client to be no faster than the server-specified interval; it doesn't forbid slower. But it's a client-side hardening that HF-CLI has and heygen-cli doesn't. A one-line if intervalSeconds < 5 { intervalSeconds = 5 } in both locations closes the divergence. Maps to the sibling-asymmetry lens in past reviews — same surface, two implementations, one slightly softer.

maxDevicePollDelay = 60s and maxDeviceLifetime = 30m do match the sibling's MAX_DEVICE_POLL_SECONDS / MAX_DEVICE_FLOW_SECONDS.

ATTRIBUTION (/v3/users/me pre-write) — Ordering is correct: PollDeviceToken → verifyCurrentUser → SaveVerifiedOAuthSession (cmd/heygen/auth_login.go:625-668). The identity returned by /v3/users/me is used to populate the friendly-display block (username/email/first_name/last_name) and drives the PostHog identifyAccount key. verifyCurrentUser is deliberately stricter than lookupCurrentUser — empty access token rejected, non-200 → error, envelope's data field must be non-null before decoding — so a token that authenticates but returns an empty identity envelope fails closed. Good.

Revoke path on failure (revokeMintedDeviceTokens, cmd/heygen/auth_login.go:733-745) dedupes access+refresh to avoid a double-revoke when they're the same string, and calls client.RevokeToken which swallows network errors per RFC 7009 best-effort semantics. Runs on both identity-verify-fail and credential-persist-fail branches. Clean.

Findings

Concerns (non-blocking)

• [cross-repo asymmetry] MIN device-poll interval enforced in heygen-com/hyperframes#2836, not here. internal/auth/oauth/device.go:112-114 + line 90-92. Sibling clamps Math.max(interval, 5s) on positive values; here only <= 0 defaults to 5. A misconfigured or hostile IdP that returns interval: 1 gets 1-second polling from this CLI and 5-second polling from HF-CLI. Suggested one-liner: replace if intervalSeconds <= 0 { intervalSeconds = 5 } with if intervalSeconds < 5 { intervalSeconds = 5 } in both call sites.

• [cross-repo asymmetry] Loopback allowlist broader than sibling. safeVerificationURI, device.go:227 accepts any IP where net.ParseIP(host).IsLoopback() is true — that covers 127.0.0.1, any 127.x.x.x, and IPv6 ::1. Sibling accepts only literal "127.0.0.1" / "localhost". Both are safe (loopback is local), and heygen-cli's check is arguably more correct per RFC 8252. Flagging for parity awareness, not as a defect — if you'd rather both CLIs converge, easier direction is to widen the TS side to match the Go side.

• [defense-in-depth] HEYGEN_OAUTH_* env vars redirect the OAuth flow silently. cmd/heygen/auth_login.go:544-560. No --dev gate, no stderr breadcrumb. Consider printing "note: using non-default OAuth endpoints from environment" to stderr on runDeviceLogin entry when any of the four env vars is non-empty — makes a mis-set var visible to the user and gives ops a signal during forensic review.

• [minor] expires_in multiplication is safe by luck, not construction. device.go:118: lifetime := time.Duration(expiresInSeconds) * time.Second. If a server returns expires_in > ~9.22e9 (nearly 300 years), this overflows int64 nanoseconds on 64-bit builds. The overflow currently manifests as a small/negative lifetime, which trips the Now().Add(delay).Before(deadline) guard on the very first iteration and returns expired_token — SAFE fallthrough. Still, capping expiresInSeconds to int(maxDeviceLifetime/time.Second) before the multiplication would remove reliance on overflow semantics for correctness.

Nits

• device.go:227 calls net.ParseIP(host) twice on the loopback path (net.ParseIP(host) != nil && net.ParseIP(host).IsLoopback()). Bind once:

  if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() {
      return true
  }
  return false

• Ordering nit: the resource field is trimmed of trailing / in both device.go:56 and auth_login.go:609. Belt-and-suspenders; harmless but one of them is redundant.

Questions

• The --device-code alias is registered as a hidden flag and shares the deviceMode || deviceCodeMode dispatch branch — is that alias for compatibility with an earlier iteration you were maintaining, or for something HF-CLI exposes? If it's not load-bearing anywhere, dropping it before merge would reduce the flag surface by one hidden entry.

What I didn't verify

• The unmerged EF device-authorization endpoint and the Pacific consent UI — PR body notes DEV certification is pending both, so end-to-end I can't reach past httptest fixtures. Once those land, worth re-running the flow against DEV to confirm the resource/scope round-trip against a real IdP.
• Concurrent heygen auth login --device invocations against the same credentials file. The atomic write via temp+rename in credentials_file.go:70-105 is race-safe for a single writer per rename, but two concurrent logins can still race on the read-modify-write window (loadCredentialsFile → mutate → writeCredentialsFile), where the later rename replaces the earlier one wholesale. heygen auth login isn't a workflow anyone runs concurrently on purpose, so this is an academic note rather than a finding.
• Windows-specific path/file-mode behavior for the credentials file (0o600 / 0o700) — CI passes test (windows-latest) at HEAD; taking that at face value.

Recommendation

Ship after Miguel eyeballs the MIN-interval divergence — either close it here with the one-liner in both locations to match #2836, or explicitly decide to leave heygen-cli looser and document why. Everything else is defense-in-depth / nit territory.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

Cross-repo followup: parity issues flagged on the HF-CLI sibling

I re-checked my HF-CLI reviewer's findings on hyperframes#2836 against this PR at HEAD 1933258b. Most don't apply on the Go side (Go's shared http.Client{Timeout: 30s} uniformly covers device-auth / poll / revoke, the interval default and malformed-store refusal are both correct here). Two land, one narrower than the TS sibling and one at parity.

1. user_code printed unvalidated — narrower ANSI-injection vector than the TS side. cmd/heygen/auth_login.go:622 does fmt.Fprintf(cmd.ErrOrStderr(), "Enter code: %s\n", authorization.UserCode) with no control-character check on UserCode. The response-shape check at internal/auth/oauth/device.go:86-89 only rejects empty. A hostile / MITM'd IdP that returns "user_code": "�[2K�[1Aphish" survives json.Unmarshal as a Go string containing the raw ESC byte, and printing it rewrites earlier terminal lines — same phishing primitive the HF sibling was flagged for, just via user_code instead of verification_uri.

   Good news: the verification_uri vector — which was the primary hit on the TS side — is closed on the Go side without additional code. safeVerificationURI at device.go:215-228 runs the raw string through net/url.Parse, and Go's stdlib rejects any byte < 0x20 or 0x7f at the top of parse() (stringContainsCTLByte). Only user_code needs the same treatment — a one-line helper at parity with isHeaderSafe in oauth.go:521 applied inside RequestDeviceAuthorization closes it.

2. PollDeviceToken treats HTTP 429 without a slow_down body as fatal. device.go:184-192 parses the non-2xx body for an error field, and if the body is empty / non-JSON / lacks error, it falls into decodeRedactedDeviceError and returns a fatal oauth: device endpoint returned HTTP 429. PollDeviceToken at device.go:136-139 propagates that out and terminates the poll loop — the caller sees a login failure even though the server was asking for backoff, not rejecting the grant. RFC 8628 §3.5 doesn't explicitly define 429 for the device-token endpoint, but a well-behaved client should back off on 429 regardless of body. Same defensible fix the HF sibling got: when resp.StatusCode == http.StatusTooManyRequests and the body doesn't carry an RFC-8628 error code, treat it as an implicit slow_down and continue the poll loop with the delay += 5s bump.

Skipped as not-applicable on Go: per-request timeouts (oauth.go:120 sets http.Client{Timeout: DefaultExchangeTimeout} = 30s on the shared client, exercised uniformly by device-auth, poll, and revoke); interval treated as required (device.go:90-92 correctly defaults <= 0 to 5, and the incomplete-response check at L86-89 omits Interval); corrupted-store handling (SaveVerifiedOAuthSession at oauth_store.go:23-60 and SaveOAuthTokens at :69-91 use byte-identical format != formatAbsent → refuse logic, and FileCredentialStore.Save matches for the api-key path); and the IDN / verification_uri_complete / Codespaces items the HF review mentioned only in passing (verification_uri_complete isn't parsed by the Go struct, so it's ignored; Codespaces users still get a TTY and land on the attended-terminal check at auth_login.go:580).

— Follow-up by Rames D Jusso

[miguel-heygen]

Addressed the remaining device-flow review findings at 5a5c72c. user_code now rejects terminal control characters before presentation, and a bare HTTP 429 is normalized to RFC 8628 slow_down so the poll delay increases by five seconds instead of failing. Added focused regressions for both paths. Verification: go test ./internal/auth/oauth/... and full go test ./... are green.

[miguel-heygen]

@somanshreddy @james-russo-rames-d-jusso The two follow-ups from the last pass are fixed at 5a5c72c: terminal-safe user_code validation and bare-429 slow_down handling, each with a regression. All CI is terminal-green and there are no unresolved threads. Could one of you give the fresh head an explicit approval when ready?

[somanshreddy]

Re-review @ 5a5c72cc9 — undrafted, 2 of 3 items fixed. One non-blocking item remains.

• user_code ANSI injection: fixed. safeUserCode (device.go:224, unicode.IsControl check) now validates UserCode at parse (:91) and rejects control/ESC bytes before the raw print at auth_login.go:622 — so a hostile endpoint's escape sequences can't reach the terminal. (verification_uri was already closed by the Go stdlib control-byte check + safeVerificationURI.)
• HTTP 429 without a slow_down body: fixed. device.go:195-197 now treats a bare 429 as slow_down backoff instead of fatal, preserving RFC 8628 behavior.
• Draft: cleared (isDraft: false), CI green (9/9).

🟡 Still open (non-blocking) — the MIN-poll-interval floor. Both :94-95 and :116-117 only default interval <= 0 → 5; a positive server value is used as-is, so a hostile/misconfigured IdP returning interval: 1 still gets 1s polling on the Go side. This is the parity one-liner from earlier — clamp the floor, e.g. if intervalSeconds < minDevicePollSeconds { intervalSeconds = minDevicePollSeconds } (const 5). HF-CLI #2836 landed the equivalent (max(interval, 5)), so this closes the last cross-surface divergence.

Everything else holds (verify-before-persist, revoke-on-failure, redacted errors, 64KB reads, 0600/0700 storage). Not blocking on that one-liner — but it's the only thing left before this is at parity with the hardened TS side.

— Somu

[miguel-heygen]

positive server interval values below 5 seconds are currently used as-is

Addressed at cf7b456: both authorization-response normalization and direct token polling now clamp intervals below five seconds to five seconds. The existing zero/negative default remains five seconds. Added regressions for interval: 1 on both paths.

Validation:

• go test ./internal/auth/oauth/... -count=1
• go test ./... -count=1
• go vet ./...
• gofmt and git diff --check

CI run 30878710526 is green across all nine checks, including pinned golangci-lint and Ubuntu/macOS/Windows tests.

[somanshreddy]

Re-review @ cf7b45635 — MIN-interval floor fixed; all findings now resolved. LGTM.

The floor is in at both sites: device.go:94 (if result.Interval < 5 { result.Interval = 5 }) and :116 (same in PollDeviceToken). Changed from <= 0 to < 5, so a hostile/misconfigured interval: 1 is now floored to 5s; the maxDevicePollDelay = 60s cap is unchanged, so the interval is bounded [5, 60] on both the initial and post-slow_down paths. You kept Go's silent-floor rather than the TS reject-on-invalid — that's the friendlier-default polarity we landed on (a user's own IdP sending a garbage interval shouldn't hard-fail their login), so this is the right call, not a divergence to close.

Full slate now resolved on this PR:

• ✅ undrafted, CI green (9/9)
• ✅ user_code control-char rejected at parse (safeUserCode / unicode.IsControl) before print
• ✅ bare HTTP 429 honored as slow_down backoff
• ✅ MIN-poll-interval floor (this commit)

Plus the invariants from the first pass hold (verify-before-persist, revoke-on-failure, redacted errors, 64KB reads, 0600/0700 storage). Nothing blocking from my side. (Stamp needs an authorized member — flagging for the merge-gate, not something I place.)

— Somu

[jrusso1020]

Approving at cf7b4563.

The floor fix is the right shape and it is enforced rather than merely asserted. Both sites moved from <= 0 to < 5: device.go:94 at parse time and :116 in PollDeviceToken. Clamping in both places is correct rather than redundant, since PollDeviceToken takes intervalSeconds as a parameter and is reachable independently of the parse path. The two new tests pin observable behavior, and TestPollDeviceTokenClampsIntervalToMinimum asserts the recorded sleep is 5s rather than checking an internal field, which is the stronger assertion. Reverting either site fails them.

The interval is now bounded [5, 60]. This change supplies the floor; maxDevicePollDelay (device.go:19) already supplied the cap, applied at the top of every poll-loop iteration including before the first sleep.

Verified at this head: all six required contexts green (test on ubuntu/macos/windows, lint, secrets, goreleaser-check), commit signature valid, and the branch is not behind main, which matters here because protection is strict: true.

One non-blocking note. The cap is a named constant, maxDevicePollDelay, while the floor is now a bare literal 5 at two call sites plus twice more in the tests. A minDevicePollDelay constant beside it would match this file's own idiom, keep the two sites from drifting, and mirror the TS side, which spells it MIN_DEVICE_POLL_SECONDS. Related: both new tests use interval: 1, so they pin the behavior but not the threshold. Changing < 5 to < 4 would leave both green; an interval: 4 case would close that.

Keeping Go's silent floor rather than the TS side's hard reject on an explicitly invalid interval is a defensible call and I would not change it. A user's own IdP sending a garbage interval should not fail their login.

Review by Rames Jusso