diff --git a/docs/concepts.md b/docs/concepts.md index 02c8d07..6270c7b 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -406,9 +406,7 @@ path. ## Environments -Environment selection is application-defined. If an application needs an environment flag, it can -register one through `CliConfig::register_flags` and store the parsed value on middleware through -`CliConfig::apply_flags`. Auth providers receive the environment value during credential operations. +`cli_engine` provides a first-class environment system with layered resolution, a config-file layer, env-var overrides, sticky active-env persistence, and per-environment OAuth for `PkceAuthProvider`; see [Environments](environments.md) for the full reference. ## Risk Tiers diff --git a/docs/environments.md b/docs/environments.md new file mode 100644 index 0000000..3bcff84 --- /dev/null +++ b/docs/environments.md @@ -0,0 +1,171 @@ +# Environments + +`cli_engine` provides a first-class environment system that lets CLIs target named deployment environments — `prod`, `ote`, `dev`, and any others the application defines — without consumers having to wire flags, config lookups, or OAuth overrides by hand. + +When `CliConfig::with_environments` is called, the engine: + +- Registers a global `--env` flag on every command. +- Seeds the active environment into middleware at startup. +- Exposes the resolved environment to handlers via `CommandContext::environment`. +- Mounts the built-in `env list / get / set / info` commands under the admin help category. + +## Resolution Layers + +`Environments::resolve(name)` builds a fully-merged `Environment` by applying three layers in order. +Later layers win over earlier ones. + +1. **Compiled-in defaults** — `EnvironmentDef` values registered with `Environments::with_environment` in the application source code. +2. **`environments.toml`** — the file at `//environments.toml`, when enabled with `Environments::with_config_file(true)`. +3. **Environment-variable overrides** — `_OAUTH_CLIENT_ID`, `_OAUTH_AUTH_URL`, `_OAUTH_TOKEN_URL`, and `_` for each bag key already present in the merged record. + +A name that is unknown to all three layers — not in compiled defaults, not in the file, and not resolvable — returns an error listing the known names. + +## environments.toml Schema + +The file uses one top-level TOML table per environment name: + +```toml +[prod] +client_id = "prod-client-id" +auth_url = "https://api.example.com/v2/oauth2/authorize" +token_url = "https://api.example.com/v2/oauth2/token" +scopes = ["openid", "profile"] +api_url = "https://api.example.com" + +[ote] +client_id = "ote-client-id" +auth_url = "https://api.ote.example.com/v2/oauth2/authorize" +token_url = "https://api.ote.example.com/v2/oauth2/token" +api_url = "https://api.ote.example.com" +``` + +The recognized OAuth keys — `client_id`, `auth_url`, `token_url`, and `scopes` (an array of strings) — are parsed into the typed `OAuthConfig` slice of the resolved `Environment`. +Every other key is captured as a free-form field in `Environment::extra`, which is a `BTreeMap` — so these values **must be TOML strings** (for example `api_url` above). +A non-OAuth key whose value is a number, boolean, or array fails to parse; quote it as a string instead. +The `extra` bag is printed verbatim by `env info`, so it must not hold secrets. + +## Environment-Variable Overrides + +The prefix is the environment name uppercased with `-` replaced by `_` (`ote` → `OTE`, `prod-us` → `PROD_US`). +Names that differ only by `-` vs `_` map to the same prefix and will collide; avoid such names. + +The three OAuth fields are always overridable: + +| Variable | Field overridden | +| --- | --- | +| `_OAUTH_CLIENT_ID` | `oauth.client_id` | +| `_OAUTH_AUTH_URL` | `oauth.auth_url` | +| `_OAUTH_TOKEN_URL` | `oauth.token_url` | + +Scopes are **not** env-var overridable; set them in the compiled-in layer or `environments.toml`. + +Bag keys in `Environment::extra` are overridable via `_` only when the key is already present in the merged record after layers 1 and 2. +For example, `api_url` must exist in either the compiled defaults or the file before `PROD_API_URL` has any effect. + +## Active Environment + +The active environment controls which environment is targeted when no `--env` flag is passed. + +**Precedence** (highest first): + +1. `--env ` on the command line. +2. The `environment.active` key in the per-application config file (persisted by `env set`). +3. The default set in `Environments::new(default_env)`. + +`env set ` validates that the environment is defined — by a compiled default or `environments.toml` — and then writes `environment.active` to the config file. +Environment variables override fields of a defined environment but cannot define a new, selectable environment on their own, so a name known only through `_*` variables is rejected. +The next invocation (without `--env`) picks it up from layer 2. + +The built-in commands are: + +| Command | Description | +| --- | --- | +| `env list` | Lists all known environments (compiled + file), marking the active one. | +| `env get` | Prints the active environment name. | +| `env set ` | Validates and persists `name` as the active environment. | +| `env info` | Prints the fully resolved active environment including OAuth and extra fields. | + +`env set` is marked `Tier::Mutate` so `--dry-run` short-circuits the config-file write. + +## Per-Environment OAuth via PkceAuthProvider + +`PkceAuthProvider::with_environments(Arc)` wires the provider to the same `Environments` instance, making it the single source of truth for per-environment OAuth config. +Build one `Arc` — with `Environments::with_app_id()` set on it — and share that same `Arc` between the provider and `CliConfig::with_environments`. +If the two receive different instances, a file-defined environment (or a file override of a compiled environment's `client_id`) is visible to `env info` yet invisible to the actual OAuth login. + +When the provider resolves a credential for `env`, it calls `Environments::resolve(env)` and uses the resulting `OAuthConfig`. +Each field falls through to the next source when empty: + +1. The resolved environment's field (when non-empty). +2. The legacy provider-prefixed env var (`_OAUTH_CLIENT_ID`, `_AUTH_URL`, `_TOKEN_URL`), where `` is the provider name uppercased with `-` → `_`. +3. The base configuration supplied to `PkceAuthProvider::new`. + +Scopes follow the same pattern: the resolved environment's scopes when non-empty, otherwise the provider's base scopes. + +If `Environments::resolve` fails — because the name is unknown or the environments file cannot be parsed — the provider logs the error at `DEBUG` level and falls back to the next source. +No token or secret is included in the log; only the environment name and error message appear. + +## Example + +```rust,no_run +use std::sync::Arc; +use cli_engine::{ + BuildInfo, Cli, CliConfig, + auth::pkce::PkceAuthProvider, + environments::{EnvironmentDef, Environments}, +}; + +// Build one Arc and share it. `with_app_id` must match the +// CliConfig app_id ("my-cli") so the environments.toml file path resolves. +let environments = Arc::new( + Environments::new("prod") + .with_app_id("my-cli") + .with_environment( + "prod", + EnvironmentDef::new() + .with_client_id("prod-client-id") + .with_auth_url("https://api.example.com/v2/oauth2/authorize") + .with_token_url("https://api.example.com/v2/oauth2/token") + .with_scopes(&["openid", "profile"]) + .with_field("api_url", "https://api.example.com"), + ) + .with_environment( + "ote", + EnvironmentDef::new() + .with_client_id("ote-client-id") + .with_auth_url("https://api.ote.example.com/v2/oauth2/authorize") + .with_token_url("https://api.ote.example.com/v2/oauth2/token") + .with_field("api_url", "https://api.ote.example.com"), + ) + .with_config_file(true), +); + +let provider = Arc::new( + PkceAuthProvider::new( + "primary", + "https://api.example.com/v2/oauth2/authorize", + "https://api.example.com/v2/oauth2/token", + "fallback-client-id", + &["openid"], + ) + .with_environments(Arc::clone(&environments)), +); + +let cli = Cli::new( + CliConfig::new("my-cli", "My CLI", "my-cli") + .with_build(BuildInfo::new(env!("CARGO_PKG_VERSION"))) + .with_default_auth_provider("primary") + .with_auth_provider(provider) + // The same Arc the provider was wired with — not a separate copy — so the + // file layer and active-env persistence resolve identically for both. + .with_environments(environments), +); +``` + +With this setup: + +- Running `my-cli env list` prints `ote` and `prod`, marking whichever is active. +- Running `my-cli env set ote` persists `ote` as active; subsequent invocations target OTE. +- Running `my-cli --env prod ` overrides the active environment for that invocation only. +- `PROD_OAUTH_CLIENT_ID=override my-cli --env prod auth login` injects the override at the env-var layer. +- A user-supplied `environments.toml` in the config directory can add new environments or override fields without recompiling. diff --git a/src/auth/pkce.rs b/src/auth/pkce.rs index 148d3b3..25048c3 100644 --- a/src/auth/pkce.rs +++ b/src/auth/pkce.rs @@ -41,7 +41,15 @@ //! .with_auth_provider(provider); //! ``` //! -//! Override endpoints and client ID via environment variables: +//! For per-environment OAuth config (different client id or endpoints per env), +//! wire the provider to a shared +//! [`Environments`](crate::environments::Environments) with +//! [`PkceAuthProvider::with_environments`](crate::auth::pkce::PkceAuthProvider::with_environments); +//! the resolved environment then drives the OAuth config for the active `env`. +//! +//! Without a wired resolver (or for a field the resolved environment leaves +//! empty), endpoints and client ID can still be overridden via environment +//! variables: //! - `_OAUTH_CLIENT_ID` //! - `_OAUTH_AUTH_URL` //! - `_OAUTH_TOKEN_URL` @@ -77,6 +85,9 @@ use crate::{ const REDIRECT_PORT_DEFAULT: u16 = 7443; const TOKEN_EXPIRY_BUFFER_SECS: i64 = 30; +/// Default timeout applied to OAuth token-endpoint requests (exchange/refresh) +/// so a stalled token server cannot hang the CLI indefinitely. +const TOKEN_REQUEST_TIMEOUT_DEFAULT: Duration = Duration::from_secs(30); /// Stored token with expiry tracking. /// @@ -124,6 +135,17 @@ impl StoredToken { } } +/// The effective OAuth values for an environment, computed from a single +/// environment resolution. A token flow resolves this once and reuses it across +/// the authorize URL, code exchange, and refresh, rather than re-reading and +/// re-parsing `environments.toml` once per field. +struct EffectiveOAuth { + client_id: String, + auth_url: String, + token_url: String, + scopes: Vec, +} + /// OAuth 2.0 PKCE authentication provider. /// /// Stores one token per `(env, provider)` pair in the system keychain. @@ -135,8 +157,20 @@ pub struct PkceAuthProvider { token_url: String, client_id: String, scopes: Vec, + /// Optional environment resolver; when set, per-env OAuth config comes from + /// the resolved environment instead of the base config / legacy env override. + /// Looked up by the `env` passed to [`AuthProvider::get_credential`]. + environments: Option>, redirect_port: u16, redirect_uri: Option, + /// Timeout applied to token-endpoint requests (exchange and refresh). + token_timeout: Duration, + /// Shared HTTP client for token-endpoint traffic, built once and reused by + /// exchange and refresh so connections and TLS configuration are pooled + /// rather than rebuilt per request. The user-agent and timeout are applied + /// per request (not baked into the client) so they reflect the value + /// published at execution time, not at provider construction. + client: reqwest::Client, app_id: String, env_prefix: String, /// Explicit storage backend injected via [`PkceAuthProvider::with_storage`]. @@ -183,8 +217,11 @@ impl PkceAuthProvider { token_url: token_url.into(), client_id: client_id.into(), scopes: scopes.iter().map(|s| s.as_ref().to_owned()).collect(), + environments: None, redirect_port: REDIRECT_PORT_DEFAULT, redirect_uri: None, + token_timeout: TOKEN_REQUEST_TIMEOUT_DEFAULT, + client: reqwest::Client::new(), app_id: String::new(), env_prefix, storage_override: None, @@ -198,6 +235,62 @@ impl PkceAuthProvider { } } + /// Sources per-environment OAuth config from a shared + /// [`Environments`](crate::environments::Environments). + /// + /// Given an `env`, the provider resolves the environment and uses its + /// [`OAuthConfig`](crate::environments::OAuthConfig). This is the + /// single-source-of-truth path; prefer it over the base + /// `client_id`/`auth_url`/`token_url` when the consumer registers + /// environments via + /// [`CliConfig::with_environments`](crate::CliConfig::with_environments). + /// + /// Precedence per OAuth field, for the resolved env: the resolved + /// environment's value when non-empty; otherwise the legacy + /// provider-prefixed env var (`_OAUTH_CLIENT_ID`, `_AUTH_URL`, + /// `_TOKEN_URL`); otherwise the base configuration supplied to + /// [`PkceAuthProvider::new`]. An empty field on the resolved environment + /// falls through, so a partial environment can override only the client id + /// while inheriting the provider's base endpoints. + /// + /// # Examples + /// + /// ``` + /// use std::sync::Arc; + /// use cli_engine::{ + /// auth::pkce::PkceAuthProvider, + /// environments::{EnvironmentDef, Environments}, + /// }; + /// + /// let environments = Arc::new( + /// Environments::new("prod").with_environment( + /// "dev", + /// EnvironmentDef::new() + /// .with_client_id("dev-client-id") + /// .with_auth_url("https://api.dev-godaddy.com/v2/oauth2/authorize") + /// .with_token_url("https://api.dev-godaddy.com/v2/oauth2/token"), + /// ), + /// ); + /// + /// let provider = PkceAuthProvider::new( + /// "godaddy", + /// "https://api.godaddy.com/v2/oauth2/authorize", + /// "https://api.godaddy.com/v2/oauth2/token", + /// "prod-client-id", + /// &["openid", "profile"], + /// ) + /// .with_environments(environments); + /// # let _ = provider; + /// ``` + #[must_use] + pub fn with_environments( + mut self, + environments: Arc, + ) -> Self { + self.environments = Some(environments); + self + } + /// Sets the local redirect server port (default: 7443). #[must_use] pub fn with_redirect_port(mut self, port: u16) -> Self { @@ -205,6 +298,17 @@ impl PkceAuthProvider { self } + /// Sets the timeout applied to token-endpoint requests (authorization-code + /// exchange and refresh). + /// + /// Defaults to 30 seconds. This bounds only the HTTP token requests; the + /// interactive browser/callback wait has its own separate timeout. + #[must_use] + pub fn with_token_timeout(mut self, timeout: Duration) -> Self { + self.token_timeout = timeout; + self + } + /// Overrides the redirect URI sent to the authorization server. /// /// By default the redirect URI is `http://127.0.0.1:{port}/callback`. Use @@ -317,19 +421,71 @@ impl PkceAuthProvider { } } - fn effective_client_id(&self) -> String { - let key = format!("{}_OAUTH_CLIENT_ID", self.env_prefix); - std::env::var(&key).unwrap_or_else(|_| self.client_id.clone()) + /// Resolves the [`OAuthConfig`](crate::environments::OAuthConfig) for `env` + /// from the wired [`Environments`](crate::environments::Environments), if + /// any. Returns `None` when no resolver is wired, the env does not resolve, + /// or the resolved environment carries no OAuth slice. + fn resolved_oauth(&self, env: &str) -> Option { + let envs = self.environments.as_ref()?; + match envs.resolve(env) { + Ok(resolved) => resolved.oauth, + Err(e) => { + tracing::debug!( + env, + error = %e, + "environment resolve failed; falling back to base OAuth config" + ); + None + } + } } - fn effective_auth_url(&self) -> String { - let key = format!("{}_OAUTH_AUTH_URL", self.env_prefix); - std::env::var(&key).unwrap_or_else(|_| self.auth_url.clone()) + /// Computes the effective OAuth config for `env` with a SINGLE environment + /// resolution (at most one `environments.toml` read), then applies the + /// resolved → `_OAUTH_*` env var → base precedence to each field. + /// + /// Token flows call this once and reuse the result so they don't re-read the + /// environments file once per field. + fn effective_oauth(&self, env: &str) -> EffectiveOAuth { + let resolved = self.resolved_oauth(env); + // Resolved value when non-empty, else the provider-prefixed env var, else base. + let field = |resolved_value: Option<&String>, var_suffix: &str, base: &str| -> String { + if let Some(value) = resolved_value + && !value.is_empty() + { + return value.clone(); + } + std::env::var(format!("{}_OAUTH_{var_suffix}", self.env_prefix)) + .unwrap_or_else(|_| base.to_owned()) + }; + let scopes = match &resolved { + Some(oauth) if !oauth.scopes.is_empty() => oauth.scopes.clone(), + _ => self.scopes.clone(), + }; + EffectiveOAuth { + client_id: field( + resolved.as_ref().map(|o| &o.client_id), + "CLIENT_ID", + &self.client_id, + ), + auth_url: field( + resolved.as_ref().map(|o| &o.auth_url), + "AUTH_URL", + &self.auth_url, + ), + token_url: field( + resolved.as_ref().map(|o| &o.token_url), + "TOKEN_URL", + &self.token_url, + ), + scopes, + } } - fn effective_token_url(&self) -> String { - let key = format!("{}_OAUTH_TOKEN_URL", self.env_prefix); - std::env::var(&key).unwrap_or_else(|_| self.token_url.clone()) + /// Default scopes for `env`: the resolved environment's scopes when + /// non-empty, otherwise the provider's base scopes. + fn effective_scopes(&self, env: &str) -> Vec { + self.effective_oauth(env).scopes } fn effective_redirect_uri(&self) -> String { @@ -424,7 +580,8 @@ impl PkceAuthProvider { if let Some(token) = self.existing_token(env).await? { return Ok(token); } - self.reauthenticate(env, &self.scopes).await + let scopes = self.effective_scopes(env); + self.reauthenticate(env, &scopes).await } /// Returns a usable token from the in-memory cache, keychain, or a refresh — @@ -442,7 +599,7 @@ impl PkceAuthProvider { } if let Some(refresh_token) = token.refresh_token.as_deref() && let Ok(mut refreshed) = self - .refresh_access_token(refresh_token, &token.scopes) + .refresh_access_token(env, refresh_token, &token.scopes) .await { if refreshed.refresh_token.is_none() { @@ -459,7 +616,7 @@ impl PkceAuthProvider { /// Runs a fresh interactive PKCE flow requesting exactly `scopes`, replacing /// any stored token for `env`. async fn reauthenticate(&self, env: &str, scopes: &[String]) -> Result { - let token = self.run_pkce_flow_with(scopes).await?; + let token = self.run_pkce_flow_with(env, scopes).await?; // Persist first — the keychain write overwrites the existing entry for // this env — and only update the in-memory cache after a successful // save. This avoids destroying a still-valid token if the save fails @@ -471,24 +628,24 @@ impl PkceAuthProvider { /// Runs the browser PKCE flow requesting exactly `scopes` (used both for the /// default login and for scope step-up, which requests a wider union). - async fn run_pkce_flow_with(&self, scopes: &[String]) -> Result { + async fn run_pkce_flow_with(&self, env: &str, scopes: &[String]) -> Result { let (code_verifier, code_challenge) = pkce_challenge(); let state = random_state(); - let client_id = self.effective_client_id(); - let auth_url = self.effective_auth_url(); + // Resolve the OAuth config once for this whole flow (authorize + exchange). + let oauth = self.effective_oauth(env); let redirect_uri = self.effective_redirect_uri(); let scope = scopes.join(" "); let auth_params = [ ("response_type", "code"), - ("client_id", &client_id), + ("client_id", &oauth.client_id), ("redirect_uri", &redirect_uri), ("scope", &scope), ("state", &state), ("code_challenge", &code_challenge), ("code_challenge_method", "S256"), ]; - let url = url::Url::parse_with_params(&auth_url, &auth_params) + let url = url::Url::parse_with_params(&oauth.auth_url, &auth_params) .map_err(|err| CliCoreError::message(format!("invalid auth URL: {err}")))?; let (bind_port, callback_path) = self.parse_redirect_uri()?; @@ -507,30 +664,47 @@ impl PkceAuthProvider { let code = wait_for_callback(listener, &state, &callback_path, Duration::from_secs(120)).await?; - self.exchange_code_for_token(&code, &code_verifier, scopes) + self.exchange_code_for_token(&oauth, &code, &code_verifier, scopes) .await } + /// Builds a POST to an OAuth token endpoint on the provider's shared client. + /// + /// Token traffic does not go through [`HttpClient`](crate::transport::HttpClient) + /// — that client is built for authenticated, JSON-bodied backend calls, + /// whereas the token endpoint is unauthenticated and form-encoded. The + /// user-agent and timeout are attached here per request (read at call time) + /// so every outbound call, including credential acquisition and refresh, is + /// attributed consistently and bounded. + fn token_request(&self, token_url: &str, params: &[(&str, &str)]) -> reqwest::RequestBuilder { + self.client + .post(token_url) + .header( + reqwest::header::USER_AGENT, + crate::transport::client::default_user_agent(), + ) + .timeout(self.token_timeout) + .form(params) + } + async fn exchange_code_for_token( &self, + oauth: &EffectiveOAuth, code: &str, code_verifier: &str, requested_scopes: &[String], ) -> Result { let redirect_uri = self.effective_redirect_uri(); - let client_id = self.effective_client_id(); - let token_url = self.effective_token_url(); let params = [ ("grant_type", "authorization_code"), - ("client_id", &client_id), + ("client_id", &oauth.client_id), ("redirect_uri", &redirect_uri), ("code", code), ("code_verifier", code_verifier), ]; - let response = reqwest::Client::new() - .post(&token_url) - .form(¶ms) + let response = self + .token_request(&oauth.token_url, ¶ms) .send() .await .map_err(|err| CliCoreError::message(format!("token request failed: {err}")))?; @@ -548,19 +722,18 @@ impl PkceAuthProvider { async fn refresh_access_token( &self, + env: &str, refresh_token: &str, prior_scopes: &[String], ) -> Result { - let client_id = self.effective_client_id(); - let token_url = self.effective_token_url(); + let oauth = self.effective_oauth(env); let params = [ ("grant_type", "refresh_token"), - ("client_id", &client_id), + ("client_id", &oauth.client_id), ("refresh_token", refresh_token), ]; - let response = reqwest::Client::new() - .post(&token_url) - .form(¶ms) + let response = self + .token_request(&oauth.token_url, ¶ms) .send() .await .map_err(|err| CliCoreError::message(format!("token refresh failed: {err}")))?; @@ -602,14 +775,16 @@ impl AuthProvider for PkceAuthProvider { // server has no silent scope-expansion grant, so in non-interactive // contexts we fail fast rather than hang on the callback timeout. let granted = granted_scopes(&token); - match plan_step_up(&self.scopes, &granted, required, session_is_interactive()) { + match plan_step_up(&granted, required, session_is_interactive()) { StepUp::Covered => return Ok(self.build_credential(env, &token)), StepUp::MissingNonInteractive(missing) => { return Err(missing_scope_error(env, &missing)); } - // Union (defaults ∪ already-granted ∪ required) so step-up never - // drops scopes acquired by an earlier login or step-up. - StepUp::Reauthenticate(union) => { + // Re-consent: resolve per-env defaults only now (off the + // cached-token hot path) and request defaults ∪ already-granted ∪ + // required so step-up never drops previously-acquired scopes. + StepUp::Reauthenticate => { + let union = union_scopes(&self.effective_scopes(env), &granted, required); let token = self.reauthenticate(env, &union).await?; ensure_granted(env, &token, required)?; return Ok(self.build_credential(env, &token)); @@ -618,7 +793,7 @@ impl AuthProvider for PkceAuthProvider { } // No usable token: authenticate once, requesting defaults ∪ required. - let union = union_scopes(&self.scopes, &[], required); + let union = union_scopes(&self.effective_scopes(env), &[], required); let token = self.reauthenticate(env, &union).await?; ensure_granted(env, &token, required)?; Ok(self.build_credential(env, &token)) @@ -870,19 +1045,22 @@ fn granted_scopes(token: &StoredToken) -> Vec { enum StepUp { /// The token already covers every required scope. Covered, - /// Re-authenticate requesting this scope set (defaults ∪ granted ∪ required). - Reauthenticate(Vec), + /// Re-authenticate (interactively) to acquire the missing scopes. The caller + /// builds the requested set (defaults ∪ granted ∪ required) only on this + /// path, so resolving per-env default scopes stays off the cached-token hot + /// path. + Reauthenticate, /// Scopes are missing but the session is non-interactive, so step-up cannot /// prompt; carries the missing scopes for the error message. MissingNonInteractive(Vec), } -fn plan_step_up( - defaults: &[String], - granted: &[String], - required: &[String], - interactive: bool, -) -> StepUp { +/// Decides the step-up action from what the token grants versus what the command +/// requires. Deliberately does NOT take the per-env default scopes: the coverage +/// decision needs only `granted`/`required`, so the caller can avoid resolving +/// defaults (potential `environments.toml` I/O) when a cached token already +/// covers the requirement. +fn plan_step_up(granted: &[String], required: &[String], interactive: bool) -> StepUp { let missing: Vec = required .iter() .filter(|scope| !granted.iter().any(|have| have == *scope)) @@ -891,7 +1069,7 @@ fn plan_step_up( if missing.is_empty() { StepUp::Covered } else if interactive { - StepUp::Reauthenticate(union_scopes(defaults, granted, required)) + StepUp::Reauthenticate } else { StepUp::MissingNonInteractive(missing) } @@ -990,6 +1168,7 @@ async fn parse_token_response( } #[cfg(test)] +#[allow(unsafe_code)] mod tests { use serde_json::json; @@ -1035,6 +1214,100 @@ mod tests { } } + fn envs_for_test() -> Arc { + use crate::environments::{EnvironmentDef, Environments}; + Arc::new( + Environments::new("prod").with_environment( + "prod", + EnvironmentDef::new() + .with_client_id("prod-client") + .with_auth_url("https://prod.example.com/auth") + .with_token_url("https://prod.example.com/token") + .with_scopes(&["openid", "prod.read"]), + ), + ) + } + + /// A provider wired to an [`Environments`](crate::environments::Environments) + /// resolver sources its per-env OAuth config (client id, endpoints, scopes) + /// from the resolved environment, making the environment the single source + /// of truth. + #[test] + fn environment_wired_provider_sources_oauth_from_resolver() { + let provider = PkceAuthProvider::new( + "godaddy", + "https://base/auth", + "https://base/token", + "base-client", + &["openid"], + ) + .with_environments(envs_for_test()); + let oauth = provider.effective_oauth("prod"); + assert_eq!(oauth.client_id, "prod-client"); + assert_eq!(oauth.auth_url, "https://prod.example.com/auth"); + assert_eq!(oauth.token_url, "https://prod.example.com/token"); + assert_eq!( + oauth.scopes, + vec!["openid".to_owned(), "prod.read".to_owned()] + ); + } + + /// A provider with no environment resolver falls back to the base client id, + /// endpoints, and scopes for every env. + #[test] + fn non_wired_provider_uses_base_config() { + let provider = PkceAuthProvider::new( + "godaddy", + "https://base/auth", + "https://base/token", + "base-client", + &["openid"], + ); + let oauth = provider.effective_oauth("anything"); + assert_eq!(oauth.client_id, "base-client"); + assert_eq!(oauth.scopes, vec!["openid".to_owned()]); + } + + /// OAuth token traffic must carry the engine's configured default + /// user-agent so it is attributed consistently with all other outbound + /// calls (some upstream WAFs reject requests without a User-Agent). + #[test] + fn token_request_carries_default_user_agent() { + let _guard = crate::transport::client::UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = crate::transport::client::RestoreDefaultUserAgent; + crate::transport::set_default_user_agent("ua-probe/7.7"); + let provider = test_provider().with_token_timeout(Duration::from_secs(12)); + let request = provider + .token_request( + "https://example.com/token", + &[("grant_type", "refresh_token")], + ) + .build() + .expect("token request should build"); + let header = request + .headers() + .get(reqwest::header::USER_AGENT) + .expect("token request should set a user-agent"); + assert_eq!(header, "ua-probe/7.7"); + assert_eq!(request.timeout(), Some(&Duration::from_secs(12))); + } + + /// OAuth token requests must not hang indefinitely: the provider applies a + /// 30s timeout by default. + #[test] + fn default_token_timeout_is_thirty_seconds() { + assert_eq!(test_provider().token_timeout, Duration::from_secs(30)); + } + + /// The default token timeout can be overridden per provider. + #[test] + fn with_token_timeout_overrides_default() { + let provider = test_provider().with_token_timeout(Duration::from_secs(5)); + assert_eq!(provider.token_timeout, Duration::from_secs(5)); + } + /// store_cached_token + cached_token round-trip: the mechanism used by /// the persistence fix must reliably write and read tokens from the cache. #[tokio::test] @@ -1114,30 +1387,22 @@ mod tests { #[test] fn plan_step_up_covers_reauths_and_fails_non_interactive() { - let defaults = vec!["base".to_owned()]; let granted = vec!["base".to_owned(), "read".to_owned()]; let read = vec!["read".to_owned()]; let write = vec!["write".to_owned()]; - // Already covered. - assert_eq!( - plan_step_up(&defaults, &granted, &read, true), - StepUp::Covered - ); - // Missing + interactive → reauth requesting the union. - assert_eq!( - plan_step_up(&defaults, &granted, &write, true), - StepUp::Reauthenticate(vec![ - "base".to_owned(), - "read".to_owned(), - "write".to_owned() - ]) - ); + // Already covered (decision needs only granted vs required). + assert_eq!(plan_step_up(&granted, &read, true), StepUp::Covered); + // Missing + interactive → reauth (the caller builds the union with + // per-env defaults only on this path). + assert_eq!(plan_step_up(&granted, &write, true), StepUp::Reauthenticate); // Missing + non-interactive → fail fast, carrying the missing scopes. assert_eq!( - plan_step_up(&defaults, &granted, &write, false), + plan_step_up(&granted, &write, false), StepUp::MissingNonInteractive(vec!["write".to_owned()]) ); + // The union itself (defaults ∪ granted ∪ required) is covered by + // union_scopes' own test. } /// An opaque cached token whose recorded scopes cover the requirement is @@ -1554,4 +1819,95 @@ mod tests { let cred = provider.status("dev").await.expect("status"); assert_eq!(cred.token, "filetok"); } + + /// Serializes env-var access and removes the vars on drop (matching the + /// convention in `src/environments.rs` tests). Vars persist process-wide, so + /// concurrent tests would otherwise see each other's writes. + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// RAII guard removing an env var on drop, even on panic, while ENV_LOCK held. + struct EnvGuard(&'static str); + impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: the test holds ENV_LOCK for the guard's lifetime. + unsafe { std::env::remove_var(self.0) } + } + } + + /// The fix: a provider wired to a shared `Arc` whose + /// `environments.toml` file layer defines `prod` with a different `client_id` + /// resolves the FILE's client id. This proves the provider's file layer + /// resolves — the shared, app_id-stamped instance reaches the provider rather + /// than an unstamped copy whose file path is `None`. + #[test] + fn wired_provider_resolves_client_id_from_environments_file() { + use crate::environments::{EnvironmentDef, Environments}; + + let dir = tempfile::tempdir().expect("tempdir"); + let file = dir.path().join("environments.toml"); + std::fs::write( + &file, + r#" +[prod] +client_id = "file-prod-client" +"#, + ) + .expect("write environments.toml"); + + let environments = Arc::new( + Environments::new("prod") + .with_app_id("x") + .with_environment( + "prod", + EnvironmentDef::new().with_client_id("compiled-prod-client"), + ) + .with_config_file_path_override(file), + ); + + let provider = PkceAuthProvider::new( + "godaddy", + "https://base/auth", + "https://base/token", + "base-client", + &["openid"], + ) + .with_environments(environments); + + // The file overrides the compiled client id, which itself overrides the + // provider's base — proving the wired provider reads the file layer. + assert_eq!( + provider.effective_oauth("prod").client_id, + "file-prod-client" + ); + } + + /// Legacy escape hatch: a NON-wired provider still honors the provider-prefixed + /// `_OAUTH_CLIENT_ID` env var, and a wired provider whose resolved env + /// yields a non-empty client id takes precedence over that legacy var. + #[test] + fn legacy_oauth_client_id_env_var_overrides_base_but_yields_to_wired_env() { + use crate::environments::{EnvironmentDef, Environments}; + + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // `` = provider name uppercased, '-' -> '_'. Provider is "test". + // SAFETY: serialized by ENV_LOCK; guard removes the var on any exit. + unsafe { std::env::set_var("TEST_OAUTH_CLIENT_ID", "legacy-client") }; + let _guard = EnvGuard("TEST_OAUTH_CLIENT_ID"); + + // Non-wired provider: the legacy var overrides the base client id. + let bare = test_provider(); + assert_eq!(bare.effective_oauth("prod").client_id, "legacy-client"); + + // Wired provider whose resolved env carries a non-empty client id: the + // resolved environment wins over the legacy var. + let environments = Arc::new( + Environments::new("prod") + .with_app_id("x") + .with_environment("prod", EnvironmentDef::new().with_client_id("env-client")), + ); + let wired = test_provider().with_environments(environments); + assert_eq!(wired.effective_oauth("prod").client_id, "env-client"); + } } diff --git a/src/cli.rs b/src/cli.rs index 0e0e5cb..0d50de7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -210,6 +210,10 @@ pub struct CliConfig { pub views: Vec, /// Providers registered before command execution starts. pub auth_providers: Vec>, + /// Optional override for the process-wide outbound User-Agent. When unset, + /// the engine derives `name/version` from this config. See + /// [`CliConfig::user_agent_string`]. + pub user_agent: Option, /// Optional authorization gatekeeper injected into middleware. pub authz: Option>, /// Optional audit recorder injected into middleware. @@ -250,6 +254,13 @@ pub struct CliConfig { /// to a binary that never opted in. Populate via [`CliConfig::with_argv0_alias`] /// and [`CliConfig::with_argv0_personality`]. pub argv0_routes: BTreeMap, + /// Optional first-class environment system. + /// + /// Registered via [`CliConfig::with_environments`]. When set, the engine + /// registers a global `--env` flag, seeds the active environment into + /// middleware, and exposes it to handlers through + /// [`CommandContext::environment`](crate::command::CommandContext::environment). + pub environments: Option>, } impl CliConfig { @@ -289,6 +300,75 @@ impl CliConfig { self } + /// Registers a first-class environment system. + /// + /// When set, [`Cli::new`] registers a global `--env` flag, seeds the active + /// environment into middleware (explicit `--env` > persisted active > + /// configured default), and exposes the resolved environment to handlers via + /// [`CommandContext::environment`](crate::command::CommandContext::environment). + /// + /// The [`Environments`](crate::environments::Environments) is stored as-is, so + /// the consumer is responsible for configuring it before wrapping it in an + /// `Arc`: + /// + /// - Call + /// [`Environments::with_app_id`](crate::environments::Environments::with_app_id) + /// with the **same** `app_id` passed to [`CliConfig::new`], so the config + /// file and active-environment persistence resolve to the application's + /// config directory. (An empty `app_id` makes + /// [`Environments::config_file_path`](crate::environments::Environments::config_file_path) + /// return `None`, silently disabling the `environments.toml` file layer.) + /// - Call + /// [`Environments::with_config_file(true)`](crate::environments::Environments::with_config_file) + /// if the application loads a user-editable `environments.toml`. + /// - **Share the same `Arc`** with any `PkceAuthProvider::with_environments` + /// (available with the `pkce-auth` feature): + /// the provider's OAuth file layer and active-environment persistence must + /// resolve against the identical, `app_id`-stamped instance the engine sees, + /// or a file-defined environment (or a file override of a compiled + /// environment's `client_id`) will be visible to `env info` yet invisible to + /// the actual OAuth login. + #[must_use] + pub fn with_environments( + mut self, + environments: Arc, + ) -> Self { + self.environments = Some(environments); + self + } + + /// Overrides the outbound User-Agent string for all HTTP traffic. + /// + /// When unset, the engine derives `name/version` from this config (see + /// [`CliConfig::user_agent_string`]). Set this when the upstream APIs expect + /// a specific product token. The resolved value is applied process-wide on + /// execution via [`crate::transport::set_default_user_agent`], so it reaches + /// both command [`HttpClient`](crate::transport::HttpClient)s and the + /// engine's own OAuth token requests. + #[must_use] + pub fn with_user_agent(mut self, user_agent: impl Into) -> Self { + self.user_agent = Some(user_agent.into()); + self + } + + /// Returns the outbound User-Agent string the CLI presents on HTTP requests. + /// + /// Resolution order: + /// 1. an explicit [`with_user_agent`](Self::with_user_agent) override; + /// 2. otherwise `name/version` (for example `gdx/1.2.3`); + /// 3. otherwise just `name` when no build version is set. + #[must_use] + pub fn user_agent_string(&self) -> String { + if let Some(user_agent) = &self.user_agent { + return user_agent.clone(); + } + if self.build.version.is_empty() { + self.name.clone() + } else { + format!("{}/{}", self.name, self.build.version) + } + } + /// Adds one domain module. #[must_use] pub fn with_module(mut self, module: Module) -> Self { @@ -688,6 +768,15 @@ impl Cli { if let Some(register_flags) = &config.register_flags { root = register_flags(root); } + if config.environments.is_some() { + root = root.arg( + clap::Arg::new("env") + .long("env") + .global(true) + .value_name("ENV") + .help("Override the active environment (see: env list)"), + ); + } let intro = config .long .as_deref() @@ -712,6 +801,14 @@ impl Cli { middleware .human_views .merge(&global_human_view_registry_snapshot()); + if let Some(environments) = &config.environments { + // Seed the sticky/default active environment now; the global `--env` + // flag overrides it per invocation in `run_with_depth`. The same + // `Arc` the consumer shared with any `PkceAuthProvider` is reused, so + // the file layer and active-env persistence resolve consistently. + middleware.env = environments.effective_active(None, &middleware.config); + middleware.environments = Some(Arc::clone(environments)); + } let mut cli = Self { config, @@ -753,6 +850,9 @@ impl Cli { if cli.config.config_commands { cli.ensure_config_command(); } + if cli.config.environments.is_some() { + cli.ensure_env_command(); + } cli } @@ -845,6 +945,7 @@ impl Cli { E: Write, Shutdown: Future, { + self.install_default_user_agent(); let output = run_until_signal(self.run(args), shutdown).await; if output.exit_code == 130 && output.rendered == "command interrupted\n" @@ -860,6 +961,17 @@ impl Cli { Ok(process_exit_code(output.exit_code)) } + /// Publishes the configured outbound User-Agent process-wide so that + /// command [`HttpClient`](crate::transport::HttpClient)s and the engine's + /// own OAuth token requests share it. + /// + /// Called from the execution entrypoints rather than [`Cli::new`] so that + /// merely constructing a `Cli` (as tests do in bulk) does not mutate global + /// state. See [`CliConfig::user_agent_string`] for resolution order. + fn install_default_user_agent(&self) { + crate::transport::set_default_user_agent(self.config.user_agent_string()); + } + /// Registers an auth provider after construction. pub fn register_auth_provider(&mut self, provider: Arc) -> &mut Self { self.middleware.auth.register(provider); @@ -1294,6 +1406,11 @@ impl Cli { if let Err(err) = self.apply_config_flags(&matches, &mut middleware) { return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); } + // Validate and apply `--env` for built-in paths (help/tree/guide/group + // help) so they reflect the selected environment and reject unknowns. + if let Err(err) = self.apply_env_flag(&matches, &mut middleware) { + return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); + } let command_path = command_path_from_matches(&self.config.name, &matches); if command_path == "help" { @@ -1376,6 +1493,11 @@ impl Cli { if let Err(err) = self.apply_config_flags(&matches, &mut middleware) { return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); } + // The global `--env` flag overrides the seeded active environment for + // this invocation; an unknown name surfaces as an error envelope. + if let Err(err) = self.apply_env_flag(&matches, &mut middleware) { + return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); + } let leaf = leaf_matches(&matches); let args = command_args_from_matches(leaf, &command.spec, false); @@ -1820,6 +1942,38 @@ impl Cli { self.refresh_root_long(); } + /// Mounts the built-in `env` command group and files it under the admin + /// help category. Idempotent and yields to a consumer-defined `env` + /// subcommand if one already exists. + fn ensure_env_command(&mut self) { + if has_subcommand(&self.root, "env") { + return; + } + let group = crate::env_commands::env_command_group(); + let mut prefix = Vec::new(); + group.register_commands(&mut prefix, &mut self.commands); + let mut prefix = Vec::new(); + let clap_group = runtime_group_clap_command_with_schema_help( + &group, + &mut prefix, + &self.middleware.schema_registry, + ); + self.root = self.root.clone().subcommand(clap_group); + let category = self + .config + .admin_category + .clone() + .unwrap_or_else(|| DEFAULT_ADMIN_CATEGORY.to_owned()); + if !self.module_entries.iter().any(|e| e.name == "env") { + self.module_entries.push(ModuleHelpEntry { + category, + name: "env".to_owned(), + short: "Manage the active environment".to_owned(), + }); + } + self.refresh_root_long(); + } + fn default_auth_provider(&self) -> String { if !self.middleware.default_auth_provider.is_empty() { return self.middleware.default_auth_provider.clone(); @@ -1858,6 +2012,28 @@ impl Cli { Ok(()) } + /// Applies the global `--env` override to a per-run middleware snapshot. + /// + /// The flag is only registered when environments are configured, so when it + /// is present `middleware.environments` is set too. Validates the requested + /// name against the registered environments and updates `middleware.env`, + /// returning an error for an unknown environment. + fn apply_env_flag(&self, matches: &ArgMatches, middleware: &mut Middleware) -> Result<()> { + // Guard on the environment system FIRST. The `--env` arg is only + // registered when environments are configured (the same condition that + // sets `middleware.environments`); calling `matches.get_one("env")` for + // an arg that was never registered panics in clap, which would break + // every CLI that does not use environments. + let Some(environments) = middleware.environments.as_ref() else { + return Ok(()); + }; + if let Some(env) = matches.get_one::("env") { + environments.resolve(env)?; + middleware.env = env.clone(); + } + Ok(()) + } + fn run_pre_run( &self, middleware: &mut Middleware, @@ -2784,3 +2960,105 @@ async fn run_streaming_command( }), } } + +#[cfg(test)] +mod user_agent_tests { + use super::*; + + #[test] + fn user_agent_string_derives_name_and_version_by_default() { + let config = + CliConfig::new("gdx", "GoDaddy CLI", "gdx").with_build(BuildInfo::new("1.2.3")); + assert_eq!(config.user_agent_string(), "gdx/1.2.3"); + } + + #[test] + fn user_agent_string_prefers_explicit_override() { + let config = CliConfig::new("gdx", "GoDaddy CLI", "gdx") + .with_build(BuildInfo::new("1.2.3")) + .with_user_agent("gdx-cli/9.9 (custom)"); + assert_eq!(config.user_agent_string(), "gdx-cli/9.9 (custom)"); + } + + #[test] + fn user_agent_string_omits_version_when_absent() { + let config = CliConfig::new("gdx", "GoDaddy CLI", "gdx"); + assert_eq!(config.user_agent_string(), "gdx"); + } + + #[test] + fn install_default_user_agent_publishes_config_value() { + let _guard = crate::transport::client::UA_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _restore = crate::transport::client::RestoreDefaultUserAgent; + crate::transport::set_default_user_agent("cli/dev"); + let cli = Cli::new( + CliConfig::new("uatest", "UA test", "uatest").with_build(BuildInfo::new("4.5.6")), + ); + cli.install_default_user_agent(); + assert_eq!( + crate::transport::client::default_user_agent(), + "uatest/4.5.6" + ); + } +} + +#[cfg(test)] +mod env_config_tests { + use super::*; + + #[test] + fn with_environments_stores_shared_arc_with_consumer_app_id() { + // The consumer sets app_id on the Environments before sharing the Arc; + // CliConfig stores it as-is, so the file path resolves only because the + // consumer stamped the matching app_id (not because the engine did). + let cfg = CliConfig::new("gddy", "GoDaddy CLI", "gddy").with_environments(Arc::new( + crate::environments::Environments::new("prod") + .with_app_id("gddy") + .with_config_file(true), + )); + let envs = cfg.environments.as_ref().expect("environments set"); + assert!(envs.config_file_path().is_some()); + } + + #[tokio::test] + async fn env_flag_overrides_default_and_reaches_middleware_env() { + use crate::{CommandResult, CommandSpec, RuntimeCommandSpec}; + use serde_json::json; + let mut cli = Cli::new( + CliConfig::new("envtest", "Env test", "envtest").with_environments(Arc::new( + crate::environments::Environments::new("prod") + .with_environment("prod", crate::environments::EnvironmentDef::new()) + .with_environment("ote", crate::environments::EnvironmentDef::new()), + )), + ); + cli.add_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("whichenv", "echo env").no_auth(true), + async |ctx| { + Ok(CommandResult::new( + json!({ "env": ctx.environment()?.name }), + )) + }, + )); + let out = cli + .run(["envtest", "whichenv", "--env", "ote", "--output", "json"]) + .await; + assert_eq!(out.exit_code, 0, "rendered: {}", out.rendered); + assert!(out.rendered.contains("\"env\"")); + assert!(out.rendered.contains("ote")); + } + + #[tokio::test] + async fn unknown_env_flag_produces_error_envelope() { + let cli = Cli::new( + CliConfig::new("envtest2", "Env test", "envtest2").with_environments(Arc::new( + crate::environments::Environments::new("prod") + .with_environment("prod", crate::environments::EnvironmentDef::new()), + )), + ); + let out = cli.run(["envtest2", "tree", "--env", "nope"]).await; + assert_ne!(out.exit_code, 0); + assert!(out.rendered.contains("nope")); + } +} diff --git a/src/command.rs b/src/command.rs index 5679a4b..d9e3047 100644 --- a/src/command.rs +++ b/src/command.rs @@ -127,6 +127,35 @@ impl CommandContext { &self.middleware.config } + /// Resolves the active [`Environment`](crate::environments::Environment) for + /// this invocation. + /// + /// The active environment name is `self.middleware.env`, seeded at startup + /// from the persisted active environment or configured default and + /// overridden per invocation by the global `--env` flag. Resolution merges + /// the compiled-in definition, the `environments.toml` file layer, and + /// `_*` environment-variable overrides. + /// + /// # Blocking + /// + /// When the `environments.toml` file layer is enabled, this performs + /// synchronous filesystem I/O via + /// [`Environments::resolve`](crate::environments::Environments::resolve). + /// Call it once per invocation and reuse the result rather than calling it + /// repeatedly inside an async handler on a latency-sensitive path. + /// + /// # Errors + /// + /// Returns an error if no environment system was registered via + /// [`CliConfig::with_environments`](crate::CliConfig::with_environments) or + /// if the active name does not resolve to a known environment. + pub fn environment(&self) -> Result { + let environments = self.middleware.environments.as_ref().ok_or_else(|| { + crate::error::CliCoreError::message("no environment system configured") + })?; + environments.resolve(&self.middleware.env) + } + /// Deserializes the raw argument matches into a typed args struct. /// /// Use this with `#[derive(clap::Args)]` structs to get type-safe access diff --git a/src/env_commands.rs b/src/env_commands.rs new file mode 100644 index 0000000..5df9151 --- /dev/null +++ b/src/env_commands.rs @@ -0,0 +1,105 @@ +//! Built-in `env` command group: list/get/set/info for environments. +//! +//! Mounted automatically when [`crate::CliConfig::with_environments`] is used. +//! The group exposes: +//! +//! - `env list` — list known environments with active flag. +//! - `env get` — print the active environment name. +//! - `env set ` — persist the active environment. +//! - `env info` — show the fully resolved active environment. + +use serde_json::json; + +use crate::{ + CommandResult, CommandSpec, GroupSpec, RuntimeCommandSpec, RuntimeGroupSpec, Tier, + error::CliCoreError, +}; + +/// Builds the built-in `env` command group. +#[must_use] +pub fn env_command_group() -> RuntimeGroupSpec { + RuntimeGroupSpec::new(GroupSpec::new("env", "Manage the active environment")) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("list", "List known environments") + .no_auth(true) + .with_system("env"), + async |ctx| { + let envs = ctx + .middleware + .environments + .as_ref() + .ok_or_else(|| CliCoreError::message("no environment system configured"))?; + let active = ctx.middleware.env.clone(); + let items: Vec<_> = envs + .list() + .into_iter() + .map(|name| { + let is_active = name == active; + json!({ "name": name, "active": is_active }) + }) + .collect(); + Ok(CommandResult::new(json!(items))) + }, + )) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("get", "Show the active environment name") + .no_auth(true) + .with_system("env"), + async |ctx| Ok(CommandResult::new(json!({ "active": ctx.middleware.env }))), + )) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("info", "Show the resolved active environment") + .no_auth(true) + .with_system("env"), + async |ctx| { + let env = ctx.environment()?; + let oauth = env.oauth.map(|o| { + json!({ + "client_id": o.client_id, + "auth_url": o.auth_url, + "token_url": o.token_url, + "scopes": o.scopes, + }) + }); + Ok(CommandResult::new(json!({ + "name": env.name, + "oauth": oauth, + "extra": env.extra, + }))) + }, + )) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("set", "Set and persist the active environment") + .no_auth(true) + .with_system("env") + .with_tier(Tier::Mutate) + .mutates(true) + .with_arg( + clap::Arg::new("name") + .value_name("NAME") + .required(true) + .help("Environment to make active (must be a known environment)"), + ), + async |ctx| { + let envs = ctx + .middleware + .environments + .as_ref() + .ok_or_else(|| CliCoreError::message("no environment system configured"))?; + let name = string_arg(&ctx.args, "name"); + if name.is_empty() { + return Err(CliCoreError::message("missing environment name")); + } + envs.persist_active(&name)?; + Ok(CommandResult::new(json!({ "active": name }))) + }, + )) +} + +/// Reads a required string argument, defaulting to empty when absent. +fn string_arg(args: &serde_json::Map, name: &str) -> String { + args.get(name) + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .to_owned() +} diff --git a/src/environments.rs b/src/environments.rs new file mode 100644 index 0000000..72b3084 --- /dev/null +++ b/src/environments.rs @@ -0,0 +1,614 @@ +//! First-class environment definitions and layered resolution. +//! +//! An [`Environments`] value holds compiled-in environment definitions and, +//! optionally, an `environments.toml` file plus `_*` env-var overrides. +//! Resolving a name merges those layers (later wins) into an [`Environment`]. + +use std::collections::BTreeMap; + +use serde::Deserialize; + +use crate::{Result, error::CliCoreError}; + +/// Standard OAuth slice of an environment, consumed by `PkceAuthProvider`. +/// +/// `auth_url`, `token_url`, and `scopes` may be empty when a layer set only +/// `client_id`. Consumers should treat empty endpoint strings as "fall back to +/// the provider's default base endpoints". +#[non_exhaustive] +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct OAuthConfig { + /// OAuth client id. + pub client_id: String, + /// Authorization endpoint. + pub auth_url: String, + /// Token endpoint. + pub token_url: String, + /// Default scopes. + pub scopes: Vec, +} + +/// A fully-resolved environment. +#[non_exhaustive] +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Environment { + /// Environment name (e.g. `prod`). + pub name: String, + /// OAuth configuration, present when the environment participates in OAuth. + pub oauth: Option, + /// App-specific fields (for example `api_url`). + pub extra: BTreeMap, +} + +/// An unresolved per-environment declaration (one layer of configuration). +/// +/// Fields are optional so layers can override individual values during +/// resolution. The same shape parses the `environments.toml` file. +#[derive(Clone, Debug, Default, Deserialize)] +pub struct EnvironmentDef { + #[serde(default)] + client_id: Option, + #[serde(default)] + auth_url: Option, + #[serde(default)] + token_url: Option, + #[serde(default)] + scopes: Option>, + /// Everything not recognised above is captured here (app-specific fields). + #[serde(flatten, default)] + extra: BTreeMap, +} + +impl EnvironmentDef { + /// Creates an empty declaration; every field falls back to lower layers. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Sets the OAuth client id. + #[must_use] + pub fn with_client_id(mut self, value: impl Into) -> Self { + self.client_id = Some(value.into()); + self + } + + /// Sets the authorization endpoint. + #[must_use] + pub fn with_auth_url(mut self, value: impl Into) -> Self { + self.auth_url = Some(value.into()); + self + } + + /// Sets the token endpoint. + #[must_use] + pub fn with_token_url(mut self, value: impl Into) -> Self { + self.token_url = Some(value.into()); + self + } + + /// Sets the default scopes. + #[must_use] + pub fn with_scopes(mut self, scopes: &[impl AsRef]) -> Self { + self.scopes = Some(scopes.iter().map(|s| s.as_ref().to_owned()).collect()); + self + } + + /// Sets an app-specific bag field. + #[must_use] + pub fn with_field(mut self, key: impl Into, value: impl Into) -> Self { + self.extra.insert(key.into(), value.into()); + self + } +} + +/// Engine-owned environment system: definitions + resolution + active-env state. +#[derive(Clone, Debug)] +pub struct Environments { + default: String, + defs: BTreeMap, + use_config_file: bool, + app_id: String, + file_path_override: Option, +} + +impl Environments { + /// Creates an environment system with the given default environment name. + #[must_use] + pub fn new(default_env: impl Into) -> Self { + Self { + default: default_env.into(), + defs: BTreeMap::new(), + use_config_file: false, + app_id: String::new(), + file_path_override: None, + } + } + + /// Registers (or replaces) a compiled-in environment definition. + #[must_use] + pub fn with_environment(mut self, name: impl Into, def: EnvironmentDef) -> Self { + self.defs.insert(name.into(), def); + self + } + + /// Enables loading `//environments.toml` during resolution. + #[must_use] + pub fn with_config_file(mut self, enabled: bool) -> Self { + self.use_config_file = enabled; + self + } + + /// Sets the application id used to locate the config file. + /// + /// The consumer must set this to the same `app_id` passed to + /// [`CliConfig::new`](crate::CliConfig::new) before sharing the + /// [`Environments`] with both + /// [`CliConfig::with_environments`](crate::CliConfig::with_environments) and + /// `PkceAuthProvider::with_environments` (with the `pkce-auth` feature), + /// or [`config_file_path`](Self::config_file_path) returns `None` and the + /// `environments.toml` file layer silently resolves empty. + #[must_use] + pub fn with_app_id(mut self, app_id: impl Into) -> Self { + self.app_id = app_id.into(); + self + } + + /// Test/advanced seam: force the environments file path. + #[must_use] + pub fn with_config_file_path_override(mut self, path: std::path::PathBuf) -> Self { + self.file_path_override = Some(path); + self.use_config_file = true; + self + } + + /// The default environment name. + #[must_use] + pub fn default_env(&self) -> &str { + &self.default + } + + /// Enumerable environment names (compiled-in + file-defined), sorted. + /// + /// Any error from reading or parsing the environments file (missing file, + /// permission/read error, or malformed TOML) is silently swallowed and only + /// the compiled-in names are returned. Use [`resolve`](Self::resolve) when + /// you need those errors surfaced; a fallible listing variant can be added + /// later if needed. + /// + /// # Blocking + /// + /// When the config-file layer is enabled, this performs synchronous + /// filesystem I/O to read and parse `environments.toml` (like + /// [`resolve`](Self::resolve)). Avoid calling it repeatedly on a + /// latency-sensitive async path. + #[must_use] + pub fn list(&self) -> Vec { + let mut names: std::collections::BTreeSet = self.defs.keys().cloned().collect(); + if let Ok(file) = self.file_defs() { + names.extend(file.into_keys()); + } + names.into_iter().collect() + } + + /// Resolves `name` by merging compiled defaults, the config file, + /// and `_*` env-var overrides (later wins) into an [`Environment`]. + /// + /// When only `client_id` was set on the matching layer(s), the returned + /// [`Environment`]'s `oauth.auth_url` / `oauth.token_url` will be empty + /// strings. Consumers should treat empty endpoint strings as "fall back to + /// the provider's default base endpoints". + /// + /// # Blocking + /// + /// When the config-file layer is enabled (via + /// [`with_config_file`](Self::with_config_file)), this performs synchronous + /// filesystem I/O to read and parse `environments.toml`. Resolve the + /// environment once at startup (or off the latency-sensitive path) rather + /// than calling it per request inside an async handler. + /// + /// # Errors + /// + /// Returns an error when `name` is not known to any layer or when the + /// environments file exists but cannot be read or parsed. + pub fn resolve(&self, name: &str) -> Result { + let compiled = self.defs.get(name); + // Parse the file once; reuse for both membership check and merge. + let mut all_file_defs = self.file_defs()?; + let file = all_file_defs.remove(name); + if compiled.is_none() && file.is_none() { + let mut known: std::collections::BTreeSet = self.defs.keys().cloned().collect(); + known.extend(all_file_defs.into_keys()); + let known_list: Vec = known.into_iter().collect(); + let known_display = if known_list.is_empty() { + "(none defined)".to_owned() + } else { + known_list.join(", ") + }; + return Err(CliCoreError::message(format!( + "unknown environment {name:?}; known: {known_display}" + ))); + } + let mut merged = EnvironmentDef::default(); + if let Some(def) = compiled { + merge_into(&mut merged, def); + } + if let Some(def) = &file { + merge_into(&mut merged, def); + } + apply_env_vars(name, &mut merged); + Ok(finalize(name, merged)) + } + + /// Path to `environments.toml` next to the engine config file, or `None` + /// when the file layer is disabled or the config dir cannot be determined. + #[must_use] + pub fn config_file_path(&self) -> Option { + if !self.use_config_file { + return None; + } + let config = crate::config::config_file_path(&self.app_id)?; + Some(config.with_file_name("environments.toml")) + } + + fn effective_file_path(&self) -> Option { + if let Some(path) = &self.file_path_override { + return Some(path.clone()); + } + self.config_file_path() + } + + /// Parses the environments file into a name -> def map. Missing file = empty. + fn file_defs(&self) -> Result> { + let Some(path) = self.effective_file_path() else { + return Ok(BTreeMap::new()); + }; + let text = match std::fs::read_to_string(&path) { + Ok(text) => text, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(BTreeMap::new()), + Err(err) => { + return Err(CliCoreError::message(format!( + "reading environments file {path:?}: {err}" + ))); + } + }; + toml_edit::de::from_str::>(&text).map_err(|err| { + CliCoreError::message(format!("parsing environments file {path:?}: {err}")) + }) + } + + /// Config-file key under which the sticky active environment is stored. + pub(crate) const ACTIVE_ENV_KEY: &'static str = "environment.active"; + + /// Reads the persisted active environment from a loaded config file. + #[must_use] + pub fn active_from_config(config: &crate::config::ConfigFile) -> Option { + config.get(Self::ACTIVE_ENV_KEY) + } + + /// Resolves the active environment name with precedence: + /// explicit `--env` override > persisted active > configured default. + #[must_use] + pub fn effective_active( + &self, + flag: Option<&str>, + config: &crate::config::ConfigFile, + ) -> String { + flag.map(ToOwned::to_owned) + .or_else(|| Self::active_from_config(config)) + .unwrap_or_else(|| self.default.clone()) + } + + /// Persists `name` as the active environment (loads, sets, saves a fresh + /// config file for `app_id`). Validates that `name` resolves first. + /// + /// # Errors + /// + /// Returns an error when `name` does not resolve to a known environment, or + /// when the config file cannot be written. + pub fn persist_active(&self, name: &str) -> Result<()> { + self.resolve(name)?; // reject unknown names + // Persisting writes the engine config file, which is keyed by app_id. + // Validate it up front so a missing/invalid app_id yields a clear, + // actionable error rather than a misleading "no config path" failure + // from ConfigFile::save() that points at XDG/HOME. + if crate::config::config_file_path(&self.app_id).is_none() { + return Err(CliCoreError::message(format!( + "cannot persist active environment {name:?}: the environment system has no usable app_id; \ + set one via Environments::with_app_id (matching the CliConfig app_id)" + ))); + } + let mut config = crate::config::ConfigFile::load(&self.app_id); + config.set(Self::ACTIVE_ENV_KEY, name)?; + config.save() + } +} + +/// Merges `src` into `dst`, with `src` winning on any field it sets. +fn merge_into(dst: &mut EnvironmentDef, src: &EnvironmentDef) { + if src.client_id.is_some() { + dst.client_id = src.client_id.clone(); + } + if src.auth_url.is_some() { + dst.auth_url = src.auth_url.clone(); + } + if src.token_url.is_some() { + dst.token_url = src.token_url.clone(); + } + if src.scopes.is_some() { + dst.scopes = src.scopes.clone(); + } + for (k, v) in &src.extra { + dst.extra.insert(k.clone(), v.clone()); + } +} + +/// Applies `_*` overrides: the three OAuth fields always, and any bag key +/// already present in the merged record (keyed `_`). +/// +/// The prefix is `name.to_uppercase().replace('-', "_")`, so environment names +/// that differ only by `-` vs `_` map to the same prefix and will collide. +/// +/// Scopes are intentionally not env-var overridable; set them via the +/// compiled-in layer or the `environments.toml` file. +fn apply_env_vars(name: &str, def: &mut EnvironmentDef) { + let prefix = name.to_uppercase().replace('-', "_"); + if let Ok(v) = std::env::var(format!("{prefix}_OAUTH_CLIENT_ID")) { + def.client_id = Some(v); + } + if let Ok(v) = std::env::var(format!("{prefix}_OAUTH_AUTH_URL")) { + def.auth_url = Some(v); + } + if let Ok(v) = std::env::var(format!("{prefix}_OAUTH_TOKEN_URL")) { + def.token_url = Some(v); + } + let keys: Vec = def.extra.keys().cloned().collect(); + for key in keys { + let var = format!("{prefix}_{}", key.to_uppercase().replace('-', "_")); + if let Ok(v) = std::env::var(&var) { + def.extra.insert(key, v); + } + } +} + +/// Turns a fully-merged declaration into a resolved [`Environment`]. OAuth is +/// present when a client id was set by any layer. +fn finalize(name: &str, def: EnvironmentDef) -> Environment { + let EnvironmentDef { + client_id, + auth_url, + token_url, + scopes, + extra, + } = def; + let oauth = client_id.map(|id| OAuthConfig { + client_id: id, + auth_url: auth_url.unwrap_or_default(), + token_url: token_url.unwrap_or_default(), + scopes: scopes.unwrap_or_default(), + }); + Environment { + name: name.to_owned(), + oauth, + extra, + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, unsafe_code)] +mod tests { + use super::*; + + use std::sync::Mutex; + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + /// RAII guard that removes an env var on drop, even if a test panics. + struct EnvGuard(&'static str); + impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: test holds ENV_LOCK; clean up on any exit including panic. + unsafe { std::env::remove_var(self.0) } + } + } + + fn sample() -> Environments { + Environments::new("prod") + .with_environment( + "prod", + EnvironmentDef::new() + .with_client_id("prod-client") + .with_auth_url("https://api.example.com/authorize") + .with_token_url("https://api.example.com/token") + .with_scopes(&["openid"]) + .with_field("api_url", "https://api.example.com"), + ) + .with_environment("dev", EnvironmentDef::new().with_client_id("dev-client")) + } + + #[test] + fn oauth_config_defaults_are_empty() { + let c = OAuthConfig::default(); + assert!(c.client_id.is_empty() && c.scopes.is_empty()); + } + + /// With no environments defined at all, the unknown-env error renders a + /// readable placeholder instead of a dangling `known: `. + #[test] + fn resolve_unknown_env_with_no_defs_uses_placeholder() { + let err = Environments::new("prod") + .resolve("prod") + .expect_err("nothing defined should fail"); + let message = err.to_string(); + assert!( + message.contains("(none defined)"), + "expected placeholder, got: {message}" + ); + } + + /// `persist_active` without an `app_id` returns a clear, actionable error + /// (mentioning `app_id`) rather than a misleading config-path failure. + #[test] + fn persist_active_without_app_id_errors_clearly() { + let err = sample() + .persist_active("prod") + .expect_err("persist without app_id should fail"); + let message = err.to_string(); + assert!( + message.contains("app_id"), + "error should mention app_id, got: {message}" + ); + } + + #[test] + fn builder_registers_compiled_environment() { + let envs = Environments::new("prod").with_environment( + "prod", + EnvironmentDef::new() + .with_client_id("prod-client") + .with_auth_url("https://api.example.com/authorize") + .with_token_url("https://api.example.com/token") + .with_scopes(&["openid"]) + .with_field("api_url", "https://api.example.com"), + ); + assert_eq!(envs.default_env(), "prod"); + assert_eq!(envs.list(), vec!["prod".to_owned()]); + } + + #[test] + fn resolve_returns_compiled_record() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let env = sample().resolve("prod").expect("prod resolves"); + let oauth = env.oauth.expect("oauth present"); + assert_eq!(oauth.client_id, "prod-client"); + assert_eq!(oauth.scopes, vec!["openid".to_owned()]); + assert_eq!( + env.extra.get("api_url").map(String::as_str), + Some("https://api.example.com") + ); + } + + #[test] + fn resolve_unknown_env_errors_with_known_names() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let err = sample().resolve("nope").unwrap_err().to_string(); + assert!(err.contains("nope")); + assert!(err.contains("prod") && err.contains("dev")); + } + + #[test] + fn resolve_with_only_client_id_yields_partial_oauth() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let envs = Environments::new("dev") + .with_environment("dev", EnvironmentDef::new().with_client_id("dev-only")); + let env = envs.resolve("dev").expect("dev resolves"); + let oauth = env.oauth.expect("oauth present when client_id is set"); + assert_eq!(oauth.client_id, "dev-only"); + assert!( + oauth.auth_url.is_empty(), + "auth_url should be empty (fall back to provider default)" + ); + assert!( + oauth.token_url.is_empty(), + "token_url should be empty (fall back to provider default)" + ); + assert!(oauth.scopes.is_empty()); + } + + #[test] + fn env_var_layer_overrides_oauth_and_known_bag_keys() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // SAFETY: serialized by ENV_LOCK; guards remove vars on any exit incl. panic. + unsafe { std::env::set_var("PROD_OAUTH_CLIENT_ID", "override-client") }; + let _g1 = EnvGuard("PROD_OAUTH_CLIENT_ID"); + unsafe { std::env::set_var("PROD_API_URL", "https://api.override.example.com") }; + let _g2 = EnvGuard("PROD_API_URL"); + + let env = sample().resolve("prod").expect("prod resolves"); + assert_eq!(env.oauth.unwrap().client_id, "override-client"); + assert_eq!( + env.extra.get("api_url").map(String::as_str), + Some("https://api.override.example.com") + ); + } + + #[test] + fn environments_file_path_sits_next_to_config() { + let envs = sample().with_app_id("gddy").with_config_file(true); + let path = envs.config_file_path().expect("path resolves with app id"); + assert!(path.ends_with("gddy/environments.toml"), "got {path:?}"); + } + + #[test] + fn file_layer_overrides_compiled_and_adds_custom_env() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let dir = tempfile::tempdir().expect("tempdir"); + let file = dir.path().join("environments.toml"); + std::fs::write( + &file, + r#" +[prod] +client_id = "file-client" + +[custom] +client_id = "custom-client" +api_url = "https://api.custom.example.com" +"#, + ) + .expect("write file"); + + let envs = sample() + .with_config_file(true) + .with_config_file_path_override(file); + + // File overrides the compiled prod client id, keeps compiled api_url. + let prod = envs.resolve("prod").expect("prod"); + assert_eq!(prod.oauth.unwrap().client_id, "file-client"); + assert_eq!( + prod.extra.get("api_url").map(String::as_str), + Some("https://api.example.com") + ); + + // Custom env exists only in the file. + let custom = envs.resolve("custom").expect("custom"); + assert_eq!(custom.oauth.unwrap().client_id, "custom-client"); + assert!(envs.list().contains(&"custom".to_owned())); + } + + const ACTIVE_KEY: &str = "environment.active"; + + #[test] + fn active_env_round_trips_through_config_file() { + use crate::config::ConfigFile; + let mut cfg = ConfigFile::default(); + assert_eq!(Environments::active_from_config(&cfg), None); + + cfg.set(ACTIVE_KEY, "ote").expect("set"); + assert_eq!( + Environments::active_from_config(&cfg).as_deref(), + Some("ote") + ); + } + + #[test] + fn effective_active_prefers_override_then_config_then_default() { + use crate::config::ConfigFile; + let envs = sample(); + let mut cfg = ConfigFile::default(); + cfg.set(ACTIVE_KEY, "dev").expect("set"); + + assert_eq!(envs.effective_active(Some("prod"), &cfg), "prod"); // explicit wins + assert_eq!(envs.effective_active(None, &cfg), "dev"); // config next + let empty = ConfigFile::default(); + assert_eq!(envs.effective_active(None, &empty), "prod"); // default last + } +} diff --git a/src/lib.rs b/src/lib.rs index bc40336..520437d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,6 +71,10 @@ pub mod command; pub mod config; /// Built-in `config` command group. pub mod config_commands; +/// Built-in `env` command group (private; only `cli.rs` consumes it). +mod env_commands; +/// First-class environment definitions and layered resolution. +pub mod environments; /// Shared error type and error traits. pub mod error; /// Global framework flags and flag-extraction helpers. @@ -120,6 +124,7 @@ pub use config::{ credential_store_env_var, resolve_credential_store, resolve_credential_store_with, }; pub use config_commands::config_command_group; +pub use environments::{Environment, EnvironmentDef, Environments, OAuthConfig}; pub use error::{ CliCoreError, DetailedError, ExitCoder, Result, exit_code_for_error, exit_code_for_exit_coder, }; diff --git a/src/middleware.rs b/src/middleware.rs index 98204ad..ec0c276 100644 --- a/src/middleware.rs +++ b/src/middleware.rs @@ -514,6 +514,13 @@ pub struct Middleware { /// module registration via /// [`ModuleContext::config`](crate::module::ModuleContext::config). pub config: Arc, + /// Optional first-class environment system. + /// + /// Set by [`CliConfig::with_environments`](crate::CliConfig::with_environments) + /// and cloned into each per-run middleware snapshot. Handlers resolve the + /// active environment through + /// [`CommandContext::environment`](crate::command::CommandContext::environment). + pub environments: Option>, } /// Rendered result produced by middleware. @@ -1054,3 +1061,22 @@ impl From for Value { Value::String(error.to_string()) } } + +#[cfg(test)] +mod env_wire_tests { + use super::*; + + #[test] + fn middleware_carries_optional_environments() { + use std::sync::Arc; + let mut mw = Middleware::new(); + assert!(mw.environments.is_none()); + mw.environments = Some(Arc::new(crate::environments::Environments::new("prod"))); + assert_eq!( + mw.environments + .as_ref() + .map(|envs| envs.default_env().to_owned()), + Some("prod".to_owned()) + ); + } +} diff --git a/src/transport/client.rs b/src/transport/client.rs index 8a2c42a..9fff60d 100644 --- a/src/transport/client.rs +++ b/src/transport/client.rs @@ -19,7 +19,13 @@ const BASE_BACKOFF: Duration = Duration::from_millis(500); const BUILTIN_DEFAULT_USER_AGENT: &str = "cli/dev"; static DEFAULT_USER_AGENT: OnceLock> = OnceLock::new(); -/// Sets the user-agent used by subsequently created [`HttpClient`] values. +/// Sets the process-wide default user-agent for outbound requests. +/// +/// Applies to subsequently created [`HttpClient`] values (those that do not set +/// their own via [`HttpClientBuilder::user_agent`]) and to the engine's other +/// outbound token traffic that reads this default — the PKCE provider's +/// token/refresh requests and the client-credentials injector. A per-client +/// user-agent still overrides it for that client. pub fn set_default_user_agent(user_agent: impl Into) { let lock = DEFAULT_USER_AGENT.get_or_init(|| RwLock::new(BUILTIN_DEFAULT_USER_AGENT.to_owned())); @@ -28,7 +34,12 @@ pub fn set_default_user_agent(user_agent: impl Into) { } } -fn default_user_agent() -> String { +/// Returns the process-wide default user-agent set via +/// [`set_default_user_agent`], or the builtin default when none was set. +/// +/// Used by [`HttpClientBuilder`] and by the engine's OAuth token requests so +/// that all outbound traffic carries the same user-agent. +pub(crate) fn default_user_agent() -> String { DEFAULT_USER_AGENT .get_or_init(|| RwLock::new(BUILTIN_DEFAULT_USER_AGENT.to_owned())) .read() @@ -38,6 +49,26 @@ fn default_user_agent() -> String { ) } +/// Serializes unit tests that mutate the process-wide default user-agent so +/// they cannot observe one another's writes. Integration tests in +/// `tests/foundation.rs` run in a separate binary and use their own lock. +#[cfg(test)] +pub(crate) static UA_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +/// Restores the process-wide default user-agent to the builtin on drop, so a +/// panicking assertion in a test that mutates it cannot leak the value into +/// later tests in this binary. Declare it after acquiring [`UA_TEST_LOCK`] so +/// the reset runs while the lock is still held. +#[cfg(test)] +pub(crate) struct RestoreDefaultUserAgent; + +#[cfg(test)] +impl Drop for RestoreDefaultUserAgent { + fn drop(&mut self) { + set_default_user_agent(BUILTIN_DEFAULT_USER_AGENT); + } +} + #[derive(serde::Deserialize)] struct GraphQlError { message: String, diff --git a/src/transport/injector.rs b/src/transport/injector.rs index 1ade0ea..46be9a7 100644 --- a/src/transport/injector.rs +++ b/src/transport/injector.rs @@ -259,6 +259,10 @@ impl ClientCredentialsInjector { reqwest::header::CONTENT_TYPE, "application/x-www-form-urlencoded", ) + .header( + reqwest::header::USER_AGENT, + crate::transport::client::default_user_agent(), + ) .form(&form) .send(), ) diff --git a/tests/foundation.rs b/tests/foundation.rs index 85a3649..15a0150 100644 --- a/tests/foundation.rs +++ b/tests/foundation.rs @@ -150,6 +150,18 @@ impl RecordingTransportLogger { static USER_AGENT_TEST_LOCK: Mutex<()> = Mutex::const_new(()); +/// Restores the process-wide default User-Agent to the builtin on drop, so a +/// panicking assertion in a test that publishes a config-derived UA cannot leak +/// it into later tests. Hold alongside (declared after) the `USER_AGENT_TEST_LOCK` +/// guard so the reset runs while the lock is still held. +struct RestoreDefaultUserAgent; + +impl Drop for RestoreDefaultUserAgent { + fn drop(&mut self) { + transport::set_default_user_agent("cli/dev"); + } +} + #[test] fn tier_string_forms_and_mutating_parity() { assert_eq!(Tier::Read.to_string(), "read"); @@ -442,6 +454,11 @@ async fn cli_runtime_root_help_includes_find_commands_without_modules() { #[tokio::test] async fn cli_execute_from_writes_success_to_stdout_and_errors_to_stderr() { + // execute_from publishes the config-derived User-Agent process-wide, so this + // test shares the lock with the user-agent tests and restores the default + // on exit (including panic) via the RAII guard, while the lock is held. + let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; let mut cli = Cli::new(CliConfig { name: "my-cli".to_owned(), short: "Developer tooling".to_owned(), @@ -487,6 +504,10 @@ async fn cli_execute_from_writes_success_to_stdout_and_errors_to_stderr() { #[tokio::test] async fn cli_execute_from_shutdown_signal_writes_interrupt_to_stderr() { + // execute_from_until_signal publishes the config-derived User-Agent + // process-wide; share the lock and restore the default (panic-safe) like above. + let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; let shutdown_count = Arc::new(AtomicUsize::new(0)); let shutdown_for_closure = Arc::clone(&shutdown_count); let mut cli = Cli::new(CliConfig { @@ -4936,12 +4957,19 @@ async fn provider_bearer_injector_empty_token_does_not_short_circuit_cache_prese #[tokio::test] async fn client_credentials_injector_requests_and_caches_bearer_token() { + // The injector tags its token request with the process default User-Agent; + // hold the user-agent lock and pin the default so the assertion is stable, + // restoring it on unwind via the RAII guard while the lock is still held. + let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; + transport::set_default_user_agent("cli/dev"); let token_requests = Arc::new(AtomicUsize::new(0)); let token_requests_for_server = Arc::clone(&token_requests); let server = TestServer::sequence(vec![Box::new(move |request| { token_requests_for_server.fetch_add(1, Ordering::SeqCst); assert!(request.contains("POST /token HTTP/1.1")); assert!(request.contains("content-type: application/x-www-form-urlencoded")); + assert!(request.contains("user-agent: cli/dev")); assert!(request.contains("grant_type=client_credentials")); assert!(request.contains("client_id=client")); assert!(request.contains("client_secret=secret")); @@ -5074,6 +5102,8 @@ async fn client_credentials_injector_missing_token_and_negative_expiry_match_leg #[tokio::test] async fn http_client_get_sets_headers_and_decodes_json() { let _guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; + transport::set_default_user_agent("cli/dev"); let server = TestServer::new(|request| { assert!(request.contains("GET /thing HTTP/1.1")); assert!(request.contains("user-agent: cli/dev")); @@ -5179,6 +5209,7 @@ async fn http_client_null_json_returns_default_result_preserves_legacy_zero_valu #[tokio::test] async fn http_client_set_default_user_agent_affects_new_clients_only() { let _guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; transport::set_default_user_agent("cli/custom"); let custom_server = TestServer::new(|request| { assert!(request.contains("GET /thing HTTP/1.1")); @@ -5215,8 +5246,6 @@ async fn http_client_set_default_user_agent_affects_new_clients_only() { .await .expect("explicit user agent should override default"); assert_eq!(value, json!({"ok": true})); - - transport::set_default_user_agent("cli/dev"); } #[tokio::test] @@ -5466,6 +5495,8 @@ async fn http_client_default_headers_can_override_json_content_type_preserves_le #[tokio::test] async fn http_client_do_raw_sends_method_content_type_body_and_decodes_json() { let _guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; + transport::set_default_user_agent("cli/dev"); let server = TestServer::new(|request| { assert!(request.contains("OPTIONS /raw HTTP/1.1")); assert!(request.contains("content-type: application/x-www-form-urlencoded")); @@ -5504,6 +5535,9 @@ async fn http_client_do_raw_sends_method_content_type_body_and_decodes_json() { #[tokio::test] async fn http_client_do_raw_optional_none_body_matches_legacy_nil_reader() { + let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; + transport::set_default_user_agent("cli/dev"); let server = TestServer::new(|request| { assert!(request.contains("OPTIONS /raw HTTP/1.1")); assert!(request.contains("user-agent: cli/dev")); @@ -5646,6 +5680,9 @@ async fn http_client_etag_if_match_and_multipart_without_response_skip_success_d #[tokio::test] async fn http_client_post_raw_none_body_omits_json_content_type_preserves_legacy() { + let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; + let _restore_ua = RestoreDefaultUserAgent; + transport::set_default_user_agent("cli/dev"); let server = TestServer::new(|request| { assert!(request.contains("POST /raw HTTP/1.1")); assert!(request.contains("user-agent: cli/dev")); @@ -9885,6 +9922,56 @@ async fn auth_registered_after_construction_is_categorized() { ); } +#[tokio::test] +async fn env_group_lists_gets_and_shows_info_for_active_environment() { + use cli_engine::environments::{EnvironmentDef, Environments}; + + let cli = Cli::new( + CliConfig::new("envcmds", "Env cmds", "envcmds").with_environments(Arc::new( + Environments::new("prod") + .with_environment( + "prod", + EnvironmentDef::new().with_field("api_url", "https://p"), + ) + .with_environment( + "ote", + EnvironmentDef::new().with_field("api_url", "https://o"), + ), + )), + ); + + // env list returns both environments. + let list = cli + .run(["envcmds", "env", "list", "--output", "json"]) + .await; + assert_eq!(list.exit_code, 0, "env list failed: {}", list.rendered); + assert!( + list.rendered.contains("prod") && list.rendered.contains("ote"), + "env list missing environments: {}", + list.rendered + ); + + // env get returns the default active environment. + let get = cli.run(["envcmds", "env", "get", "--output", "json"]).await; + assert_eq!(get.exit_code, 0, "env get failed: {}", get.rendered); + assert!( + get.rendered.contains("prod"), + "env get missing default env: {}", + get.rendered + ); + + // env info --env ote shows ote's extra fields. + let info = cli + .run(["envcmds", "env", "info", "--env", "ote", "--output", "json"]) + .await; + assert_eq!(info.exit_code, 0, "env info failed: {}", info.rendered); + assert!( + info.rendered.contains("https://o"), + "env info missing ote api_url: {}", + info.rendered + ); +} + #[derive(Debug)] struct RecordingEnvProvider { envs: Arc>>,