From 56cc450bba3c918de0a28c635abf1ff768489a8c Mon Sep 17 00:00:00 2001 From: Factory Agent Date: Fri, 23 Oct 2026 14:32:42 +0700 Subject: [PATCH 1/3] Add Copilot device-code login to the CLI Add codexbar config login --provider copilot [--json]. It prints the GitHub verification URL and user code (never the token), polls with the existing backoff (persistent slow_down +5s), and saves the account through the same finish step the desktop app uses. Move the shared start and finish logic into providers/copilot/login.rs so the Tauri shell and the CLI call one save path. Other providers get a friendly error before any network call. Add CODEXBAR_GITHUB_BASE_URL to point both GitHub roots at a fake server (https or loopback http only). Add unit tests for success, slow_down, expired and denied flows, stdout without the token, and the CLI parser. Update docs/CLI.md. --- .../src-tauri/src/commands/mod.rs | 4 +- .../src-tauri/src/commands/system.rs | 113 +-- docs/CLI.md | 37 + rust/src/cli/config.rs | 119 +++ rust/src/cli/config/tests.rs | 162 +++++ rust/src/providers/copilot/api.rs | 26 +- rust/src/providers/copilot/device_flow.rs | 684 +++++++++++------- rust/src/providers/copilot/endpoints.rs | 163 +++++ rust/src/providers/copilot/login.rs | 317 ++++++++ rust/src/providers/copilot/mod.rs | 4 + rust/src/providers/copilot/test_server.rs | 139 ++++ 11 files changed, 1393 insertions(+), 375 deletions(-) create mode 100644 rust/src/providers/copilot/endpoints.rs create mode 100644 rust/src/providers/copilot/login.rs create mode 100644 rust/src/providers/copilot/test_server.rs diff --git a/apps/desktop-tauri/src-tauri/src/commands/mod.rs b/apps/desktop-tauri/src-tauri/src/commands/mod.rs index 1e4ac159a8a..37dda19b1ca 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/mod.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/mod.rs @@ -3,11 +3,11 @@ use std::sync::Mutex; use codexbar::core::{ FetchContext, ManualEmptyCookiePolicy, ProviderAccountData, ProviderFetchResult, ProviderId, - ProviderMetadata, RateWindow, SourceMode, TokenAccount, TokenAccountStore, - instantiate_provider, + ProviderMetadata, RateWindow, SourceMode, TokenAccountStore, instantiate_provider, }; use codexbar::locale; use codexbar::login::{self, LoginOutcome, LoginPhase}; +use codexbar::providers::copilot::login::{finish_device_login, start_device_login}; use codexbar::providers::copilot::{CopilotApi, device_flow::CopilotDeviceFlow}; use codexbar::secure_file::{self, SecureFileStatus}; use codexbar::settings::{ diff --git a/apps/desktop-tauri/src-tauri/src/commands/system.rs b/apps/desktop-tauri/src-tauri/src/commands/system.rs index 146561cd447..133a1ab5b05 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/system.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/system.rs @@ -351,57 +351,14 @@ async fn run_cli_provider_login( async fn run_copilot_device_login(app: &tauri::AppHandle) -> Result<(), String> { let flow = CopilotDeviceFlow::new(); - let device = flow - .start_flow() - .await - .map_err(|e| format!("GitHub device login failed: {e}"))?; + let device = start_device_login(&flow).await.map_err(|e| e.to_string())?; open_url_in_browser(device.verification_url_to_open())?; - let token = flow - .wait_for_token(&device.device_code, device.interval, device.expires_in) - .await - .map_err(|e| format!("GitHub device login failed: {e}"))?; - let api = CopilotApi::new(); - let identity = api.fetch_identity_with_token(&token, None).await.ok(); - let plan = api - .fetch_usage_with_token(&token, None, None) - .await - .ok() - .and_then(|usage| usage.login_method); - - let login = identity.as_ref().map(|identity| identity.login.clone()); - let label = match (login.as_deref(), plan.as_deref()) { - (Some(login), Some(plan)) => format!("{login} ({plan})"), - (Some(login), None) => login.to_string(), - (None, Some(plan)) => plan.to_string(), - (None, None) => "GitHub Copilot".to_string(), - }; - let store = TokenAccountStore::new(); - let mut data = store - .load_provider(ProviderId::Copilot) - .map_err(|e| e.to_string())?; - let identity_id = identity.as_ref().and_then(|identity| identity.id); - let existing_index = - find_existing_copilot_account(&api, &data.accounts, identity_id, login.as_deref()).await; - - if let Some(index) = existing_index { - data.accounts[index].token = token; - data.accounts[index].label = label; - data.accounts[index].external_identifier = identity_id.map(copilot_external_identifier); - data.set_active(index); - } else { - let mut account = TokenAccount::new(label, token); - account.external_identifier = identity_id.map(copilot_external_identifier); - account.mark_used(); - data.add_account(account); - data.set_active(data.accounts.len().saturating_sub(1)); - } - - store - .save_provider(ProviderId::Copilot, &data) + finish_device_login(&flow, &api, &store, &device) + .await .map_err(|e| e.to_string())?; let _ = app.emit( @@ -411,59 +368,6 @@ async fn run_copilot_device_login(app: &tauri::AppHandle) -> Result<(), String> Ok(()) } -async fn find_existing_copilot_account( - api: &CopilotApi, - accounts: &[TokenAccount], - identity_id: Option, - login: Option<&str>, -) -> Option { - if let Some(identity_id) = identity_id - && let Some(index) = find_copilot_account_by_identity(accounts, identity_id) - { - return Some(index); - } - - let mut label_fallback = None; - for (index, account) in accounts - .iter() - .enumerate() - .filter(|(_, account)| account.external_identifier.is_none()) - { - if let Some(identity_id) = identity_id { - match api.fetch_identity_with_token(&account.token, None).await { - Ok(resolved_identity) if resolved_identity.id == Some(identity_id) => { - return Some(index); - } - Ok(resolved_identity) if resolved_identity.id.is_some() => continue, - _ => {} - } - } - - if label_fallback.is_none() && copilot_label_matches_login(account, login) { - label_fallback = Some(index); - } - } - - label_fallback -} - -fn find_copilot_account_by_identity(accounts: &[TokenAccount], identity_id: u64) -> Option { - let external_identifier = copilot_external_identifier(identity_id); - accounts.iter().position(|account| { - account.external_identifier.as_deref() == Some(external_identifier.as_str()) - }) -} - -fn copilot_external_identifier(identity_id: u64) -> String { - format!("github:user:{identity_id}") -} - -fn copilot_label_matches_login(account: &TokenAccount, login: Option<&str>) -> bool { - login.is_some_and(|login| { - account.label == login || account.label.starts_with(&format!("{login} (")) - }) -} - #[cfg(test)] mod tests { use super::*; @@ -489,15 +393,4 @@ mod tests { Some("https://openrouter.ai/activity") ); } - - #[test] - fn copilot_stored_identity_precedes_legacy_label_match() { - let legacy = TokenAccount::new("octocat (Pro)", "old-token"); - let mut identified = TokenAccount::new("Renamed account", "known-token"); - identified.external_identifier = Some(copilot_external_identifier(123)); - - let accounts = vec![legacy.clone(), identified]; - assert_eq!(find_copilot_account_by_identity(&accounts, 123), Some(1)); - assert!(copilot_label_matches_login(&legacy, Some("octocat"))); - } } diff --git a/docs/CLI.md b/docs/CLI.md index d3cec5e98e8..0e5f61a7231 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -346,6 +346,7 @@ codexbar config disable cursor --json codexbar config set-source --provider codex --source cli printf '%s' $env:OPENROUTER_API_KEY | codexbar config set-api-key -p openrouter --stdin codexbar config claude-code-credentials status --json +codexbar config login --provider copilot codexbar config preferences export --file C:\temp\codexbar-preferences.json codexbar config preferences import --file C:\temp\codexbar-preferences.json codexbar config validate @@ -361,6 +362,7 @@ codexbar config validate | `enable`, `disable` | Turns a provider on or off and saves the setting. | | `set-source` | Saves a provider's usage source (`--source auto\|web\|cli\|oauth`) without changing whether it is enabled. `auto` returns to the default. | | `set-api-key` | Stores a provider API key. `--stdin` reads it from stdin. `--no-enable` stores it without enabling the provider. | +| `login` | Signs in to GitHub Copilot with GitHub's device-code flow. Only `--provider copilot` is supported. See [Sign in to Copilot from the CLI](#sign-in-to-copilot-from-the-cli). | | `path` | Shows the configuration file paths. | | `preferences export`, `preferences import` | Exports or imports portable preferences. See [CONFIGURATION.md](./CONFIGURATION.md#portable-preferences). | | `claude-code-credentials` | `allow`, `deny` or `status`. | @@ -380,6 +382,41 @@ codexbar config validate `config preferences import` changes the saved settings. Restart a running CodexBar afterwards. +### Sign in to Copilot from the CLI + +```powershell +codexbar config login --provider copilot +codexbar config login --provider copilot --json +``` + +`config login` starts GitHub's device-code flow for GitHub Copilot. It prints the verification URL and a one-time code, then waits while you approve the code in your browser. The code expires after the lifetime GitHub returns, and the command stops when it does. + +Text output is two lines and then a confirmation: + +```text +https://github.com/login/device +ABCD-1234 +Connected +``` + +The token is never printed. Once you approve the code, the account is saved the same way the desktop app's Copilot sign-in saves it. The account is matched by GitHub identity, so signing in again updates the existing account instead of adding a second one. Its label is the GitHub login, with the plan in brackets when GitHub reports one. + +`--json` prints one JSON object per line, using upstream's field names: + +| Line | Shape | +| --- | --- | +| Code issued | `{"status":"authorization_pending","verificationURI":"https://github.com/login/device","userCode":"ABCD-1234","expiresIn":900}` | +| Approved | `{"status":"authorized"}` | +| Failed | `{"status":"failed","error":"GitHub device login failed: Access denied by user"}`, and the command exits with an error | + +Polling follows the interval GitHub returns. Each `slow_down` response adds 5 seconds for the rest of the login. GitHub's `access_denied` and `expired_token` responses end the login with a friendly error, and nothing is saved. + +Other providers fail before any network call: `Login is only supported for copilot. Use --provider copilot.` + +The login always signs in on github.com. This port has no GitHub Enterprise host option for the login flow, and the desktop app's sign-in behaves the same way. + +For tests and proof runs, `CODEXBAR_GITHUB_BASE_URL` replaces both GitHub roots with one origin. The device-code requests (`/login/device/code`, `/login/oauth/access_token`) and the identity and usage requests (`/user`, `/copilot_internal/user`) all go there. Only `https://` URLs, or loopback `http://` URLs (`127.0.0.1`, `localhost`, `[::1]`), with no path, query or credentials are accepted. Anything else is ignored with a warning, and the default GitHub roots are used. The variable also applies to the desktop app's Copilot sign-in and usage fetches, so unset it for normal use. + ### Separate config file (`CODEXBAR_CONFIG`) ```powershell diff --git a/rust/src/cli/config.rs b/rust/src/cli/config.rs index 7a0e81d4be2..6185681ddde 100755 --- a/rust/src/cli/config.rs +++ b/rust/src/cli/config.rs @@ -5,10 +5,14 @@ use clap::{Parser, Subcommand}; use serde::Serialize; use serde::de::DeserializeOwned; +use std::io::Write; use std::path::Path; use super::usage::OutputFormat; use crate::core::{ProviderId, SourceMode, TokenAccountStore, instantiate_provider}; +use crate::providers::copilot::CopilotApi; +use crate::providers::copilot::device_flow::CopilotDeviceFlow; +use crate::providers::copilot::login::{finish_device_login, start_device_login}; use crate::settings::{ApiKeys, ManualCookies, PreferencesDocument, Settings}; /// Arguments for the config command @@ -74,6 +78,13 @@ pub enum ConfigCommand { #[arg(long = "no-enable")] no_enable: bool, }, + /// Log in to a provider through its own device flow (GitHub Copilot only) + Login { + #[command(flatten)] + target: ConfigProviderArg, + #[command(flatten)] + output: ConfigOutputArgs, + }, /// Show configuration file paths Path, /// Export or import portable preferences (no secrets, no machine state) @@ -202,6 +213,7 @@ pub async fn run(args: ConfigArgs) -> anyhow::Result<()> { stdin, no_enable, } => set_api_key(target.name(), api_key.as_deref(), stdin, !no_enable).await, + ConfigCommand::Login { target, output } => login(target.name(), output).await, ConfigCommand::Path => show_paths().await, ConfigCommand::Preferences { action } => transfer_preferences(action), ConfigCommand::ClaudeCodeCredentials { action, output } => { @@ -771,6 +783,113 @@ fn resolve_api_key_input(api_key: Option<&str>, read_from_stdin: bool) -> anyhow Ok(value) } +/// One `config login` event (upstream `ConfigLoginEvent`). It carries the +/// verification URL and user code only; the token is never part of it. +#[derive(Debug, Serialize)] +struct ConfigLoginEvent { + status: &'static str, + #[serde(rename = "verificationURI", skip_serializing_if = "Option::is_none")] + verification_uri: Option, + #[serde(rename = "userCode", skip_serializing_if = "Option::is_none")] + user_code: Option, + #[serde(rename = "expiresIn", skip_serializing_if = "Option::is_none")] + expires_in: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error: Option, +} + +impl ConfigLoginEvent { + fn status(status: &'static str) -> Self { + Self { + status, + verification_uri: None, + user_code: None, + expires_in: None, + error: None, + } + } +} + +/// Run `config login`. Only GitHub Copilot has a CLI login flow. +async fn login(provider: &str, output: ConfigOutputArgs) -> anyhow::Result<()> { + let id = parse_provider(provider)?; + if id != ProviderId::Copilot { + anyhow::bail!("Login is only supported for copilot. Use --provider copilot."); + } + let flow = CopilotDeviceFlow::new(); + let api = CopilotApi::new(); + let store = TokenAccountStore::new(); + run_copilot_login( + &flow, + &api, + &store, + output.is_json(), + &mut std::io::stdout().lock(), + ) + .await +} + +/// Runs the Copilot device login and writes progress to `out`. In JSON mode +/// each event is one line; in text mode the URL and code are two lines. A +/// failure is also reported as a `failed` JSON event before the error returns. +async fn run_copilot_login( + flow: &CopilotDeviceFlow, + api: &CopilotApi, + store: &TokenAccountStore, + json: bool, + out: &mut impl Write, +) -> anyhow::Result<()> { + match login_steps(flow, api, store, json, out).await { + Ok(()) => write_login_event(out, json, &ConfigLoginEvent::status("authorized")), + Err(error) => { + if json { + let mut failed = ConfigLoginEvent::status("failed"); + failed.error = Some(error.to_string()); + write_login_event(out, json, &failed)?; + } + Err(error) + } + } +} + +async fn login_steps( + flow: &CopilotDeviceFlow, + api: &CopilotApi, + store: &TokenAccountStore, + json: bool, + out: &mut impl Write, +) -> anyhow::Result<()> { + let device = start_device_login(flow).await?; + write_login_event( + out, + json, + &ConfigLoginEvent { + status: "authorization_pending", + verification_uri: Some(device.verification_uri.clone()), + user_code: Some(device.user_code.clone()), + expires_in: Some(device.expires_in), + error: None, + }, + )?; + finish_device_login(flow, api, store, &device).await?; + Ok(()) +} + +fn write_login_event( + out: &mut impl Write, + json: bool, + event: &ConfigLoginEvent, +) -> anyhow::Result<()> { + if json { + writeln!(out, "{}", serde_json::to_string(event)?)?; + } else if let (Some(url), Some(code)) = (&event.verification_uri, &event.user_code) { + writeln!(out, "{url}\n{code}")?; + } else if event.status == "authorized" { + writeln!(out, "Connected")?; + } + Ok(()) +} + /// Show configuration file paths async fn show_paths() -> anyhow::Result<()> { println!("Configuration paths:"); diff --git a/rust/src/cli/config/tests.rs b/rust/src/cli/config/tests.rs index b13e1bbef9c..3a19105544f 100644 --- a/rust/src/cli/config/tests.rs +++ b/rust/src/cli/config/tests.rs @@ -458,3 +458,165 @@ fn sanitize_settings_for_dump_show_secrets_keeps_raw() { assert_eq!(out["api_key"], "keep-me"); assert_eq!(out["nested"]["token"], "also-keep"); } + +#[test] +fn config_login_parses_the_positional_or_provider_option() { + let ConfigCommand::Login { target, output } = + parse_config(&["login", "--provider", "copilot", "--json"]).unwrap() + else { + panic!("expected login"); + }; + assert_eq!(target.name(), "copilot"); + assert!(output.is_json()); + + let ConfigCommand::Login { target, output } = + parse_config(&["login", "-p", "copilot", "--format", "json", "--pretty"]).unwrap() + else { + panic!("expected login"); + }; + assert_eq!(target.name(), "copilot"); + assert!(output.is_json()); + + let ConfigCommand::Login { target, output } = parse_config(&["login", "copilot"]).unwrap() + else { + panic!("expected login"); + }; + assert_eq!(target.name(), "copilot"); + assert!(!output.is_json()); + + assert!(parse_config(&["login"]).is_err(), "provider is required"); +} + +#[tokio::test] +async fn config_login_rejects_other_providers_before_any_io() { + let error = super::login("cursor", ConfigOutputArgs::default()) + .await + .expect_err("cursor has no CLI login"); + assert_eq!( + error.to_string(), + "Login is only supported for copilot. Use --provider copilot." + ); +} + +mod copilot_login_output { + use super::super::{CopilotApi, CopilotDeviceFlow, TokenAccountStore, run_copilot_login}; + use crate::core::ProviderId; + use crate::providers::copilot::endpoints::GitHubEndpoints; + use crate::providers::copilot::test_server::{FakeGitHub, Reply, ok}; + + const DEVICE_JSON: &str = r#"{"device_code":"device-example","user_code":"ABCD-1234","verification_uri":"https://github.com/login/device","expires_in":900,"interval":0}"#; + const TOKEN_JSON: &str = + r#"{"access_token":"gho_example_token","token_type":"bearer","scope":"read:user"}"#; + const USER_JSON: &str = r#"{"login":"octocat","id":42}"#; + const USAGE_JSON: &str = r#"{"copilot_plan":"individual","quota_snapshots":{}}"#; + + fn fake(token_replies: Vec) -> FakeGitHub { + FakeGitHub::start(vec![ + ("/login/device/code", vec![ok(DEVICE_JSON)]), + ("/login/oauth/access_token", token_replies), + ("/user", vec![ok(USER_JSON)]), + ("/copilot_internal/user", vec![ok(USAGE_JSON)]), + ]) + } + + /// The returned `TempDir` must stay alive while the store is read back. + async fn run( + fake: &FakeGitHub, + json: bool, + ) -> ( + anyhow::Result<()>, + String, + TokenAccountStore, + tempfile::TempDir, + ) { + let endpoints = GitHubEndpoints::with_origin(&fake.origin).expect("loopback origin"); + let flow = CopilotDeviceFlow::with_endpoints(&endpoints); + let api = CopilotApi::with_endpoints(endpoints); + let dir = tempfile::tempdir().expect("tempdir"); + let store = TokenAccountStore::with_path(dir.path().join("token-accounts.json")); + let mut out = Vec::new(); + let result = run_copilot_login(&flow, &api, &store, json, &mut out).await; + (result, String::from_utf8(out).expect("utf8"), store, dir) + } + + #[tokio::test] + async fn text_output_shows_url_and_code_then_connected_and_never_the_token() { + let fake = fake(vec![ok(TOKEN_JSON)]); + let (result, out, store, _dir) = run(&fake, false).await; + + result.expect("login succeeds"); + assert_eq!( + out, + "https://github.com/login/device\nABCD-1234\nConnected\n" + ); + assert!(!out.contains("gho_example_token")); + + let data = store.load_provider(ProviderId::Copilot).expect("load"); + assert_eq!(data.accounts[0].token, "gho_example_token"); + } + + #[tokio::test] + async fn json_output_is_upstream_shaped_events_without_the_token() { + let fake = fake(vec![ + ok(r#"{"error":"authorization_pending"}"#), + ok(TOKEN_JSON), + ]); + let (result, out, _store, _dir) = run(&fake, true).await; + + result.expect("login succeeds"); + assert!(!out.contains("gho_example_token")); + let events: Vec = out + .lines() + .map(|line| serde_json::from_str(line).expect("one JSON object per line")) + .collect(); + assert_eq!(events.len(), 2); + assert_eq!(events[0]["status"], "authorization_pending"); + assert_eq!( + events[0]["verificationURI"], + "https://github.com/login/device" + ); + assert_eq!(events[0]["userCode"], "ABCD-1234"); + assert_eq!(events[0]["expiresIn"], 900); + assert!(events[0].get("error").is_none()); + assert_eq!(events[1], serde_json::json!({ "status": "authorized" })); + } + + #[tokio::test] + async fn denied_json_reports_a_failed_event_and_returns_the_error() { + let fake = fake(vec![ok(r#"{"error":"access_denied"}"#)]); + let (result, out, store, _dir) = run(&fake, true).await; + + let error = result.expect_err("denied"); + assert_eq!( + error.to_string(), + "GitHub device login failed: Access denied by user" + ); + let failed: serde_json::Value = + serde_json::from_str(out.lines().last().expect("event")).expect("failed event is JSON"); + assert_eq!(failed["status"], "failed"); + assert_eq!( + failed["error"], + "GitHub device login failed: Access denied by user" + ); + assert!(!out.contains("gho_example_token")); + assert!( + store + .load_provider(ProviderId::Copilot) + .expect("load") + .accounts + .is_empty() + ); + } + + #[tokio::test] + async fn expired_text_login_prints_the_code_and_returns_an_error() { + let fake = fake(vec![ok(r#"{"error":"expired_token"}"#)]); + let (result, out, _store, _dir) = run(&fake, false).await; + + assert_eq!( + result.expect_err("expired").to_string(), + "GitHub device login failed: Device code expired" + ); + assert_eq!(out, "https://github.com/login/device\nABCD-1234\n"); + } +} diff --git a/rust/src/providers/copilot/api.rs b/rust/src/providers/copilot/api.rs index 5bc805ee1b6..6109cd2960d 100755 --- a/rust/src/providers/copilot/api.rs +++ b/rust/src/providers/copilot/api.rs @@ -4,6 +4,7 @@ //! path is app-managed device OAuth/token accounts; legacy API key and Windows //! Credential Manager tokens remain supported as fallbacks. +use super::endpoints::GitHubEndpoints; use crate::core::{NamedRateWindow, ProviderError, RateWindow, UsageSnapshot}; use chrono::{DateTime, Utc}; use serde::Deserialize; @@ -41,17 +42,32 @@ pub struct GitHubIdentity { /// Copilot API client. pub struct CopilotApi { client: reqwest::Client, + endpoints: GitHubEndpoints, } impl CopilotApi { pub fn new() -> Self { + Self::with_endpoints(GitHubEndpoints::from_env()) + } + + pub(crate) fn with_endpoints(endpoints: GitHubEndpoints) -> Self { let client = crate::core::credentialed_http_client_builder() .use_rustls_tls() .timeout(std::time::Duration::from_secs(30)) .build() .unwrap_or_else(|_| reqwest::Client::new()); - Self { client } + Self { client, endpoints } + } + + /// Builds an API URL. The enterprise host picks the API host, unless an + /// endpoint override is set; then the override root replaces it. + fn api_url(&self, github_host: Option<&str>, path: &str) -> String { + if self.endpoints.overridden { + format!("{}{path}", self.endpoints.api_root) + } else { + github_api_url(github_host, path) + } } /// Fetch usage information from Copilot API with an already-resolved @@ -66,7 +82,7 @@ impl CopilotApi { ) -> Result { let request = self .client - .get(copilot_usage_url(github_host)) + .get(self.api_url(github_host, COPILOT_USAGE_PATH)) .header("Authorization", format!("token {}", token.trim())) .header("Accept", "application/json") .header("Editor-Version", "vscode/1.96.2") @@ -87,7 +103,7 @@ impl CopilotApi { ) -> Result { let request = self .client - .get(github_api_url(github_host, GITHUB_USER_PATH)) + .get(self.api_url(github_host, GITHUB_USER_PATH)) .header("Authorization", format!("token {}", token.trim())) .header("Accept", "application/vnd.github+json") .header("User-Agent", "Win-CodexBar") @@ -627,10 +643,6 @@ fn github_api_url(github_host: Option<&str>, path: &str) -> String { format!("https://{host}{path}") } -fn copilot_usage_url(github_host: Option<&str>) -> String { - github_api_url(github_host, COPILOT_USAGE_PATH) -} - fn normalized_api_host(github_host: Option<&str>) -> String { let host = github_host .map(str::trim) diff --git a/rust/src/providers/copilot/device_flow.rs b/rust/src/providers/copilot/device_flow.rs index 2c2d874253d..850420182e5 100755 --- a/rust/src/providers/copilot/device_flow.rs +++ b/rust/src/providers/copilot/device_flow.rs @@ -1,256 +1,428 @@ -//! Copilot Device Flow OAuth -//! -//! Implements GitHub's OAuth Device Flow for authenticating GitHub Copilot. -//! This allows the user to authorize the app via browser while the app polls for the token. - -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::time::Duration; - -/// VS Code's GitHub OAuth Client ID (public) -const CLIENT_ID: &str = "Iv1.b507a08c87ecfe98"; -const SCOPES: &str = "read:user"; - -/// Device code response from GitHub -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct DeviceCodeResponse { - /// The device verification code - pub device_code: String, - /// The user-visible code to enter - pub user_code: String, - /// The URL where the user should enter the code - pub verification_uri: String, - /// Pre-filled verification URL (includes user_code in query string) - #[serde(default)] - pub verification_uri_complete: Option, - /// How long until the codes expire (seconds) - pub expires_in: u32, - /// Polling interval in seconds - pub interval: u32, -} - -impl DeviceCodeResponse { - /// Returns the best URL to open — prefers the pre-filled complete URI - pub fn verification_url_to_open(&self) -> &str { - self.verification_uri_complete - .as_deref() - .unwrap_or(&self.verification_uri) - } -} - -/// Access token response from GitHub -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct AccessTokenResponse { - /// The OAuth access token - pub access_token: String, - /// Token type (usually "bearer") - pub token_type: String, - /// Granted scopes - pub scope: String, -} - -/// Error response from GitHub OAuth -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct OAuthErrorResponse { - pub error: String, - #[serde(default)] - pub error_description: Option, -} - -/// Device flow errors -#[derive(Debug, Clone)] -pub enum DeviceFlowError { - /// Network or HTTP error - Network(String), - /// Authorization is still pending (keep polling) - AuthorizationPending, - /// Need to slow down polling - SlowDown, - /// The device code expired - ExpiredToken, - /// Access was denied by the user - AccessDenied, - /// Generic OAuth error - OAuthError(String), - /// JSON parsing error - ParseError(String), -} - -impl std::fmt::Display for DeviceFlowError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - DeviceFlowError::Network(e) => write!(f, "Network error: {}", e), - DeviceFlowError::AuthorizationPending => write!(f, "Authorization pending"), - DeviceFlowError::SlowDown => write!(f, "Polling too fast, slowing down"), - DeviceFlowError::ExpiredToken => write!(f, "Device code expired"), - DeviceFlowError::AccessDenied => write!(f, "Access denied by user"), - DeviceFlowError::OAuthError(e) => write!(f, "OAuth error: {}", e), - DeviceFlowError::ParseError(e) => write!(f, "Parse error: {}", e), - } - } -} - -impl std::error::Error for DeviceFlowError {} - -/// Copilot Device Flow OAuth client -pub struct CopilotDeviceFlow { - client: reqwest::Client, -} - -impl CopilotDeviceFlow { - pub fn new() -> Self { - Self { - client: reqwest::Client::new(), - } - } - - /// Request a device code from GitHub - /// - /// The returned response contains: - /// - `user_code`: Display this to the user - /// - `verification_uri`: User should visit this URL - /// - `device_code`: Used for polling (don't show to user) - /// - `interval`: How often to poll in seconds - pub async fn request_device_code(&self) -> Result { - let mut params = HashMap::new(); - params.insert("client_id", CLIENT_ID); - params.insert("scope", SCOPES); - - let response = self - .client - .post("https://github.com/login/device/code") - .header("Accept", "application/json") - .form(¶ms) - .send() - .await - .map_err(|e| DeviceFlowError::Network(e.to_string()))?; - - if !response.status().is_success() { - return Err(DeviceFlowError::Network(format!( - "HTTP {}", - response.status() - ))); - } - - let text = response - .text() - .await - .map_err(|e| DeviceFlowError::Network(e.to_string()))?; - - serde_json::from_str(&text).map_err(|e| DeviceFlowError::ParseError(e.to_string())) - } - - /// Poll for the access token - /// - /// Call this in a loop with the specified interval until you get a token or error. - pub async fn poll_for_token(&self, device_code: &str) -> Result { - let mut params = HashMap::new(); - params.insert("client_id", CLIENT_ID); - params.insert("device_code", device_code); - params.insert("grant_type", "urn:ietf:params:oauth:grant-type:device_code"); - - let response = self - .client - .post("https://github.com/login/oauth/access_token") - .header("Accept", "application/json") - .form(¶ms) - .send() - .await - .map_err(|e| DeviceFlowError::Network(e.to_string()))?; - - let text = response - .text() - .await - .map_err(|e| DeviceFlowError::Network(e.to_string()))?; - - // First, try to parse as an error response - if let Ok(error) = serde_json::from_str::(&text) { - return match error.error.as_str() { - "authorization_pending" => Err(DeviceFlowError::AuthorizationPending), - "slow_down" => Err(DeviceFlowError::SlowDown), - "expired_token" => Err(DeviceFlowError::ExpiredToken), - "access_denied" => Err(DeviceFlowError::AccessDenied), - _ => Err(DeviceFlowError::OAuthError( - error.error_description.unwrap_or(error.error), - )), - }; - } - - // Try to parse as success response - let token_response: AccessTokenResponse = - serde_json::from_str(&text).map_err(|e| DeviceFlowError::ParseError(e.to_string()))?; - - Ok(token_response.access_token) - } - - /// Convenience method to run the full device flow - /// - /// Returns the device code response for displaying to user, and a future that - /// will resolve to the access token when the user completes authorization. - pub async fn start_flow(&self) -> Result { - self.request_device_code().await - } - - /// Poll until token is obtained or flow fails - /// - /// Respects the interval and handles slow_down requests automatically. - pub async fn wait_for_token( - &self, - device_code: &str, - initial_interval: u32, - expires_in: u32, - ) -> Result { - let mut interval = initial_interval; - let start = std::time::Instant::now(); - let timeout = Duration::from_secs(u64::from(expires_in)); - - loop { - // Check if we've exceeded the timeout - if start.elapsed() >= timeout { - return Err(DeviceFlowError::ExpiredToken); - } - - // Wait the required interval - tokio::time::sleep(Duration::from_secs(u64::from(interval))).await; - - match self.poll_for_token(device_code).await { - Ok(token) => return Ok(token), - Err(DeviceFlowError::AuthorizationPending) => { - // Continue polling - continue; - } - Err(DeviceFlowError::SlowDown) => { - // Increase interval by 5 seconds as per OAuth spec - interval += 5; - continue; - } - Err(e) => return Err(e), - } - } - } -} - -impl Default for CopilotDeviceFlow { - fn default() -> Self { - Self::new() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_device_flow_new() { - let _flow = CopilotDeviceFlow::new(); - } - - #[test] - fn test_error_display() { - let err = DeviceFlowError::AuthorizationPending; - assert_eq!(err.to_string(), "Authorization pending"); - - let err = DeviceFlowError::ExpiredToken; - assert_eq!(err.to_string(), "Device code expired"); - } -} +//! Copilot Device Flow OAuth +//! +//! Implements GitHub's OAuth Device Flow for authenticating GitHub Copilot. +//! This allows the user to authorize the app via browser while the app polls for the token. + +use super::endpoints::GitHubEndpoints; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::time::Duration; + +/// VS Code's GitHub OAuth Client ID (public) +const CLIENT_ID: &str = "Iv1.b507a08c87ecfe98"; +const SCOPES: &str = "read:user"; + +/// Device code response from GitHub +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DeviceCodeResponse { + /// The device verification code + pub device_code: String, + /// The user-visible code to enter + pub user_code: String, + /// The URL where the user should enter the code + pub verification_uri: String, + /// Pre-filled verification URL (includes user_code in query string) + #[serde(default)] + pub verification_uri_complete: Option, + /// How long until the codes expire (seconds) + pub expires_in: u32, + /// Polling interval in seconds + pub interval: u32, +} + +impl DeviceCodeResponse { + /// Returns the best URL to open — prefers the pre-filled complete URI + pub fn verification_url_to_open(&self) -> &str { + self.verification_uri_complete + .as_deref() + .unwrap_or(&self.verification_uri) + } +} + +/// Access token response from GitHub +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AccessTokenResponse { + /// The OAuth access token + pub access_token: String, + /// Token type (usually "bearer") + pub token_type: String, + /// Granted scopes + pub scope: String, +} + +/// Error response from GitHub OAuth +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct OAuthErrorResponse { + pub error: String, + #[serde(default)] + pub error_description: Option, +} + +/// Device flow errors +#[derive(Debug, Clone)] +pub enum DeviceFlowError { + /// Network or HTTP error + Network(String), + /// Authorization is still pending (keep polling) + AuthorizationPending, + /// Need to slow down polling + SlowDown, + /// The device code expired + ExpiredToken, + /// Access was denied by the user + AccessDenied, + /// Generic OAuth error + OAuthError(String), + /// JSON parsing error + ParseError(String), +} + +impl std::fmt::Display for DeviceFlowError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DeviceFlowError::Network(e) => write!(f, "Network error: {}", e), + DeviceFlowError::AuthorizationPending => write!(f, "Authorization pending"), + DeviceFlowError::SlowDown => write!(f, "Polling too fast, slowing down"), + DeviceFlowError::ExpiredToken => write!(f, "Device code expired"), + DeviceFlowError::AccessDenied => write!(f, "Access denied by user"), + DeviceFlowError::OAuthError(e) => write!(f, "OAuth error: {}", e), + DeviceFlowError::ParseError(e) => write!(f, "Parse error: {}", e), + } + } +} + +impl std::error::Error for DeviceFlowError {} + +/// Copilot Device Flow OAuth client +pub struct CopilotDeviceFlow { + client: reqwest::Client, + web_root: String, +} + +impl CopilotDeviceFlow { + pub fn new() -> Self { + Self::with_endpoints(&GitHubEndpoints::from_env()) + } + + pub(crate) fn with_endpoints(endpoints: &GitHubEndpoints) -> Self { + Self { + client: reqwest::Client::new(), + web_root: endpoints.web_root.clone(), + } + } + + /// Request a device code from GitHub + /// + /// The returned response contains: + /// - `user_code`: Display this to the user + /// - `verification_uri`: User should visit this URL + /// - `device_code`: Used for polling (don't show to user) + /// - `interval`: How often to poll in seconds + pub async fn request_device_code(&self) -> Result { + let mut params = HashMap::new(); + params.insert("client_id", CLIENT_ID); + params.insert("scope", SCOPES); + + let response = self + .client + .post(format!("{}/login/device/code", self.web_root)) + .header("Accept", "application/json") + .form(¶ms) + .send() + .await + .map_err(|e| DeviceFlowError::Network(e.to_string()))?; + + if !response.status().is_success() { + return Err(DeviceFlowError::Network(format!( + "HTTP {}", + response.status() + ))); + } + + let text = response + .text() + .await + .map_err(|e| DeviceFlowError::Network(e.to_string()))?; + + serde_json::from_str(&text).map_err(|e| DeviceFlowError::ParseError(e.to_string())) + } + + /// Poll for the access token + /// + /// Call this in a loop with the specified interval until you get a token or error. + pub async fn poll_for_token(&self, device_code: &str) -> Result { + let mut params = HashMap::new(); + params.insert("client_id", CLIENT_ID); + params.insert("device_code", device_code); + params.insert("grant_type", "urn:ietf:params:oauth:grant-type:device_code"); + + let response = self + .client + .post(format!("{}/login/oauth/access_token", self.web_root)) + .header("Accept", "application/json") + .form(¶ms) + .send() + .await + .map_err(|e| DeviceFlowError::Network(e.to_string()))?; + + let text = response + .text() + .await + .map_err(|e| DeviceFlowError::Network(e.to_string()))?; + + // First, try to parse as an error response + if let Ok(error) = serde_json::from_str::(&text) { + return match error.error.as_str() { + "authorization_pending" => Err(DeviceFlowError::AuthorizationPending), + "slow_down" => Err(DeviceFlowError::SlowDown), + "expired_token" => Err(DeviceFlowError::ExpiredToken), + "access_denied" => Err(DeviceFlowError::AccessDenied), + _ => Err(DeviceFlowError::OAuthError( + error.error_description.unwrap_or(error.error), + )), + }; + } + + // Try to parse as success response + let token_response: AccessTokenResponse = + serde_json::from_str(&text).map_err(|e| DeviceFlowError::ParseError(e.to_string()))?; + + Ok(token_response.access_token) + } + + /// Convenience method to run the full device flow + /// + /// Returns the device code response for displaying to user, and a future that + /// will resolve to the access token when the user completes authorization. + pub async fn start_flow(&self) -> Result { + self.request_device_code().await + } + + /// Poll until token is obtained or flow fails + /// + /// Respects the interval and handles slow_down requests automatically. + pub async fn wait_for_token( + &self, + device_code: &str, + initial_interval: u32, + expires_in: u32, + ) -> Result { + self.wait_for_token_with_sleep(device_code, initial_interval, expires_in, |delay| { + tokio::time::sleep(delay) + }) + .await + } + + /// `wait_for_token` with the delay injected, so tests can record the + /// backoff schedule without real waits. + async fn wait_for_token_with_sleep( + &self, + device_code: &str, + initial_interval: u32, + expires_in: u32, + mut sleep: S, + ) -> Result + where + S: FnMut(Duration) -> Fut, + Fut: std::future::Future, + { + let mut interval = initial_interval; + let start = std::time::Instant::now(); + let timeout = Duration::from_secs(u64::from(expires_in)); + + loop { + // Check if we've exceeded the timeout + if start.elapsed() >= timeout { + return Err(DeviceFlowError::ExpiredToken); + } + + // Wait the required interval + sleep(Duration::from_secs(u64::from(interval))).await; + + match self.poll_for_token(device_code).await { + Ok(token) => return Ok(token), + Err(DeviceFlowError::AuthorizationPending) => { + // Continue polling + continue; + } + Err(DeviceFlowError::SlowDown) => { + // Increase interval by 5 seconds as per OAuth spec + interval += 5; + continue; + } + Err(e) => return Err(e), + } + } + } +} + +impl Default for CopilotDeviceFlow { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_device_flow_new() { + let _flow = CopilotDeviceFlow::new(); + } + + #[test] + fn test_error_display() { + let err = DeviceFlowError::AuthorizationPending; + assert_eq!(err.to_string(), "Authorization pending"); + + let err = DeviceFlowError::ExpiredToken; + assert_eq!(err.to_string(), "Device code expired"); + } + + use crate::providers::copilot::test_server::{FakeGitHub, Reply, ok}; + use std::future::ready; + + const DEVICE_JSON: &str = r#"{"device_code":"device-example","user_code":"ABCD-1234","verification_uri":"https://github.com/login/device","expires_in":900,"interval":1}"#; + const TOKEN_JSON: &str = + r#"{"access_token":"gho_example_token","token_type":"bearer","scope":"read:user"}"#; + const PENDING: &str = r#"{"error":"authorization_pending"}"#; + const SLOW_DOWN: &str = r#"{"error":"slow_down"}"#; + + fn flow_for(fake: &FakeGitHub) -> CopilotDeviceFlow { + let endpoints = GitHubEndpoints::with_origin(&fake.origin).expect("loopback origin"); + CopilotDeviceFlow::with_endpoints(&endpoints) + } + + fn token_fake(replies: Vec) -> FakeGitHub { + FakeGitHub::start(vec![ + ("/login/device/code", vec![ok(DEVICE_JSON)]), + ("/login/oauth/access_token", replies), + ]) + } + + #[tokio::test] + async fn device_code_then_poll_succeeds_after_pending() { + let fake = token_fake(vec![ok(PENDING), ok(TOKEN_JSON)]); + let flow = flow_for(&fake); + + let device = flow.start_flow().await.expect("device code"); + assert_eq!(device.user_code, "ABCD-1234"); + assert_eq!( + device.verification_url_to_open(), + "https://github.com/login/device" + ); + + let mut sleeps = Vec::new(); + let token = flow + .wait_for_token_with_sleep( + &device.device_code, + device.interval, + device.expires_in, + |d| { + sleeps.push(d); + ready(()) + }, + ) + .await + .expect("token"); + + assert_eq!(token, "gho_example_token"); + assert_eq!(sleeps, vec![Duration::from_secs(1), Duration::from_secs(1)]); + assert_eq!( + fake.requests(), + vec![ + "POST /login/device/code", + "POST /login/oauth/access_token", + "POST /login/oauth/access_token", + ] + ); + } + + #[tokio::test] + async fn slow_down_adds_five_seconds_and_keeps_the_longer_interval() { + let fake = token_fake(vec![ok(SLOW_DOWN), ok(PENDING), ok(TOKEN_JSON)]); + let flow = flow_for(&fake); + + let mut sleeps = Vec::new(); + let token = flow + .wait_for_token_with_sleep("device-example", 1, 900, |d| { + sleeps.push(d); + ready(()) + }) + .await + .expect("token"); + + assert_eq!(token, "gho_example_token"); + // slow_down raises the interval from 1s to 6s, and the next pending + // poll keeps 6s instead of resetting to the original 1s. + assert_eq!( + sleeps, + vec![ + Duration::from_secs(1), + Duration::from_secs(6), + Duration::from_secs(6), + ] + ); + } + + #[tokio::test] + async fn repeated_slow_down_accumulates() { + let fake = token_fake(vec![ok(SLOW_DOWN), ok(SLOW_DOWN), ok(TOKEN_JSON)]); + let flow = flow_for(&fake); + + let mut sleeps = Vec::new(); + flow.wait_for_token_with_sleep("device-example", 1, 900, |d| { + sleeps.push(d); + ready(()) + }) + .await + .expect("token"); + + assert_eq!( + sleeps, + vec![ + Duration::from_secs(1), + Duration::from_secs(6), + Duration::from_secs(11), + ] + ); + } + + #[tokio::test] + async fn expired_token_response_ends_the_flow() { + let fake = token_fake(vec![ok(r#"{"error":"expired_token"}"#)]); + let flow = flow_for(&fake); + + let result = flow + .wait_for_token_with_sleep("device-example", 1, 900, |_| ready(())) + .await; + + assert!(matches!(result, Err(DeviceFlowError::ExpiredToken))); + } + + #[tokio::test] + async fn denied_authorization_ends_the_flow() { + let fake = token_fake(vec![ok(r#"{"error":"access_denied"}"#)]); + let flow = flow_for(&fake); + + let result = flow + .wait_for_token_with_sleep("device-example", 1, 900, |_| ready(())) + .await; + + assert!(matches!(result, Err(DeviceFlowError::AccessDenied))); + } + + #[tokio::test] + async fn zero_lifetime_stops_before_polling() { + let fake = token_fake(vec![ok(TOKEN_JSON)]); + let flow = flow_for(&fake); + + let result = flow + .wait_for_token_with_sleep("device-example", 1, 0, |_| ready(())) + .await; + + assert!(matches!(result, Err(DeviceFlowError::ExpiredToken))); + assert!(fake.requests().is_empty()); + } +} diff --git a/rust/src/providers/copilot/endpoints.rs b/rust/src/providers/copilot/endpoints.rs new file mode 100644 index 00000000000..3cb50c260e0 --- /dev/null +++ b/rust/src/providers/copilot/endpoints.rs @@ -0,0 +1,163 @@ +//! GitHub endpoint roots for the Copilot device login and usage API. +//! +//! Production always uses `github.com` for the device flow and +//! `api.github.com` for identity and usage. `CODEXBAR_GITHUB_BASE_URL` points +//! both roots at one origin so a proof run or test can use a fake server. Only +//! `https://` and loopback `http://` origins are accepted, so a token is never +//! sent to a remote plain-HTTP host. Anything else is ignored with a warning. + +use reqwest::Url; + +/// Environment override for both GitHub roots (proof runs and fake servers). +pub(crate) const GITHUB_BASE_URL_ENV: &str = "CODEXBAR_GITHUB_BASE_URL"; + +const GITHUB_WEB_ROOT: &str = "https://github.com"; +const GITHUB_API_ROOT: &str = "https://api.github.com"; + +/// The GitHub roots a Copilot login or usage request talks to. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct GitHubEndpoints { + /// Root for `/login/device/code` and `/login/oauth/access_token`. + pub(crate) web_root: String, + /// Root for `/user` and `/copilot_internal/user`. + pub(crate) api_root: String, + /// True when an override replaced the production roots. + pub(crate) overridden: bool, +} + +impl GitHubEndpoints { + /// Production endpoints on github.com and api.github.com. + pub(crate) fn github() -> Self { + Self { + web_root: GITHUB_WEB_ROOT.to_string(), + api_root: GITHUB_API_ROOT.to_string(), + overridden: false, + } + } + + /// Both roots on one origin, such as `http://127.0.0.1:8080`. + pub(crate) fn with_origin(origin: &str) -> Option { + let origin = validated_origin(origin)?; + Some(Self { + web_root: origin.clone(), + api_root: origin, + overridden: true, + }) + } + + /// Reads `CODEXBAR_GITHUB_BASE_URL`; falls back to GitHub when it is unset + /// or rejected. + pub(crate) fn from_env() -> Self { + Self::from_override(std::env::var(GITHUB_BASE_URL_ENV).ok().as_deref()) + } + + fn from_override(raw: Option<&str>) -> Self { + let Some(raw) = raw.map(str::trim).filter(|raw| !raw.is_empty()) else { + return Self::github(); + }; + match Self::with_origin(raw) { + Some(endpoints) => endpoints, + None => { + tracing::warn!( + "Ignoring {GITHUB_BASE_URL_ENV}: use an https:// URL or a loopback http:// URL with no path, query, or credentials" + ); + Self::github() + } + } + } +} + +/// Accepts `https://host[:port]` or loopback `http://` (127.0.0.1, ::1, +/// localhost) with an empty or `/` path and no query, fragment, or userinfo. +/// Returns the origin without a trailing slash. +fn validated_origin(raw: &str) -> Option { + let url = Url::parse(raw.trim()).ok()?; + if !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + || !(url.path().is_empty() || url.path() == "/") + { + return None; + } + let loopback = matches!(url.host_str(), Some("127.0.0.1" | "localhost" | "[::1]")); + let allowed = match url.scheme() { + "https" => url.host_str().is_some(), + "http" => loopback, + _ => false, + }; + if !allowed { + return None; + } + let host = url.host_str()?; + let port = url + .port() + .map(|port| format!(":{port}")) + .unwrap_or_default(); + Some(format!("{}://{host}{port}", url.scheme())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn production_endpoints_are_github() { + let endpoints = GitHubEndpoints::from_override(None); + assert_eq!(endpoints, GitHubEndpoints::github()); + assert!(!endpoints.overridden); + } + + #[test] + fn blank_override_uses_github() { + assert_eq!( + GitHubEndpoints::from_override(Some(" ")), + GitHubEndpoints::github() + ); + } + + #[test] + fn accepts_https_and_loopback_http_origins() { + assert_eq!( + validated_origin("https://github.example.com/").as_deref(), + Some("https://github.example.com") + ); + assert_eq!( + validated_origin("http://127.0.0.1:4010").as_deref(), + Some("http://127.0.0.1:4010") + ); + assert_eq!( + validated_origin("http://localhost:9").as_deref(), + Some("http://localhost:9") + ); + } + + #[test] + fn rejects_remote_plain_http_and_unsafe_forms() { + for raw in [ + "http://github.example.com", + "ftp://127.0.0.1", + "https://user:pass@github.example.com", + "https://github.example.com/api", + "https://github.example.com?x=1", + "https://github.example.com#frag", + "not a url", + ] { + assert_eq!(validated_origin(raw), None, "{raw}"); + } + } + + #[test] + fn rejected_override_falls_back_to_github() { + let endpoints = GitHubEndpoints::from_override(Some("http://attacker.example.com")); + assert_eq!(endpoints, GitHubEndpoints::github()); + } + + #[test] + fn accepted_override_replaces_both_roots() { + let endpoints = GitHubEndpoints::from_override(Some("http://127.0.0.1:4010")); + assert_eq!(endpoints.web_root, "http://127.0.0.1:4010"); + assert_eq!(endpoints.api_root, "http://127.0.0.1:4010"); + assert!(endpoints.overridden); + } +} diff --git a/rust/src/providers/copilot/login.rs b/rust/src/providers/copilot/login.rs new file mode 100644 index 00000000000..d375c4aa819 --- /dev/null +++ b/rust/src/providers/copilot/login.rs @@ -0,0 +1,317 @@ +//! Finish step for the GitHub Copilot device-code login. +//! +//! The desktop shell and `codexbar config login --provider copilot` both call +//! these functions, so an account saved from either surface is the same: it is +//! labelled from the GitHub identity and plan, matched to an existing account +//! by stable identity, and written to the token-account store. + +use super::CopilotApi; +use super::device_flow::{CopilotDeviceFlow, DeviceCodeResponse, DeviceFlowError}; +use crate::core::{ProviderId, TokenAccount, TokenAccountError, TokenAccountStore}; + +/// Why a Copilot device login did not finish. +#[derive(Debug, thiserror::Error)] +pub enum CopilotLoginError { + #[error("GitHub device login failed: {0}")] + Flow(#[from] DeviceFlowError), + #[error("{0}")] + Store(#[from] TokenAccountError), +} + +/// The account a successful login saved. The token is never included. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SavedCopilotAccount { + pub label: String, + pub login: Option, + pub plan: Option, + pub identity_id: Option, +} + +/// Asks GitHub for a device code. The caller shows `verification_uri` and +/// `user_code` to the user. +pub async fn start_device_login( + flow: &CopilotDeviceFlow, +) -> Result { + Ok(flow.start_flow().await?) +} + +/// Polls until the user approves the device code, then saves the account. +/// Polling uses the device flow's backoff, including persistent `slow_down`. +pub async fn finish_device_login( + flow: &CopilotDeviceFlow, + api: &CopilotApi, + store: &TokenAccountStore, + device: &DeviceCodeResponse, +) -> Result { + let token = flow + .wait_for_token(&device.device_code, device.interval, device.expires_in) + .await?; + save_device_token(api, store, token).await +} + +async fn save_device_token( + api: &CopilotApi, + store: &TokenAccountStore, + token: String, +) -> Result { + let identity = api.fetch_identity_with_token(&token, None).await.ok(); + let plan = api + .fetch_usage_with_token(&token, None, None) + .await + .ok() + .and_then(|usage| usage.login_method); + + let login = identity.as_ref().map(|identity| identity.login.clone()); + let label = match (login.as_deref(), plan.as_deref()) { + (Some(login), Some(plan)) => format!("{login} ({plan})"), + (Some(login), None) => login.to_string(), + (None, Some(plan)) => plan.to_string(), + (None, None) => "GitHub Copilot".to_string(), + }; + + let mut data = store.load_provider(ProviderId::Copilot)?; + let identity_id = identity.as_ref().and_then(|identity| identity.id); + let existing_index = + find_existing_copilot_account(api, &data.accounts, identity_id, login.as_deref()).await; + + let saved = SavedCopilotAccount { + label: label.clone(), + login: login.clone(), + plan, + identity_id, + }; + + if let Some(index) = existing_index { + data.accounts[index].token = token; + data.accounts[index].label = label; + data.accounts[index].external_identifier = identity_id.map(copilot_external_identifier); + data.set_active(index); + } else { + let mut account = TokenAccount::new(label, token); + account.external_identifier = identity_id.map(copilot_external_identifier); + account.mark_used(); + data.add_account(account); + data.set_active(data.accounts.len().saturating_sub(1)); + } + + store.save_provider(ProviderId::Copilot, &data)?; + Ok(saved) +} + +async fn find_existing_copilot_account( + api: &CopilotApi, + accounts: &[TokenAccount], + identity_id: Option, + login: Option<&str>, +) -> Option { + if let Some(identity_id) = identity_id + && let Some(index) = find_copilot_account_by_identity(accounts, identity_id) + { + return Some(index); + } + + let mut label_fallback = None; + for (index, account) in accounts + .iter() + .enumerate() + .filter(|(_, account)| account.external_identifier.is_none()) + { + if let Some(identity_id) = identity_id { + match api.fetch_identity_with_token(&account.token, None).await { + Ok(resolved_identity) if resolved_identity.id == Some(identity_id) => { + return Some(index); + } + Ok(resolved_identity) if resolved_identity.id.is_some() => continue, + _ => {} + } + } + + if label_fallback.is_none() && copilot_label_matches_login(account, login) { + label_fallback = Some(index); + } + } + + label_fallback +} + +fn find_copilot_account_by_identity(accounts: &[TokenAccount], identity_id: u64) -> Option { + let external_identifier = copilot_external_identifier(identity_id); + accounts.iter().position(|account| { + account.external_identifier.as_deref() == Some(external_identifier.as_str()) + }) +} + +fn copilot_external_identifier(identity_id: u64) -> String { + format!("github:user:{identity_id}") +} + +fn copilot_label_matches_login(account: &TokenAccount, login: Option<&str>) -> bool { + login.is_some_and(|login| { + account.label == login || account.label.starts_with(&format!("{login} (")) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::core::ProviderAccountData; + use crate::providers::copilot::endpoints::GitHubEndpoints; + use crate::providers::copilot::test_server::{FakeGitHub, ok}; + use uuid::Uuid; + + // interval 0 keeps the polling sleep instant in these tests. + const DEVICE_JSON: &str = r#"{"device_code":"device-example","user_code":"ABCD-1234","verification_uri":"https://github.com/login/device","expires_in":900,"interval":0}"#; + const TOKEN_JSON: &str = + r#"{"access_token":"gho_example_token","token_type":"bearer","scope":"read:user"}"#; + const USER_JSON: &str = r#"{"login":"octocat","id":42,"name":"Example Octocat"}"#; + const USAGE_JSON: &str = r#"{"copilot_plan":"individual","quota_snapshots":{}}"#; + + fn github_fake(token_reply: &'static str) -> FakeGitHub { + FakeGitHub::start(vec![ + ("/login/device/code", vec![ok(DEVICE_JSON)]), + ("/login/oauth/access_token", vec![ok(token_reply)]), + ("/user", vec![ok(USER_JSON)]), + ("/copilot_internal/user", vec![ok(USAGE_JSON)]), + ]) + } + + fn clients(fake: &FakeGitHub) -> (CopilotDeviceFlow, CopilotApi) { + let endpoints = GitHubEndpoints::with_origin(&fake.origin).expect("loopback origin"); + ( + CopilotDeviceFlow::with_endpoints(&endpoints), + CopilotApi::with_endpoints(endpoints), + ) + } + + fn store_in(dir: &tempfile::TempDir) -> TokenAccountStore { + TokenAccountStore::with_path(dir.path().join("token-accounts.json")) + } + + fn seed(store: &TokenAccountStore, accounts: Vec) { + let mut data = ProviderAccountData::new(); + for account in accounts { + data.add_account(account); + } + store + .save_provider(ProviderId::Copilot, &data) + .expect("seed store"); + } + + #[tokio::test] + async fn saves_a_new_account_with_a_stable_identifier() { + let fake = github_fake(TOKEN_JSON); + let (flow, api) = clients(&fake); + let dir = tempfile::tempdir().expect("tempdir"); + let store = store_in(&dir); + + let device = start_device_login(&flow).await.expect("device code"); + let saved = finish_device_login(&flow, &api, &store, &device) + .await + .expect("login"); + + assert_eq!(saved.label, "octocat (Copilot Individual)"); + assert_eq!(saved.login.as_deref(), Some("octocat")); + assert_eq!(saved.plan.as_deref(), Some("Copilot Individual")); + assert_eq!(saved.identity_id, Some(42)); + + let data = store.load_provider(ProviderId::Copilot).expect("load"); + assert_eq!(data.accounts.len(), 1); + assert_eq!(data.active_index, 0); + let account = &data.accounts[0]; + assert_eq!(account.label, "octocat (Copilot Individual)"); + assert_eq!(account.token, "gho_example_token"); + assert_eq!( + account.external_identifier.as_deref(), + Some("github:user:42") + ); + assert!(account.last_used.is_some()); + } + + #[tokio::test] + async fn relogin_updates_the_matching_account_in_place() { + let fake = github_fake(TOKEN_JSON); + let (flow, api) = clients(&fake); + let dir = tempfile::tempdir().expect("tempdir"); + let store = store_in(&dir); + + let existing_id = Uuid::new_v4(); + let mut existing = TokenAccount::new("octocat (old)", "gho_old_token"); + existing.id = existing_id; + existing.external_identifier = Some("github:user:42".to_string()); + seed(&store, vec![existing]); + + let device = start_device_login(&flow).await.expect("device code"); + finish_device_login(&flow, &api, &store, &device) + .await + .expect("login"); + + let data = store.load_provider(ProviderId::Copilot).expect("load"); + assert_eq!(data.accounts.len(), 1); + assert_eq!(data.accounts[0].id, existing_id); + assert_eq!(data.accounts[0].token, "gho_example_token"); + assert_eq!(data.accounts[0].label, "octocat (Copilot Individual)"); + } + + #[tokio::test] + async fn adopts_a_legacy_account_with_the_same_github_identity() { + let fake = github_fake(TOKEN_JSON); + let (flow, api) = clients(&fake); + let dir = tempfile::tempdir().expect("tempdir"); + let store = store_in(&dir); + + let legacy_id = Uuid::new_v4(); + let mut legacy = TokenAccount::new("Work account", "gho_legacy_token"); + legacy.id = legacy_id; + seed(&store, vec![legacy]); + + let device = start_device_login(&flow).await.expect("device code"); + finish_device_login(&flow, &api, &store, &device) + .await + .expect("login"); + + let data = store.load_provider(ProviderId::Copilot).expect("load"); + assert_eq!(data.accounts.len(), 1, "legacy account is reused"); + assert_eq!(data.accounts[0].id, legacy_id); + assert_eq!( + data.accounts[0].external_identifier.as_deref(), + Some("github:user:42") + ); + } + + #[test] + fn stored_identity_precedes_legacy_label_match() { + let legacy = TokenAccount::new("octocat (Pro)", "old-token"); + let mut identified = TokenAccount::new("Renamed account", "known-token"); + identified.external_identifier = Some(copilot_external_identifier(123)); + + let accounts = vec![legacy.clone(), identified]; + assert_eq!(find_copilot_account_by_identity(&accounts, 123), Some(1)); + assert!(copilot_label_matches_login(&legacy, Some("octocat"))); + } + + #[tokio::test] + async fn denied_login_saves_nothing() { + let fake = github_fake(r#"{"error":"access_denied"}"#); + let (flow, api) = clients(&fake); + let dir = tempfile::tempdir().expect("tempdir"); + let store = store_in(&dir); + + let device = start_device_login(&flow).await.expect("device code"); + let error = finish_device_login(&flow, &api, &store, &device) + .await + .expect_err("denied"); + + assert!(matches!( + error, + CopilotLoginError::Flow(DeviceFlowError::AccessDenied) + )); + assert_eq!( + error.to_string(), + "GitHub device login failed: Access denied by user" + ); + assert!( + !dir.path().join("token-accounts.json").exists(), + "a denied login must not create the store" + ); + } +} diff --git a/rust/src/providers/copilot/mod.rs b/rust/src/providers/copilot/mod.rs index 6383ba57818..e49adf5e47a 100755 --- a/rust/src/providers/copilot/mod.rs +++ b/rust/src/providers/copilot/mod.rs @@ -4,6 +4,10 @@ mod api; pub mod device_flow; +pub(crate) mod endpoints; +pub mod login; +#[cfg(test)] +pub(crate) mod test_server; use async_trait::async_trait; diff --git a/rust/src/providers/copilot/test_server.rs b/rust/src/providers/copilot/test_server.rs new file mode 100644 index 00000000000..a17456c3fc0 --- /dev/null +++ b/rust/src/providers/copilot/test_server.rs @@ -0,0 +1,139 @@ +//! Loopback GitHub fake for Copilot device-login tests. +//! +//! Each route pops its scripted replies in order and repeats the last one, so +//! a test can script `authorization_pending` followed by a success on the same +//! polling path. Requests are recorded as `"METHOD /path"`. + +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +/// A scripted reply: HTTP status and JSON body. +#[derive(Debug, Clone)] +pub(crate) struct Reply { + pub(crate) status: u16, + pub(crate) body: &'static str, +} + +pub(crate) fn ok(body: &'static str) -> Reply { + Reply { status: 200, body } +} + +/// A running fake that stops when dropped. +pub(crate) struct FakeGitHub { + pub(crate) origin: String, + requests: Arc>>, + stop: Arc, +} + +impl FakeGitHub { + pub(crate) fn start(routes: Vec<(&'static str, Vec)>) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind loopback fake"); + listener.set_nonblocking(true).expect("nonblocking"); + let origin = format!("http://{}", listener.local_addr().expect("local addr")); + let requests = Arc::new(Mutex::new(Vec::new())); + let stop = Arc::new(AtomicBool::new(false)); + let routes = Arc::new(Mutex::new(routes)); + + let thread_requests = Arc::clone(&requests); + let thread_stop = Arc::clone(&stop); + std::thread::spawn(move || { + while !thread_stop.load(Ordering::SeqCst) { + match listener.accept() { + Ok((stream, _)) => { + stream.set_nonblocking(false).ok(); + handle(stream, &routes, &thread_requests); + } + Err(_) => std::thread::sleep(Duration::from_millis(2)), + } + } + }); + + Self { + origin, + requests, + stop, + } + } + + /// Recorded requests, in arrival order, as `"METHOD /path"`. + pub(crate) fn requests(&self) -> Vec { + self.requests.lock().expect("requests lock").clone() + } +} + +impl Drop for FakeGitHub { + fn drop(&mut self) { + self.stop.store(true, Ordering::SeqCst); + } +} + +fn handle( + mut stream: std::net::TcpStream, + routes: &Mutex)>>, + requests: &Mutex>, +) { + stream.set_read_timeout(Some(Duration::from_secs(5))).ok(); + let Some((method, path, body_len)) = read_head(&mut stream) else { + return; + }; + let mut body = vec![0u8; body_len]; + stream.read_exact(&mut body).ok(); + + requests + .lock() + .expect("requests lock") + .push(format!("{method} {path}")); + + let reply = { + let mut routes = routes.lock().expect("routes lock"); + match routes.iter_mut().find(|(route, _)| *route == path) { + Some((_, replies)) if replies.len() > 1 => replies.remove(0), + Some((_, replies)) => replies.first().cloned().unwrap_or(Reply { + status: 500, + body: "{}", + }), + None => Reply { + status: 404, + body: "{}", + }, + } + }; + + let response = format!( + "HTTP/1.1 {} Fake\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + reply.status, + reply.body.len(), + reply.body + ); + stream.write_all(response.as_bytes()).ok(); + stream.flush().ok(); +} + +/// Reads request line and headers; returns method, path, and body length. +fn read_head(stream: &mut std::net::TcpStream) -> Option<(String, String, usize)> { + let mut buffer = Vec::new(); + let mut byte = [0u8; 1]; + while !buffer.ends_with(b"\r\n\r\n") { + if stream.read(&mut byte).ok()? == 0 { + return None; + } + buffer.push(byte[0]); + if buffer.len() > 64 * 1024 { + return None; + } + } + let head = String::from_utf8_lossy(&buffer).to_string(); + let mut lines = head.split("\r\n"); + let mut request_line = lines.next()?.split_whitespace(); + let method = request_line.next()?.to_string(); + let path = request_line.next()?.split('?').next()?.to_string(); + let body_len = lines + .filter_map(|line| line.split_once(':')) + .find(|(name, _)| name.trim().eq_ignore_ascii_case("content-length")) + .and_then(|(_, value)| value.trim().parse::().ok()) + .unwrap_or(0); + Some((method, path, body_len)) +} From d5d7c3d5339841640afb85b78fee334f7737f7d1 Mon Sep 17 00:00:00 2001 From: Factory Agent Date: Fri, 23 Oct 2026 16:13:17 +0700 Subject: [PATCH 2/3] Enable Copilot and add config login exit codes After a saved login, config login turns the Copilot provider on. Each failure has its own exit code (denied 5, expired 6, cancelled 7, other 1, usage 64), and JSON output adds cancelled and failed.reason. Ctrl-C stops the login before anything is saved. --- docs/CLI.md | 24 ++- rust/src/cli/config.rs | 129 ++++++-------- rust/src/cli/config/login.rs | 259 ++++++++++++++++++++++++++++ rust/src/cli/config/tests.rs | 157 +++++++++++++++-- rust/src/cli/mod.rs | 8 + rust/src/main.rs | 13 +- rust/src/providers/copilot/login.rs | 4 + 7 files changed, 498 insertions(+), 96 deletions(-) create mode 100644 rust/src/cli/config/login.rs diff --git a/docs/CLI.md b/docs/CLI.md index 0e5f61a7231..e52b9ca2e2c 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -401,15 +401,33 @@ Connected The token is never printed. Once you approve the code, the account is saved the same way the desktop app's Copilot sign-in saves it. The account is matched by GitHub identity, so signing in again updates the existing account instead of adding a second one. Its label is the GitHub login, with the plan in brackets when GitHub reports one. +A successful login also turns the Copilot provider on in settings, as the macOS app does. The desktop app's sign-in does not change the enabled state, so enable Copilot there yourself if you use that path. + `--json` prints one JSON object per line, using upstream's field names: | Line | Shape | | --- | --- | | Code issued | `{"status":"authorization_pending","verificationURI":"https://github.com/login/device","userCode":"ABCD-1234","expiresIn":900}` | -| Approved | `{"status":"authorized"}` | -| Failed | `{"status":"failed","error":"GitHub device login failed: Access denied by user"}`, and the command exits with an error | +| Approved and saved | `{"status":"authorized"}`. This line is written only after the account is saved and Copilot is enabled. | +| Cancelled (Ctrl-C) | `{"status":"cancelled"}` | +| Failed | `{"status":"failed","reason":"denied","error":"GitHub device login failed: Access denied by user"}`. `reason` is one of the values in the exit code table below. | + +Text mode prints the URL and code, then `Connected` on success. Errors go to stderr as `Error: `. Ctrl-C ends the login with `Error: Login cancelled.` and exit code `7`. + +### Exit codes + +| Exit code | Meaning | Final `--json` line | +| --- | --- | --- | +| `0` | Signed in. The account is saved and Copilot is enabled. | `{"status":"authorized"}` | +| `1` | Any other failure: network error, GitHub server error, or a settings or token store write error. Nothing is saved. | `{"status":"failed","reason":"error",...}` | +| `5` | The user denied the request on GitHub (`access_denied`). Nothing is saved. | `{"status":"failed","reason":"denied",...}` | +| `6` | The device code expired before approval (`expired_token`). Nothing is saved. | `{"status":"failed","reason":"expired",...}` | +| `7` | Stopped with Ctrl-C before the account was saved. Nothing is saved. | `{"status":"cancelled"}` | +| `64` | Invalid arguments, including an unsupported `--provider`. No network call is made. | None; the error goes to stderr | + +Exit code `0` means the credential was saved. Any other code means it was not. -Polling follows the interval GitHub returns. Each `slow_down` response adds 5 seconds for the rest of the login. GitHub's `access_denied` and `expired_token` responses end the login with a friendly error, and nothing is saved. +Polling follows the interval GitHub returns. Each `slow_down` response adds 5 seconds for the rest of the login. GitHub's `access_denied` and `expired_token` responses end the login with the friendly errors above, and nothing is saved. Other providers fail before any network call: `Login is only supported for copilot. Use --provider copilot.` diff --git a/rust/src/cli/config.rs b/rust/src/cli/config.rs index 6185681ddde..889289dcce2 100755 --- a/rust/src/cli/config.rs +++ b/rust/src/cli/config.rs @@ -5,6 +5,7 @@ use clap::{Parser, Subcommand}; use serde::Serialize; use serde::de::DeserializeOwned; +use std::future::Future; use std::io::Write; use std::path::Path; @@ -12,9 +13,16 @@ use super::usage::OutputFormat; use crate::core::{ProviderId, SourceMode, TokenAccountStore, instantiate_provider}; use crate::providers::copilot::CopilotApi; use crate::providers::copilot::device_flow::CopilotDeviceFlow; -use crate::providers::copilot::login::{finish_device_login, start_device_login}; +use crate::providers::copilot::login::{ + SavedCopilotAccount, finish_device_login, start_device_login, +}; use crate::settings::{ApiKeys, ManualCookies, PreferencesDocument, Settings}; +mod login; + +pub use login::ConfigLoginError; +use login::{ConfigLoginEvent, LoginFailure, report_outcome, write_event}; + /// Arguments for the config command #[derive(Parser, Debug)] pub struct ConfigArgs { @@ -213,7 +221,9 @@ pub async fn run(args: ConfigArgs) -> anyhow::Result<()> { stdin, no_enable, } => set_api_key(target.name(), api_key.as_deref(), stdin, !no_enable).await, - ConfigCommand::Login { target, output } => login(target.name(), output).await, + ConfigCommand::Login { target, output } => login(target.name(), output) + .await + .map_err(anyhow::Error::from), ConfigCommand::Path => show_paths().await, ConfigCommand::Preferences { action } => transfer_preferences(action), ConfigCommand::ClaudeCodeCredentials { action, output } => { @@ -783,38 +793,13 @@ fn resolve_api_key_input(api_key: Option<&str>, read_from_stdin: bool) -> anyhow Ok(value) } -/// One `config login` event (upstream `ConfigLoginEvent`). It carries the -/// verification URL and user code only; the token is never part of it. -#[derive(Debug, Serialize)] -struct ConfigLoginEvent { - status: &'static str, - #[serde(rename = "verificationURI", skip_serializing_if = "Option::is_none")] - verification_uri: Option, - #[serde(rename = "userCode", skip_serializing_if = "Option::is_none")] - user_code: Option, - #[serde(rename = "expiresIn", skip_serializing_if = "Option::is_none")] - expires_in: Option, - #[serde(skip_serializing_if = "Option::is_none")] - error: Option, -} - -impl ConfigLoginEvent { - fn status(status: &'static str) -> Self { - Self { - status, - verification_uri: None, - user_code: None, - expires_in: None, - error: None, - } - } -} - /// Run `config login`. Only GitHub Copilot has a CLI login flow. -async fn login(provider: &str, output: ConfigOutputArgs) -> anyhow::Result<()> { - let id = parse_provider(provider)?; +async fn login(provider: &str, output: ConfigOutputArgs) -> Result<(), ConfigLoginError> { + let id = parse_provider(provider).map_err(ConfigLoginError::usage)?; if id != ProviderId::Copilot { - anyhow::bail!("Login is only supported for copilot. Use --provider copilot."); + return Err(ConfigLoginError::usage( + "Login is only supported for copilot. Use --provider copilot.", + )); } let flow = CopilotDeviceFlow::new(); let api = CopilotApi::new(); @@ -825,69 +810,67 @@ async fn login(provider: &str, output: ConfigOutputArgs) -> anyhow::Result<()> { &store, output.is_json(), &mut std::io::stdout().lock(), + login::ctrl_c(), + enable_copilot, ) .await + .map(|_| ()) +} + +/// Turns the Copilot provider on after a saved login. Only `config login` +/// does this; the desktop sign-in keeps its current behavior. +fn enable_copilot() -> anyhow::Result<()> { + let mut settings = Settings::load(); + settings.enable_provider(ProviderId::Copilot); + settings.save() } /// Runs the Copilot device login and writes progress to `out`. In JSON mode -/// each event is one line; in text mode the URL and code are two lines. A -/// failure is also reported as a `failed` JSON event before the error returns. +/// each event is one line; in text mode the URL and code are two lines. +/// +/// The account is saved before `enable` runs, and `enable` runs before the +/// `authorized` event. `cancel` (Ctrl-C in the CLI) is checked first, so once +/// it fires the flow stops before it can save anything else. async fn run_copilot_login( flow: &CopilotDeviceFlow, api: &CopilotApi, store: &TokenAccountStore, json: bool, out: &mut impl Write, -) -> anyhow::Result<()> { - match login_steps(flow, api, store, json, out).await { - Ok(()) => write_login_event(out, json, &ConfigLoginEvent::status("authorized")), - Err(error) => { - if json { - let mut failed = ConfigLoginEvent::status("failed"); - failed.error = Some(error.to_string()); - write_login_event(out, json, &failed)?; - } - Err(error) - } - } + cancel: impl Future, + enable: impl FnOnce() -> anyhow::Result<()>, +) -> Result { + let result = tokio::select! { + biased; + () = cancel => Err(ConfigLoginError::new(LoginFailure::Cancelled, "Login cancelled.")), + saved = copilot_steps(flow, api, store, json, out) => saved, + }; + let result = result.and_then(|mut saved| { + enable().map_err(ConfigLoginError::other)?; + saved.enabled = true; + Ok(saved) + }); + report_outcome(json, out, result) } -async fn login_steps( +async fn copilot_steps( flow: &CopilotDeviceFlow, api: &CopilotApi, store: &TokenAccountStore, json: bool, out: &mut impl Write, -) -> anyhow::Result<()> { +) -> Result { let device = start_device_login(flow).await?; - write_login_event( + write_event( out, json, - &ConfigLoginEvent { - status: "authorization_pending", - verification_uri: Some(device.verification_uri.clone()), - user_code: Some(device.user_code.clone()), - expires_in: Some(device.expires_in), - error: None, - }, + &ConfigLoginEvent::pending( + &device.verification_uri, + &device.user_code, + device.expires_in, + ), )?; - finish_device_login(flow, api, store, &device).await?; - Ok(()) -} - -fn write_login_event( - out: &mut impl Write, - json: bool, - event: &ConfigLoginEvent, -) -> anyhow::Result<()> { - if json { - writeln!(out, "{}", serde_json::to_string(event)?)?; - } else if let (Some(url), Some(code)) = (&event.verification_uri, &event.user_code) { - writeln!(out, "{url}\n{code}")?; - } else if event.status == "authorized" { - writeln!(out, "Connected")?; - } - Ok(()) + Ok(finish_device_login(flow, api, store, &device).await?) } /// Show configuration file paths diff --git a/rust/src/cli/config/login.rs b/rust/src/cli/config/login.rs new file mode 100644 index 00000000000..d879ff661a7 --- /dev/null +++ b/rust/src/cli/config/login.rs @@ -0,0 +1,259 @@ +//! Shared pieces of `codexbar config login`: the JSON event shape, the typed +//! failure with its exit code, and the reporter that writes the final event. +//! +//! Each provider's login writes its own progress events, then hands its +//! result to [`report_outcome`]. Tokens and secrets never reach these types. + +use std::fmt::Display; +use std::io::Write; + +use serde::Serialize; + +use crate::cli::exit_codes; +use crate::providers::copilot::device_flow::DeviceFlowError; +use crate::providers::copilot::login::CopilotLoginError; + +/// Why `config login` did not finish. Each variant has one exit code. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LoginFailure { + /// Bad provider or an unsupported provider for `config login`. + Usage, + /// The user denied the request (`access_denied`). + Denied, + /// The device code or sign-in window expired before approval. + Expired, + /// Stopped with Ctrl-C before the credential was saved. + Cancelled, + /// The provider's CLI binary is not installed. + MissingBinary, + /// Any other failure (network, GitHub error, store or settings write). + Other, +} + +impl LoginFailure { + /// Process exit code for this failure. Success is exit code 0. + pub fn exit_code(self) -> i32 { + match self { + Self::Usage => exit_codes::USAGE_ERROR, + Self::Denied => exit_codes::LOGIN_DENIED, + Self::Expired => exit_codes::LOGIN_EXPIRED, + Self::Cancelled => exit_codes::LOGIN_CANCELLED, + Self::MissingBinary => exit_codes::LOGIN_MISSING_BINARY, + Self::Other => exit_codes::UNEXPECTED_FAILURE, + } + } + + /// Value of the `reason` field on a `failed` event. + fn reason(self) -> &'static str { + match self { + Self::Usage => "usage", + Self::Denied => "denied", + Self::Expired => "expired", + Self::Cancelled => "cancelled", + Self::MissingBinary => "missing_binary", + Self::Other => "error", + } + } +} + +/// A `config login` failure. `main` reads [`ConfigLoginError::exit_code`] to +/// pick the process exit code. +#[derive(Debug, thiserror::Error)] +#[error("{message}")] +pub struct ConfigLoginError { + failure: LoginFailure, + message: String, +} + +impl ConfigLoginError { + pub fn new(failure: LoginFailure, message: impl Into) -> Self { + Self { + failure, + message: message.into(), + } + } + + pub fn usage(message: impl Display) -> Self { + Self::new(LoginFailure::Usage, message.to_string()) + } + + pub fn other(error: impl Display) -> Self { + Self::new(LoginFailure::Other, error.to_string()) + } + + pub fn failure(&self) -> LoginFailure { + self.failure + } + + pub fn exit_code(&self) -> i32 { + self.failure.exit_code() + } +} + +impl From for ConfigLoginError { + fn from(error: CopilotLoginError) -> Self { + let failure = match &error { + CopilotLoginError::Flow(DeviceFlowError::AccessDenied) => LoginFailure::Denied, + CopilotLoginError::Flow(DeviceFlowError::ExpiredToken) => LoginFailure::Expired, + _ => LoginFailure::Other, + }; + Self::new(failure, error.to_string()) + } +} + +/// One `config login` event (upstream `ConfigLoginEvent`). It carries the +/// verification URL and user code only; the token is never part of it. +#[derive(Debug, Serialize, PartialEq, Eq)] +pub struct ConfigLoginEvent { + pub status: &'static str, + #[serde(rename = "verificationURI", skip_serializing_if = "Option::is_none")] + pub verification_uri: Option, + #[serde(rename = "userCode", skip_serializing_if = "Option::is_none")] + pub user_code: Option, + #[serde(rename = "expiresIn", skip_serializing_if = "Option::is_none")] + pub expires_in: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub reason: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +impl ConfigLoginEvent { + pub fn status(status: &'static str) -> Self { + Self { + status, + verification_uri: None, + user_code: None, + expires_in: None, + reason: None, + error: None, + } + } + + /// The `authorization_pending` event that shows the URL and code. + pub fn pending(verification_uri: &str, user_code: &str, expires_in: u32) -> Self { + Self { + verification_uri: Some(verification_uri.to_string()), + user_code: Some(user_code.to_string()), + expires_in: Some(expires_in), + ..Self::status("authorization_pending") + } + } + + /// The `failed` event. It carries the failure reason and message only. + pub fn failed(error: &ConfigLoginError) -> Self { + Self { + reason: Some(error.failure().reason()), + error: Some(error.to_string()), + ..Self::status("failed") + } + } +} + +/// Writes one event. JSON mode prints one object per line. Text mode prints +/// the URL and code for `authorization_pending` and `Connected` for +/// `authorized`; other events print nothing and their error goes to stderr. +pub fn write_event( + out: &mut impl Write, + json: bool, + event: &ConfigLoginEvent, +) -> Result<(), ConfigLoginError> { + if json { + let line = serde_json::to_string(event).map_err(ConfigLoginError::other)?; + writeln!(out, "{line}").map_err(ConfigLoginError::other)?; + } else if let (Some(url), Some(code)) = (&event.verification_uri, &event.user_code) { + writeln!(out, "{url}\n{code}").map_err(ConfigLoginError::other)?; + } else if event.status == "authorized" { + writeln!(out, "Connected").map_err(ConfigLoginError::other)?; + } + Ok(()) +} + +/// Future that resolves on Ctrl-C. If the handler cannot be installed, it +/// never resolves, so a broken signal setup cannot cancel a login. +pub async fn ctrl_c() { + if tokio::signal::ctrl_c().await.is_err() { + std::future::pending::<()>().await; + } +} + +/// Writes the final event for `result` and returns the result unchanged. +/// +/// A success writes `authorized` only after the credential is saved. A Ctrl-C +/// writes `cancelled`. Any other failure writes a `failed` event with its +/// reason in JSON mode; text mode prints the error on stderr from the caller. +/// A failed write to stdout is logged and does not change the result, so +/// exit code 0 still means the credential was saved. +pub fn report_outcome( + json: bool, + out: &mut impl Write, + result: Result, +) -> Result { + let event = match &result { + Ok(_) => Some(ConfigLoginEvent::status("authorized")), + Err(error) if error.failure() == LoginFailure::Cancelled => { + Some(ConfigLoginEvent::status("cancelled")) + } + Err(error) if json => Some(ConfigLoginEvent::failed(error)), + Err(_) => None, + }; + if let Some(event) = event + && let Err(error) = write_event(out, json, &event) + { + tracing::warn!("Could not write the config login result: {error}"); + } + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn each_failure_has_its_own_exit_code() { + assert_eq!(LoginFailure::Usage.exit_code(), exit_codes::USAGE_ERROR); + assert_eq!(LoginFailure::Denied.exit_code(), 5); + assert_eq!(LoginFailure::Expired.exit_code(), 6); + assert_eq!(LoginFailure::Cancelled.exit_code(), 7); + assert_eq!(LoginFailure::MissingBinary.exit_code(), 8); + assert_eq!( + LoginFailure::Other.exit_code(), + exit_codes::UNEXPECTED_FAILURE + ); + } + + #[test] + fn failed_event_carries_reason_and_message_only() { + let error = ConfigLoginError::new(LoginFailure::Denied, "GitHub said no"); + let json = serde_json::to_value(ConfigLoginEvent::failed(&error)).expect("json"); + assert_eq!( + json, + serde_json::json!({ + "status": "failed", + "reason": "denied", + "error": "GitHub said no", + }) + ); + } + + #[test] + fn report_outcome_writes_cancelled_for_ctrl_c_and_returns_the_error() { + let error = ConfigLoginError::new(LoginFailure::Cancelled, "Login cancelled."); + let mut out = Vec::new(); + let result: Result<(), _> = report_outcome(true, &mut out, Err(error)); + assert_eq!(result.expect_err("cancelled").exit_code(), 7); + assert_eq!( + String::from_utf8(out).expect("utf8"), + "{\"status\":\"cancelled\"}\n" + ); + } + + #[test] + fn report_outcome_text_failure_prints_nothing_on_stdout() { + let error = ConfigLoginError::new(LoginFailure::Expired, "Device code expired"); + let mut out = Vec::new(); + let result: Result<(), _> = report_outcome(false, &mut out, Err(error)); + assert!(result.is_err()); + assert!(out.is_empty()); + } +} diff --git a/rust/src/cli/config/tests.rs b/rust/src/cli/config/tests.rs index 3a19105544f..13054342dd9 100644 --- a/rust/src/cli/config/tests.rs +++ b/rust/src/cli/config/tests.rs @@ -496,12 +496,19 @@ async fn config_login_rejects_other_providers_before_any_io() { error.to_string(), "Login is only supported for copilot. Use --provider copilot." ); + assert_eq!(error.exit_code(), crate::cli::exit_codes::USAGE_ERROR); } mod copilot_login_output { - use super::super::{CopilotApi, CopilotDeviceFlow, TokenAccountStore, run_copilot_login}; + use std::cell::Cell; + use std::future::Future; + + use super::super::{ + ConfigLoginError, CopilotApi, CopilotDeviceFlow, TokenAccountStore, run_copilot_login, + }; use crate::core::ProviderId; use crate::providers::copilot::endpoints::GitHubEndpoints; + use crate::providers::copilot::login::SavedCopilotAccount; use crate::providers::copilot::test_server::{FakeGitHub, Reply, ok}; const DEVICE_JSON: &str = r#"{"device_code":"device-example","user_code":"ABCD-1234","verification_uri":"https://github.com/login/device","expires_in":900,"interval":0}"#; @@ -519,32 +526,59 @@ mod copilot_login_output { ]) } - /// The returned `TempDir` must stay alive while the store is read back. - async fn run( + /// One login run. `_dir` must stay alive while the store is read back. + struct Run { + result: Result, + out: String, + store: TokenAccountStore, + enable_calls: u32, + _dir: tempfile::TempDir, + } + + async fn run(fake: &FakeGitHub, json: bool) -> Run { + run_with_cancel(fake, json, std::future::pending()).await + } + + async fn run_with_cancel( fake: &FakeGitHub, json: bool, - ) -> ( - anyhow::Result<()>, - String, - TokenAccountStore, - tempfile::TempDir, - ) { + cancel: impl Future, + ) -> Run { let endpoints = GitHubEndpoints::with_origin(&fake.origin).expect("loopback origin"); let flow = CopilotDeviceFlow::with_endpoints(&endpoints); let api = CopilotApi::with_endpoints(endpoints); let dir = tempfile::tempdir().expect("tempdir"); let store = TokenAccountStore::with_path(dir.path().join("token-accounts.json")); let mut out = Vec::new(); - let result = run_copilot_login(&flow, &api, &store, json, &mut out).await; - (result, String::from_utf8(out).expect("utf8"), store, dir) + let enable_calls = Cell::new(0); + let result = run_copilot_login(&flow, &api, &store, json, &mut out, cancel, || { + enable_calls.set(enable_calls.get() + 1); + Ok(()) + }) + .await; + Run { + result, + out: String::from_utf8(out).expect("utf8"), + store, + enable_calls: enable_calls.get(), + _dir: dir, + } } #[tokio::test] async fn text_output_shows_url_and_code_then_connected_and_never_the_token() { let fake = fake(vec![ok(TOKEN_JSON)]); - let (result, out, store, _dir) = run(&fake, false).await; - - result.expect("login succeeds"); + let Run { + result, + out, + store, + enable_calls, + _dir, + } = run(&fake, false).await; + + let saved = result.expect("login succeeds"); + assert!(saved.enabled, "a saved login enables the provider"); + assert_eq!(enable_calls, 1); assert_eq!( out, "https://github.com/login/device\nABCD-1234\nConnected\n" @@ -561,9 +595,16 @@ mod copilot_login_output { ok(r#"{"error":"authorization_pending"}"#), ok(TOKEN_JSON), ]); - let (result, out, _store, _dir) = run(&fake, true).await; + let Run { + result, + out, + enable_calls, + _dir, + .. + } = run(&fake, true).await; result.expect("login succeeds"); + assert_eq!(enable_calls, 1); assert!(!out.contains("gho_example_token")); let events: Vec = out .lines() @@ -584,9 +625,20 @@ mod copilot_login_output { #[tokio::test] async fn denied_json_reports_a_failed_event_and_returns_the_error() { let fake = fake(vec![ok(r#"{"error":"access_denied"}"#)]); - let (result, out, store, _dir) = run(&fake, true).await; + let Run { + result, + out, + store, + enable_calls, + _dir, + } = run(&fake, true).await; let error = result.expect_err("denied"); + assert_eq!(error.exit_code(), 5); + assert_eq!( + enable_calls, 0, + "a denied login does not enable the provider" + ); assert_eq!( error.to_string(), "GitHub device login failed: Access denied by user" @@ -594,6 +646,7 @@ mod copilot_login_output { let failed: serde_json::Value = serde_json::from_str(out.lines().last().expect("event")).expect("failed event is JSON"); assert_eq!(failed["status"], "failed"); + assert_eq!(failed["reason"], "denied"); assert_eq!( failed["error"], "GitHub device login failed: Access denied by user" @@ -611,12 +664,78 @@ mod copilot_login_output { #[tokio::test] async fn expired_text_login_prints_the_code_and_returns_an_error() { let fake = fake(vec![ok(r#"{"error":"expired_token"}"#)]); - let (result, out, _store, _dir) = run(&fake, false).await; - + let Run { + result, + out, + enable_calls, + _dir, + .. + } = run(&fake, false).await; + + let error = result.expect_err("expired"); + assert_eq!(error.exit_code(), 6); + assert_eq!(enable_calls, 0); assert_eq!( - result.expect_err("expired").to_string(), + error.to_string(), "GitHub device login failed: Device code expired" ); assert_eq!(out, "https://github.com/login/device\nABCD-1234\n"); } + + #[tokio::test] + async fn cancelled_before_any_request_saves_nothing_and_reports_cancelled() { + let fake = fake(vec![ok(TOKEN_JSON)]); + let Run { + result, + out, + store, + enable_calls, + _dir, + } = run_with_cancel(&fake, true, std::future::ready(())).await; + + let error = result.expect_err("cancelled"); + assert_eq!(error.exit_code(), 7); + assert_eq!(error.to_string(), "Login cancelled."); + assert_eq!(out, "{\"status\":\"cancelled\"}\n"); + assert_eq!(enable_calls, 0); + assert!(fake.requests().is_empty(), "no GitHub request after Ctrl-C"); + assert!( + store + .load_provider(ProviderId::Copilot) + .expect("load") + .accounts + .is_empty() + ); + } + + #[tokio::test] + async fn network_failure_is_a_general_failure_with_an_error_reason() { + let fake = FakeGitHub::start(vec![( + "/login/device/code", + vec![Reply { + status: 500, + body: "{}", + }], + )]); + let Run { + result, + out, + store, + _dir, + .. + } = run(&fake, true).await; + + assert_eq!(result.expect_err("server error").exit_code(), 1); + let failed: serde_json::Value = + serde_json::from_str(out.lines().last().expect("event")).expect("failed event is JSON"); + assert_eq!(failed["status"], "failed"); + assert_eq!(failed["reason"], "error"); + assert!( + store + .load_provider(ProviderId::Copilot) + .expect("load") + .accounts + .is_empty() + ); + } } diff --git a/rust/src/cli/mod.rs b/rust/src/cli/mod.rs index 437badadb37..78536879e69 100755 --- a/rust/src/cli/mod.rs +++ b/rust/src/cli/mod.rs @@ -40,6 +40,14 @@ pub mod exit_codes { pub const USAGE_ERROR: i32 = 64; /// Quota could not be checked (`EX_UNAVAILABLE`); used by `codexbar guard`. pub const UNAVAILABLE: i32 = 69; + /// `config login`: the user denied the device or sign-in request. + pub const LOGIN_DENIED: i32 = 5; + /// `config login`: the device code expired before it was approved. + pub const LOGIN_EXPIRED: i32 = 6; + /// `config login`: stopped with Ctrl-C before the credential was saved. + pub const LOGIN_CANCELLED: i32 = 7; + /// `config login --provider grok|kiro`: the provider's CLI is not installed. + pub const LOGIN_MISSING_BINARY: i32 = 8; } /// CodexBar - Monitor AI provider usage limits diff --git a/rust/src/main.rs b/rust/src/main.rs index 83217d32a87..5c35f827adf 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -110,7 +110,9 @@ fn dispatch_command(rt: &Runtime, command: Option) -> i32 { Some(Commands::Dashboard(args)) => run_unexpected(rt, cli::dashboard::run(args)), Some(Commands::Autostart(args)) => run_unexpected(rt, cli::autostart::run(args)), Some(Commands::Account(args)) => run_unexpected(rt, cli::account::run(args)), - Some(Commands::Config(args)) => run_unexpected(rt, cli::config::run(args)), + Some(Commands::Config(args)) => { + run_command(rt, cli::config::run(args), categorize_config_error) + } Some(Commands::Hooks(args)) => run_unexpected(rt, cli::hooks::run(args)), Some(Commands::Workspaces(args)) => run_categorized(rt, cli::workspaces::run(args)), None => missing_subcommand(), @@ -155,6 +157,15 @@ fn missing_subcommand() -> i32 { exit_codes::USAGE_ERROR } +/// `config login` failures carry their own exit code (see +/// `cli::exit_codes`). Every other `config` error stays at the general failure code. +fn categorize_config_error(e: &anyhow::Error) -> i32 { + e.downcast_ref::().map_or( + exit_codes::UNEXPECTED_FAILURE, + cli::config::ConfigLoginError::exit_code, + ) +} + /// Categorize an error into the appropriate exit code fn categorize_error(e: &anyhow::Error) -> i32 { let msg = e.to_string().to_lowercase(); diff --git a/rust/src/providers/copilot/login.rs b/rust/src/providers/copilot/login.rs index d375c4aa819..1b84b43e967 100644 --- a/rust/src/providers/copilot/login.rs +++ b/rust/src/providers/copilot/login.rs @@ -25,6 +25,9 @@ pub struct SavedCopilotAccount { pub login: Option, pub plan: Option, pub identity_id: Option, + /// Whether the provider was turned on after the save. The save itself + /// leaves this false; `config login` sets it once it enables the provider. + pub enabled: bool, } /// Asks GitHub for a device code. The caller shows `verification_uri` and @@ -79,6 +82,7 @@ async fn save_device_token( login: login.clone(), plan, identity_id, + enabled: false, }; if let Some(index) = existing_index { From 42108a4bc65e31500732dc44cf514d65a9124d60 Mon Sep 17 00:00:00 2001 From: Factory Agent Date: Sat, 24 Oct 2026 03:11:44 +0700 Subject: [PATCH 3/3] Harden Copilot config login saves and errors --- docs/CLI.md | 4 +-- rust/src/cli/config.rs | 6 +++- rust/src/cli/config/tests.rs | 34 +++++++++++++++++++++++ rust/src/providers/copilot/device_flow.rs | 7 ++++- rust/src/providers/copilot/login.rs | 12 ++++++-- 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index e52b9ca2e2c..ae889729809 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -419,13 +419,13 @@ Text mode prints the URL and code, then `Connected` on success. Errors go to std | Exit code | Meaning | Final `--json` line | | --- | --- | --- | | `0` | Signed in. The account is saved and Copilot is enabled. | `{"status":"authorized"}` | -| `1` | Any other failure: network error, GitHub server error, or a settings or token store write error. Nothing is saved. | `{"status":"failed","reason":"error",...}` | +| `1` | Any other failure: network error, GitHub server error, or a settings or token store write error. Usually nothing is saved. If only the settings write that enables Copilot fails, the account is already saved and the error says `Signed in and saved the account, but could not enable Copilot`; running the login again updates that account. | `{"status":"failed","reason":"error",...}` | | `5` | The user denied the request on GitHub (`access_denied`). Nothing is saved. | `{"status":"failed","reason":"denied",...}` | | `6` | The device code expired before approval (`expired_token`). Nothing is saved. | `{"status":"failed","reason":"expired",...}` | | `7` | Stopped with Ctrl-C before the account was saved. Nothing is saved. | `{"status":"cancelled"}` | | `64` | Invalid arguments, including an unsupported `--provider`. No network call is made. | None; the error goes to stderr | -Exit code `0` means the credential was saved. Any other code means it was not. +Only exit code `0` guarantees both that the credential was saved and that Copilot is enabled. Codes `5`, `6`, `7` and `64` mean nothing was saved; code `1` can follow a saved account, as described above. Polling follows the interval GitHub returns. Each `slow_down` response adds 5 seconds for the rest of the login. GitHub's `access_denied` and `expired_token` responses end the login with the friendly errors above, and nothing is saved. diff --git a/rust/src/cli/config.rs b/rust/src/cli/config.rs index 889289dcce2..781eac16a03 100755 --- a/rust/src/cli/config.rs +++ b/rust/src/cli/config.rs @@ -846,7 +846,11 @@ async fn run_copilot_login( saved = copilot_steps(flow, api, store, json, out) => saved, }; let result = result.and_then(|mut saved| { - enable().map_err(ConfigLoginError::other)?; + enable().map_err(|error| { + ConfigLoginError::other(format!( + "Signed in and saved the account, but could not enable Copilot: {error}" + )) + })?; saved.enabled = true; Ok(saved) }); diff --git a/rust/src/cli/config/tests.rs b/rust/src/cli/config/tests.rs index 13054342dd9..7166ab9636f 100644 --- a/rust/src/cli/config/tests.rs +++ b/rust/src/cli/config/tests.rs @@ -543,6 +543,15 @@ mod copilot_login_output { fake: &FakeGitHub, json: bool, cancel: impl Future, + ) -> Run { + run_with_options(fake, json, cancel, false).await + } + + async fn run_with_options( + fake: &FakeGitHub, + json: bool, + cancel: impl Future, + enable_fails: bool, ) -> Run { let endpoints = GitHubEndpoints::with_origin(&fake.origin).expect("loopback origin"); let flow = CopilotDeviceFlow::with_endpoints(&endpoints); @@ -553,6 +562,9 @@ mod copilot_login_output { let enable_calls = Cell::new(0); let result = run_copilot_login(&flow, &api, &store, json, &mut out, cancel, || { enable_calls.set(enable_calls.get() + 1); + if enable_fails { + anyhow::bail!("settings write failed"); + } Ok(()) }) .await; @@ -589,6 +601,28 @@ mod copilot_login_output { assert_eq!(data.accounts[0].token, "gho_example_token"); } + #[tokio::test] + async fn failed_enable_reports_that_the_account_was_saved() { + let fake = fake(vec![ok(TOKEN_JSON)]); + let Run { + result, + store, + enable_calls, + _dir, + .. + } = run_with_options(&fake, false, std::future::pending(), true).await; + + let error = result.expect_err("enable failure fails the login"); + assert_eq!(enable_calls, 1); + assert_eq!(error.exit_code(), 1); + assert_eq!( + error.to_string(), + "Signed in and saved the account, but could not enable Copilot: settings write failed" + ); + let data = store.load_provider(ProviderId::Copilot).expect("load"); + assert_eq!(data.accounts.len(), 1, "the account stays saved"); + } + #[tokio::test] async fn json_output_is_upstream_shaped_events_without_the_token() { let fake = fake(vec![ diff --git a/rust/src/providers/copilot/device_flow.rs b/rust/src/providers/copilot/device_flow.rs index 850420182e5..ed05ea2e26b 100755 --- a/rust/src/providers/copilot/device_flow.rs +++ b/rust/src/providers/copilot/device_flow.rs @@ -106,7 +106,12 @@ impl CopilotDeviceFlow { pub(crate) fn with_endpoints(endpoints: &GitHubEndpoints) -> Self { Self { - client: reqwest::Client::new(), + // A stalled poll would otherwise never reach the expiry check. + client: crate::core::credentialed_http_client_builder() + .use_rustls_tls() + .timeout(Duration::from_secs(30)) + .build() + .unwrap_or_else(|_| reqwest::Client::new()), web_root: endpoints.web_root.clone(), } } diff --git a/rust/src/providers/copilot/login.rs b/rust/src/providers/copilot/login.rs index 1b84b43e967..538556d3b91 100644 --- a/rust/src/providers/copilot/login.rs +++ b/rust/src/providers/copilot/login.rs @@ -72,10 +72,18 @@ async fn save_device_token( (None, None) => "GitHub Copilot".to_string(), }; - let mut data = store.load_provider(ProviderId::Copilot)?; let identity_id = identity.as_ref().and_then(|identity| identity.id); + let snapshot = store.load_provider(ProviderId::Copilot)?; + let existing_id = + find_existing_copilot_account(api, &snapshot.accounts, identity_id, login.as_deref()) + .await + .map(|index| snapshot.accounts[index].id); + + // The match above can make one network call per legacy account; reload so + // edits saved meanwhile by the desktop app or another CLI are kept. + let mut data = store.load_provider(ProviderId::Copilot)?; let existing_index = - find_existing_copilot_account(api, &data.accounts, identity_id, login.as_deref()).await; + existing_id.and_then(|id| data.accounts.iter().position(|account| account.id == id)); let saved = SavedCopilotAccount { label: label.clone(),