diff --git a/Cargo.lock b/Cargo.lock index c0a3a08..e508114 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -460,6 +460,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -1112,6 +1122,7 @@ dependencies = [ "clap", "csv", "dirs", + "fs2", "futures", "globset", "httpdate", @@ -2159,6 +2170,22 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + [[package]] name = "winapi-util" version = "0.1.11" @@ -2168,6 +2195,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-link" version = "0.2.1" diff --git a/Cargo.toml b/Cargo.toml index 1330d48..60e93b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ aho-corasick = "1.1.4" anyhow = "1.0.102" clap = { version = "4.6.1", features = ["derive"] } dirs = "6.0.0" +fs2 = "0.4.3" futures = "0.3.32" globset = "0.4.18" httpdate = "1.0.3" diff --git a/README.md b/README.md index 1bdf14d..16c82e2 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Release binaries cover Linux x86_64 and ARM64 with glibc or musl, plus macOS on Authenticate once for hosted inference against your organization's entitlement, or bring your own model key: ```sh -postil login # zero-config: stores a credential for hosted inference +postil login # zero-config: stores a renewable login for hosted inference # or: export MODEL_API_KEY=... # OpenRouter is the default endpoint # export REVIEW_MODEL=provider/qualified-model postil doctor # validate the endpoint and repository diff --git a/docs/configuration.md b/docs/configuration.md index 2fa8594..f2f54e7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -47,7 +47,7 @@ Ignore patterns remove matching paths before grounding, batching, and large-revi | Variable | Purpose | | --- | --- | | `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, `LLM_API_KEY` | Model provider credential, checked in that order | -| `POSTIL_LOGIN_SERVER` | Postil web app used by `postil login`/`postil logout`; defaults to `https://postil.dev` | +| `POSTIL_LOGIN_SERVER` | Postil web app used by `postil login`, refresh, and `postil logout`; defaults to `https://postil.dev` | | `POSTIL_API_BASE` | Model endpoint selected by the operator | | `POSTIL_API_FORMAT` | `openai-compatible` or `anthropic` | | `POSTIL_ENDPOINT_AUTH_HEADER`, `POSTIL_ENDPOINT_AUTH_VALUE` | Additional private-gateway authentication | @@ -74,7 +74,7 @@ postil login postil logout ``` -`postil login` authenticates against postil.dev over a device-authorization flow (open the printed URL, enter the code) and stores a credential at `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, mode `0600` in a `0700` directory. That credential is a fallback: it is used only when none of `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or `LLM_API_KEY` is set. When it is used, its `apiBase` and model select the request unless `POSTIL_API_BASE`/`REVIEW_MODEL` are set, which still win. `postil logout` revokes the credential server-side and removes the local file even if that call fails. An expired credential produces one instruction to run `postil login` again rather than a provider authentication error. +`postil login` authenticates against postil.dev over a device-authorization flow (open the printed URL, enter the code) and stores a renewable credential at `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, mode `0600` in a `0700` directory. That credential is a fallback: it is used only when none of `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or `LLM_API_KEY` is set. When it is used, Postil rotates the access credential before it expires and persists the replacement; explicit API keys never trigger a refresh. Its `apiBase` and model select the request unless `POSTIL_API_BASE`/`REVIEW_MODEL` are set, which still win. `postil logout` revokes the stored refresh credential server-side and removes the local file even if that call fails. A legacy access-only login, an expired refresh inactivity window, or a rejected refresh produces one instruction to run `postil login` again rather than a provider authentication error. Temporary refresh failures retain the stored credential and ask the user to try again. ## Inspect and initialize @@ -84,4 +84,4 @@ postil config postil doctor ``` -`postil config` prints the resolved non-secret configuration and its sources. `postil doctor` validates endpoint reachability, credential acceptance, and repository setup without printing credential values, and reports whether a login credential is present and when it expires. +`postil config` prints the resolved non-secret configuration and its sources. `postil doctor` validates endpoint reachability, credential acceptance, and repository setup without printing credential values. Both commands identify renewable logins, access expiry, refresh inactivity expiry, and legacy access-only logins. diff --git a/src/api_key.rs b/src/api_key.rs index 060f2f8..84dbf06 100644 --- a/src/api_key.rs +++ b/src/api_key.rs @@ -1,16 +1,15 @@ //! Inference API-key resolution shared by CLI runtime checks. //! //! [`resolve_from_process_env`] resolves only the four explicit env vars, in -//! priority order. [`resolve_effective`] adds one more source below all -//! four: a stored `postil login` credential, used only when none of the env -//! vars is set. See `config.rs`'s module doc for the full precedence -//! statement. +//! priority order. Stored-login resolution is invoked only when none of the +//! four is set. See `config.rs`'s module doc for the full precedence statement. -use std::path::Path; +use std::future::Future; use anyhow::Result; -use crate::credentials::{self, Credentials}; +#[cfg(test)] +use crate::credentials::Credentials; pub(crate) const API_KEY_ENV_VARS: [&str; 4] = [ "POSTIL_API_KEY", @@ -33,20 +32,7 @@ pub(crate) fn resolve_with(mut lookup: impl FnMut(&str) -> Option) -> Op .find_map(|name| lookup(name).filter(|value| !value.trim().is_empty())) } -/// Bearer key resolved for inference, falling back to a stored `postil -/// login` credential when none of the four explicit env vars is set. -/// `Ok(None)` means neither source has a key; callers turn that into their -/// own "set a key or run `postil login`" message. An expired stored -/// credential is reported here as an error, so the caller surfaces one -/// actionable instruction instead of a confusing upstream auth failure once -/// the request reaches the provider. -pub(crate) fn resolve_effective(credentials_path: &Path) -> Result> { - resolve_effective_with( - |name| std::env::var(name).ok(), - || credentials::read(credentials_path), - ) -} - +#[cfg(test)] pub(crate) fn resolve_effective_with( env_lookup: impl FnMut(&str) -> Option, credential_lookup: impl FnOnce() -> Result>, @@ -64,10 +50,29 @@ pub(crate) fn resolve_effective_with( Ok(Some(credentials.token)) } +/// Resolves an explicit key without polling the stored-login future. This +/// keeps BYOK invocations independent of both the credential file and the +/// Postil authentication service. +pub(crate) async fn resolve_explicit_or_stored( + explicit: Option, + stored: F, +) -> Result> +where + F: Future>>, +{ + match explicit { + Some(key) => Ok(Some(key)), + None => stored.await, + } +} + #[cfg(test)] mod tests { use super::*; + use crate::credentials; use std::collections::HashMap; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; fn resolve(pairs: &[(&str, &str)]) -> Option { let values: HashMap<&str, &str> = pairs.iter().copied().collect(); @@ -112,6 +117,8 @@ mod tests { version: credentials::CREDENTIALS_VERSION, token: "pcli_stored-token-not-a-real-secret".to_string(), expires_at: expires_at.to_string(), + refresh_token: None, + refresh_expires_at: None, api_base: "https://postil.dev/api/inference/v1".to_string(), org: "runatlas-is".to_string(), model: "z-ai/glm-5.2".to_string(), @@ -168,4 +175,18 @@ mod tests { assert!(message.contains("postil login")); assert!(!message.contains("pcli_stored-token-not-a-real-secret")); } + + #[tokio::test] + async fn explicit_key_bypasses_stored_login_refresh() { + let refreshed = Arc::new(AtomicBool::new(false)); + let invoked = refreshed.clone(); + let resolved = resolve_explicit_or_stored(Some("explicit-key".to_string()), async move { + invoked.store(true, Ordering::SeqCst); + Ok(Some("stored-key".to_string())) + }) + .await + .unwrap(); + assert_eq!(resolved.as_deref(), Some("explicit-key")); + assert!(!refreshed.load(Ordering::SeqCst)); + } } diff --git a/src/attribution.rs b/src/attribution.rs index efc11ec..28f9692 100644 --- a/src/attribution.rs +++ b/src/attribution.rs @@ -243,7 +243,7 @@ async fn run_inner(input_path: &Path, config_path: Option<&Path>) -> Result cfg.scorer == input.model, "atomic attribution model must equal the configured primary scorer" ); - let client = LlmClient::from_env(&cfg)?; + let client = LlmClient::from_env(&cfg).await?; let system = system_prompt(); let user = user_prompt(&input)?; let review = client diff --git a/src/config.rs b/src/config.rs index e6604ba..7cbe312 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,13 +12,13 @@ //! `MODEL_API_KEY`, and `LLM_API_KEY` (first set wins) all take priority over //! a stored `postil login` credential. When none of those four is set and a //! valid, unexpired credential exists at -//! `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, its token becomes -//! the bearer key (see `api_key::resolve_effective`) and its `apiBase`/ +//! `${XDG_CONFIG_HOME:-~/.config}/postil/credentials.json`, +//! `login::resolve_stored_token` supplies its bearer key and its `apiBase`/ //! `model` replace the values resolved above -- unless `POSTIL_API_BASE` / //! `REVIEW_MODEL` are set, which still win, applied first in this same //! function. An expired or unreadable stored credential is left alone here; //! `resolve_api_key` reports it as one actionable "run `postil login` again" -//! error instead of blocking commands, like `postil config`, that never need +//! error, while `postil config` reports the stored-login state without needing //! a live key. use std::path::{Path, PathBuf}; diff --git a/src/credentials.rs b/src/credentials.rs index ac26515..a8d7414 100644 --- a/src/credentials.rs +++ b/src/credentials.rs @@ -9,16 +9,23 @@ //! `chmod`'d afterward, so there is no window where the token is briefly //! world-readable. -use std::fs::{self, OpenOptions}; +use std::fs::{self, File, OpenOptions}; use std::io::Write; #[cfg(unix)] use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; +use fs2::FileExt; use serde::{Deserialize, Serialize}; -pub const CREDENTIALS_VERSION: u32 = 1; +pub const CREDENTIALS_VERSION: u32 = 2; +pub const LEGACY_CREDENTIALS_VERSION: u32 = 1; +// The refresh exchange has separately bounded send and body-read phases. A +// second local process waits long enough for both before asking the caller to +// retry, so concurrent agent runs converge on one token rotation. +const CREDENTIAL_LOCK_WAIT: std::time::Duration = std::time::Duration::from_secs(45); +const CREDENTIAL_LOCK_RETRY: std::time::Duration = std::time::Duration::from_millis(25); #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Credentials { @@ -26,6 +33,18 @@ pub struct Credentials { pub token: String, #[serde(rename = "expiresAt")] pub expires_at: String, + #[serde( + default, + rename = "refreshToken", + skip_serializing_if = "Option::is_none" + )] + pub refresh_token: Option, + #[serde( + default, + rename = "refreshExpiresAt", + skip_serializing_if = "Option::is_none" + )] + pub refresh_expires_at: Option, #[serde(rename = "apiBase")] pub api_base: String, pub org: String, @@ -37,14 +56,36 @@ impl Credentials { /// already expired rather than trusted, so a corrupted file never grants /// silent access. pub fn is_expired(&self) -> bool { - match time::OffsetDateTime::parse( - &self.expires_at, - &time::format_description::well_known::Rfc3339, - ) { - Ok(expires_at) => expires_at <= time::OffsetDateTime::now_utc(), + self.expires_within(std::time::Duration::ZERO) + } + + pub fn expires_within(&self, margin: std::time::Duration) -> bool { + let Ok(margin) = time::Duration::try_from(margin) else { + return true; + }; + match parse_timestamp(&self.expires_at) { + Ok(expires_at) => expires_at <= time::OffsetDateTime::now_utc() + margin, Err(_) => true, } } + + pub fn refresh_is_expired(&self) -> bool { + match self.refresh_expires_at.as_deref().map(parse_timestamp) { + Some(Ok(expires_at)) => expires_at <= time::OffsetDateTime::now_utc(), + Some(Err(_)) | None => true, + } + } + + pub fn can_refresh(&self) -> bool { + self.refresh_token + .as_deref() + .is_some_and(|token| !token.trim().is_empty()) + && !self.refresh_is_expired() + } +} + +fn parse_timestamp(value: &str) -> Result { + time::OffsetDateTime::parse(value, &time::format_description::well_known::Rfc3339) } /// The real, XDG-resolved path used by `postil login`/`postil logout` and by @@ -74,6 +115,14 @@ pub fn read(path: &Path) -> Result> { }; let credentials: Credentials = serde_json::from_str(&raw).with_context(|| format!("parsing {}", path.display()))?; + anyhow::ensure!( + matches!( + credentials.version, + LEGACY_CREDENTIALS_VERSION | CREDENTIALS_VERSION + ), + "unsupported credentials version {}; run `postil login` again", + credentials.version + ); Ok(Some(credentials)) } @@ -95,6 +144,14 @@ pub fn write(path: &Path, credentials: &Credentials) -> Result<()> { let mut file = options .open(&temp_path) .context("creating private credentials file")?; + anyhow::ensure!( + matches!( + credentials.version, + LEGACY_CREDENTIALS_VERSION | CREDENTIALS_VERSION + ), + "unsupported credentials version {}", + credentials.version + ); serde_json::to_writer_pretty(&mut file, credentials).context("serializing credentials")?; file.write_all(b"\n").context("writing credentials")?; file.sync_all().context("syncing credentials")?; @@ -107,6 +164,61 @@ pub fn write(path: &Path, credentials: &Credentials) -> Result<()> { result } +/// A kernel-backed, cross-process lock shared by every stored-login mutation. +/// Keeping the lock file in place avoids stale-file recovery: the operating +/// system releases the exclusive lock when the handle closes or its process +/// exits. +pub struct CredentialLock { + _file: File, +} + +impl CredentialLock { + pub async fn acquire(credentials_path: &Path) -> Result { + let parent = credentials_path + .parent() + .context("credentials path must have a parent directory")?; + create_private_dir(parent)?; + let file_name = credentials_path + .file_name() + .and_then(|name| name.to_str()) + .context("credentials path must have a file name")?; + let path = parent.join(format!(".{file_name}.refresh.lock")); + let file = open_private_lock_file(&path)?; + let deadline = tokio::time::Instant::now() + CREDENTIAL_LOCK_WAIT; + + loop { + match file.try_lock_exclusive() { + Ok(()) => return Ok(Self { _file: file }), + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + if tokio::time::Instant::now() >= deadline { + anyhow::bail!( + "another postil process is updating the stored login; try again" + ); + } + tokio::time::sleep(CREDENTIAL_LOCK_RETRY).await; + } + Err(error) => return Err(error).context("locking stored login"), + } + } + } +} + +fn open_private_lock_file(path: &Path) -> Result { + let mut options = OpenOptions::new(); + options.read(true).write(true).create(true); + #[cfg(unix)] + options.mode(0o600); + let file = options.open(path).context("creating stored login lock")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let permissions = fs::Permissions::from_mode(0o600); + file.set_permissions(permissions) + .context("securing stored login lock")?; + } + Ok(file) +} + /// Idempotent: removing an already-absent file is success, matching the /// server-side logout endpoint's own idempotence. pub fn remove(path: &Path) -> Result<()> { @@ -140,6 +252,8 @@ mod tests { version: CREDENTIALS_VERSION, token: "pcli_test-token-not-a-real-secret".to_string(), expires_at: expires_at.to_string(), + refresh_token: Some("fixture-refresh-not-a-credential".to_string()), + refresh_expires_at: Some("2999-01-01T00:00:00.000Z".to_string()), api_base: "https://postil.dev/api/inference/v1".to_string(), org: "runatlas-is".to_string(), model: "z-ai/glm-5.2".to_string(), @@ -223,4 +337,69 @@ mod tests { fn unparsable_expiry_fails_closed_as_expired() { assert!(sample("not-a-timestamp").is_expired()); } + + #[test] + fn reads_a_v1_access_only_credential() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + fs::write( + &path, + r#"{"version":1,"token":"pcli_test-token-not-a-real-secret","expiresAt":"2999-01-01T00:00:00.000Z","apiBase":"https://postil.dev/api/inference/v1","org":"runatlas-is","model":"z-ai/glm-5.2"}"#, + ) + .unwrap(); + let credentials = read(&path).unwrap().unwrap(); + assert_eq!(credentials.version, 1); + assert!(credentials.refresh_token.is_none()); + assert!(credentials.refresh_expires_at.is_none()); + } + + #[test] + fn preserves_v1_for_an_access_only_credential() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("postil").join("credentials.json"); + let mut credentials = sample("2999-01-01T00:00:00.000Z"); + credentials.version = 1; + credentials.refresh_token = None; + credentials.refresh_expires_at = None; + write(&path, &credentials).unwrap(); + let raw = fs::read_to_string(path).unwrap(); + assert!(raw.contains("\"version\": 1")); + assert!(!raw.contains("refreshToken")); + } + + #[test] + fn rejects_unknown_future_credential_versions() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + let mut credentials = sample("2999-01-01T00:00:00.000Z"); + credentials.version = CREDENTIALS_VERSION + 1; + fs::write(&path, serde_json::to_string(&credentials).unwrap()).unwrap(); + let error = read(&path).expect_err("future credential versions must fail closed"); + assert!( + error + .to_string() + .contains("unsupported credentials version") + ); + } + + #[tokio::test] + #[cfg(unix)] + async fn stored_login_lock_file_has_owner_only_permission_bits() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + write(&credentials_path, &sample("2999-01-01T00:00:00.000Z")).unwrap(); + let lock = CredentialLock::acquire(&credentials_path).await.unwrap(); + drop(lock); + let lock_path = credentials_path + .parent() + .unwrap() + .join(".credentials.json.refresh.lock"); + let mode = fs::metadata(lock_path).unwrap().permissions().mode() & 0o777; + assert_eq!( + mode, 0o600, + "stored login lock file must be mode 0600, got {mode:o}" + ); + } } diff --git a/src/doctor.rs b/src/doctor.rs index cdd1fc0..915a74b 100644 --- a/src/doctor.rs +++ b/src/doctor.rs @@ -9,6 +9,7 @@ use crate::api_key; use crate::config::Config; use crate::credentials; use crate::llm::LlmClient; +use crate::login; use anyhow::Result; pub struct Check { @@ -49,28 +50,22 @@ pub async fn run(cfg: &Config) -> Result> { }); let credentials_path = credentials::default_path(); - let stored_login = credentials_path - .as_ref() - .ok() - .and_then(|path| credentials::read(path).ok().flatten()); - checks.push(Check { - name: "login", - ok: stored_login.as_ref().is_none_or(|c| !c.is_expired()), - detail: match &stored_login { - None => "not logged in; `postil login` gives zero-config hosted inference".to_string(), - Some(c) if c.is_expired() => format!( - "credential for org {} expired at {}; run `postil login` again", - c.org, c.expires_at - ), - Some(c) => format!("logged in to org {} (expires {})", c.org, c.expires_at), - }, + checks.push(match credentials_path.as_ref() { + Ok(path) => login_status_at(path), + Err(error) => { + login_status_error(format!("cannot inspect the stored login path: {error:#}")) + } }); let key_names = api_key::names_text(); - let key_lookup = credentials_path - .as_ref() - .map_err(|e| anyhow::anyhow!("{e:#}")) - .and_then(|path| api_key::resolve_effective(path)); + let key_lookup = if let Some(key) = api_key::resolve_from_process_env() { + Ok(Some(key)) + } else { + match credentials_path.as_ref() { + Ok(path) => login::resolve_stored_token(path).await, + Err(error) => Err(anyhow::anyhow!("{error:#}")), + } + }; let (key_ok, key_detail, key) = match key_lookup { Ok(Some(k)) => ( true, @@ -135,6 +130,82 @@ pub async fn run(cfg: &Config) -> Result> { Ok(checks) } +/// The non-secret login state shown by both `postil doctor` and `postil +/// config`. It deliberately reads without refreshing so configuration +/// inspection never rotates a credential. +pub fn login_status() -> Check { + match credentials::default_path() { + Ok(path) => login_status_at(&path), + Err(error) => { + login_status_error(format!("cannot inspect the stored login path: {error:#}")) + } + } +} + +fn login_status_error(detail: String) -> Check { + Check { + name: "login", + ok: false, + detail, + } +} + +fn login_status_at(credentials_path: &std::path::Path) -> Check { + let stored_login = match credentials::read(credentials_path) { + Ok(stored_login) => stored_login, + Err(error) => { + return login_status_error(format!( + "stored login is unreadable; run `postil login` again ({error:#})" + )); + } + }; + match stored_login { + None => Check { + name: "login", + ok: true, + detail: "not logged in; `postil login` gives zero-config hosted inference".to_string(), + }, + Some(c) if c.refresh_token.is_none() && c.refresh_expires_at.is_none() => Check { + name: "login", + ok: !c.is_expired(), + detail: if c.is_expired() { + format!( + "legacy access-only login for org {} expired at {}; run `postil login` again", + c.org, c.expires_at + ) + } else { + format!( + "legacy access-only login for org {} (access expires {})", + c.org, c.expires_at + ) + }, + }, + Some(c) if !c.can_refresh() => Check { + name: "login", + ok: false, + detail: format!( + "renewable login for org {} has refresh inactivity expiry {}; run `postil login` again", + c.org, + c.refresh_expires_at + .as_deref() + .unwrap_or("that is missing or invalid") + ), + }, + Some(c) => Check { + name: "login", + ok: true, + detail: format!( + "renewable login for org {} (access expires {}; refresh inactivity expires {})", + c.org, + c.expires_at, + c.refresh_expires_at + .as_deref() + .expect("renewable login has an expiry") + ), + }, + } +} + pub fn print_report(checks: &[Check]) -> bool { let mut all_ok = true; for c in checks { @@ -149,3 +220,74 @@ pub fn print_report(checks: &[Check]) -> bool { } all_ok } + +#[cfg(test)] +mod tests { + use super::*; + use crate::credentials::{self, Credentials}; + + fn stored_login() -> Credentials { + Credentials { + version: credentials::CREDENTIALS_VERSION, + token: "pcli_test-access-not-a-real-secret".to_string(), + expires_at: "2999-01-01T00:00:00.000Z".to_string(), + refresh_token: Some("fixture-refresh-not-a-credential".to_string()), + refresh_expires_at: Some("2999-12-01T00:00:00.000Z".to_string()), + api_base: "https://postil.dev/api/inference/v1".to_string(), + org: "runatlas-is".to_string(), + model: "z-ai/glm-5.2".to_string(), + } + } + + #[test] + fn status_distinguishes_a_renewable_login_and_its_expiries() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + credentials::write(&path, &stored_login()).unwrap(); + let check = login_status_at(&path); + assert!(check.ok); + assert!(check.detail.contains("renewable login")); + assert!(check.detail.contains("access expires")); + assert!(check.detail.contains("refresh inactivity expires")); + } + + #[test] + fn status_distinguishes_a_legacy_access_only_login() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + let mut legacy = stored_login(); + legacy.version = 1; + legacy.refresh_token = None; + legacy.refresh_expires_at = None; + credentials::write(&path, &legacy).unwrap(); + let check = login_status_at(&path); + assert!(check.ok); + assert!(check.detail.contains("legacy access-only")); + } + + #[test] + fn status_reports_refresh_inactivity_expiry() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + let mut expired = stored_login(); + expired.refresh_expires_at = Some("2020-01-01T00:00:00.000Z".to_string()); + credentials::write(&path, &expired).unwrap(); + let check = login_status_at(&path); + assert!(!check.ok); + assert!(check.detail.contains("refresh inactivity expiry")); + } + + #[test] + fn unreadable_login_is_unhealthy_without_exposing_file_contents() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.json"); + std::fs::write(&path, "private credential contents that are not JSON").unwrap(); + + let check = login_status_at(&path); + + assert!(!check.ok); + assert!(check.detail.contains("stored login is unreadable")); + assert!(check.detail.contains("postil login")); + assert!(!check.detail.contains("private credential contents")); + } +} diff --git a/src/llm.rs b/src/llm.rs index 19ff91e..c3ce009 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -2602,8 +2602,8 @@ impl LlmClient { /// Local and interactive clients have no built-in total deadline. They only /// get one when POSTIL_LLM_TOTAL_TIMEOUT_SECS is explicitly set. - pub fn from_env(cfg: &Config) -> Result { - let api_key = resolve_api_key()?; + pub async fn from_env(cfg: &Config) -> Result { + let api_key = resolve_api_key().await?; let timeouts = LlmTimeouts::from_env(DEFAULT_REQUEST_TIMEOUT_SECS, None)?; let total_deadline = timeouts.total.map(|duration| Instant::now() + duration); Self::build( @@ -2615,14 +2615,14 @@ impl LlmClient { ) } - pub(crate) fn from_env_for_remote_review( + pub(crate) async fn from_env_for_remote_review( cfg: &Config, total_budget_started_at: Instant, default_request_timeout: Duration, default_review_timeout: Duration, default_total_timeout: Duration, ) -> Result { - let api_key = resolve_api_key()?; + let api_key = resolve_api_key().await?; let timeouts = LlmTimeouts::from_env( default_request_timeout.as_secs(), Some(default_total_timeout.as_secs()), @@ -5650,16 +5650,17 @@ impl LlmTimeouts { } } -fn resolve_api_key() -> Result { +async fn resolve_api_key() -> Result { // Checked first, and independently of the credentials-path lookup below, // so an explicit key still works in a minimal environment with no HOME // or XDG_CONFIG_HOME -- exactly what worked before login existed. - if let Some(key) = api_key::resolve_from_process_env() { - return Ok(key); - } - let credentials_path = crate::credentials::default_path() - .context("resolving the postil login credentials path")?; - match api_key::resolve_effective(&credentials_path)? { + let explicit_key = api_key::resolve_from_process_env(); + let stored_login = async { + let credentials_path = crate::credentials::default_path() + .context("resolving the postil login credentials path")?; + crate::login::resolve_stored_token(&credentials_path).await + }; + match api_key::resolve_explicit_or_stored(explicit_key, stored_login).await? { Some(key) => Ok(key), None => { let key_names = api_key::names_text(); @@ -6772,13 +6773,13 @@ mod tests { EnvRestore::remove(TOTAL_TIMEOUT_ENV); EnvRestore::set("POSTIL_API_KEY", "test-key"); - let client = LlmClient::from_env_for_remote_review( + let client = futures::executor::block_on(LlmClient::from_env_for_remote_review( &Config::default(), Instant::now(), Duration::from_secs(crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS), Duration::from_secs(crate::review::HOSTED_LLM_REVIEW_TIMEOUT_SECS), Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS), - ) + )) .unwrap(); assert_eq!( client.review_model_timeout, @@ -6816,13 +6817,13 @@ mod tests { let wave_slot = Duration::from_secs(crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS); let build = || { - LlmClient::from_env_for_remote_review( + futures::executor::block_on(LlmClient::from_env_for_remote_review( &config, Instant::now(), Duration::from_secs(crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS), Duration::from_secs(review_budget), Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS), - ) + )) .unwrap() }; @@ -8743,13 +8744,13 @@ mod tests { let elapsed = Duration::from_secs(10); let started_at = Instant::now() - elapsed; - let client = LlmClient::from_env_for_remote_review( + let client = futures::executor::block_on(LlmClient::from_env_for_remote_review( &Config::default(), started_at, Duration::from_secs(crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS), Duration::from_secs(crate::review::HOSTED_LLM_REVIEW_TIMEOUT_SECS), Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS), - ) + )) .unwrap(); let remaining = client.remaining_budget(LlmPhase::Total).unwrap().unwrap(); diff --git a/src/login.rs b/src/login.rs index e9af441..38fb95b 100644 --- a/src/login.rs +++ b/src/login.rs @@ -2,16 +2,16 @@ //! Postil web app, modelled on RFC 8628. Chosen because the CLI runs over //! SSH and in containers where no localhost browser callback is reachable. //! -//! A successful login writes a credential the API-key resolver -//! (`api_key::resolve_effective`) falls back to when no explicit key env var -//! is set, so a first-time user gets working hosted inference with zero -//! configuration. +//! A successful login writes a credential that `resolve_stored_token` uses +//! when no explicit key environment variable is set, so a first-time user gets +//! working hosted inference with zero configuration. use std::path::Path; use std::time::Duration; use anyhow::{Context, Result, anyhow}; use serde::Deserialize; +use serde::de::DeserializeOwned; use crate::credentials::{self, Credentials}; use crate::llm::secure_http_client; @@ -26,6 +26,9 @@ const DEFAULT_LOGIN_SERVER: &str = "https://postil.dev"; /// this is only a client-side backstop against a stuck loop if that cap is /// ever missed (a server bug, or a hostile `interval`). const MAX_POLL_ATTEMPTS: u32 = 220; +const ACCESS_REFRESH_MARGIN: Duration = Duration::from_secs(5 * 60); +const REFRESH_RESPONSE_MAX_BYTES: usize = 16 * 1024; +const REFRESH_REQUEST_TIMEOUT: Duration = Duration::from_secs(20); fn login_server() -> String { std::env::var(LOGIN_SERVER_ENV) @@ -67,12 +70,27 @@ struct DeviceTokenApproved { token: String, #[serde(rename = "expiresAt")] expires_at: String, + #[serde(rename = "refreshToken")] + refresh_token: Option, + #[serde(rename = "refreshExpiresAt")] + refresh_expires_at: Option, #[serde(rename = "apiBase")] api_base: String, org: DeviceTokenOrg, model: String, } +#[derive(Debug, Deserialize)] +struct RefreshTokenResponse { + token: String, + #[serde(rename = "expiresAt")] + expires_at: String, + #[serde(rename = "refreshToken")] + refresh_token: String, + #[serde(rename = "refreshExpiresAt")] + refresh_expires_at: String, +} + #[derive(Debug, Deserialize)] struct DeviceTokenOrg { slug: String, @@ -136,19 +154,64 @@ async fn login_with( .json() .await .context("parsing postil login approval response")?; - let creds = Credentials { - version: credentials::CREDENTIALS_VERSION, - token: approved.token, - expires_at: approved.expires_at, - api_base: approved.api_base, - org: approved.org.slug, - model: approved.model, + let DeviceTokenApproved { + token, + expires_at, + refresh_token, + refresh_expires_at, + api_base, + org, + model, + } = approved; + let renewable = match (refresh_token, refresh_expires_at) { + (Some(refresh_token), Some(refresh_expires_at)) => { + let credentials = Credentials { + version: credentials::CREDENTIALS_VERSION, + token, + expires_at, + refresh_token: Some(refresh_token), + refresh_expires_at: Some(refresh_expires_at), + api_base, + org: org.slug, + model, + }; + validate_refresh_response(&credentials) + .context("validating postil login approval response")?; + credentials + } + (None, None) => Credentials { + version: credentials::LEGACY_CREDENTIALS_VERSION, + token, + expires_at, + refresh_token: None, + refresh_expires_at: None, + api_base, + org: org.slug, + model, + }, + _ => anyhow::bail!( + "the postil login approval response has incomplete renewable credentials" + ), }; - credentials::write(credentials_path, &creds)?; - eprintln!( - "postil: logged in to {} ({}) -- credential expires {}", - approved.org.name, creds.org, creds.expires_at - ); + let renewable_login = renewable.refresh_token.is_some(); + let _lock = credentials::CredentialLock::acquire(credentials_path).await?; + credentials::write(credentials_path, &renewable)?; + if renewable_login { + eprintln!( + "postil: logged in to {} ({}): renewable until inactive after {}", + org.name, + renewable.org, + renewable + .refresh_expires_at + .as_deref() + .expect("renewable credentials have an expiry") + ); + } else { + eprintln!( + "postil: logged in to {} ({}) with an access-only credential", + org.name, renewable.org + ); + } return Ok(0); } 428 => continue, @@ -181,15 +244,21 @@ async fn logout_with( server: &str, credentials_path: &Path, ) -> Result { + let _lock = credentials::CredentialLock::acquire(credentials_path).await?; match credentials::read(credentials_path) { Ok(Some(creds)) => { let logout_url = format!("{}/api/cli/logout", server.trim_end_matches('/')); - match client - .post(&logout_url) - .bearer_auth(&creds.token) - .send() - .await + let request = client.post(&logout_url).bearer_auth(&creds.token); + let request = if let Some(refresh_token) = creds + .refresh_token + .as_deref() + .filter(|token| !token.trim().is_empty()) { + request.json(&serde_json::json!({ "refreshToken": refresh_token })) + } else { + request + }; + match request.send().await { Ok(response) if response.status().is_success() => {} Ok(response) => eprintln!( "postil: logout request returned {}; removing the local credential anyway", @@ -210,6 +279,137 @@ async fn logout_with( Ok(0) } +/// Resolves a stored login for a hosted request. Explicit API-key environment +/// variables are handled by the caller before this function is reached, so a +/// BYOK invocation never reads, locks, or refreshes a local Postil login. +pub(crate) async fn resolve_stored_token(credentials_path: &Path) -> Result> { + let server = login_server(); + let client = secure_http_client(&server).context("building the postil refresh HTTP client")?; + resolve_stored_token_with(&client, &server, credentials_path).await +} + +async fn resolve_stored_token_with( + client: &reqwest::Client, + server: &str, + credentials_path: &Path, +) -> Result> { + let Some(credentials) = credentials::read(credentials_path)? else { + return Ok(None); + }; + if credentials.refresh_token.is_some() && !credentials.can_refresh() { + anyhow::bail!("the stored postil login can no longer be renewed; run `postil login` again"); + } + if !credentials.expires_within(ACCESS_REFRESH_MARGIN) { + return Ok(Some(credentials.token)); + } + + let _lock = credentials::CredentialLock::acquire(credentials_path).await?; + let Some(credentials) = credentials::read(credentials_path)? else { + return Ok(None); + }; + if credentials.refresh_token.is_some() && !credentials.can_refresh() { + anyhow::bail!("the stored postil login can no longer be renewed; run `postil login` again"); + } + if !credentials.expires_within(ACCESS_REFRESH_MARGIN) { + return Ok(Some(credentials.token)); + } + let Some(refresh_token) = credentials.refresh_token.as_deref() else { + anyhow::bail!("the stored postil login credential expired; run `postil login` again"); + }; + + let refreshed = refresh_token_with(client, server, refresh_token).await?; + let replacement = Credentials { + version: credentials::CREDENTIALS_VERSION, + token: refreshed.token, + expires_at: refreshed.expires_at, + refresh_token: Some(refreshed.refresh_token), + refresh_expires_at: Some(refreshed.refresh_expires_at), + api_base: credentials.api_base, + org: credentials.org, + model: credentials.model, + }; + validate_refresh_response(&replacement)?; + credentials::write(credentials_path, &replacement)?; + Ok(Some(replacement.token)) +} + +async fn refresh_token_with( + client: &reqwest::Client, + server: &str, + refresh_token: &str, +) -> Result { + let refresh_url = format!("{}/api/cli/token/refresh", server.trim_end_matches('/')); + let response = tokio::time::timeout( + REFRESH_REQUEST_TIMEOUT, + client + .post(&refresh_url) + .json(&serde_json::json!({ "refreshToken": refresh_token })) + .send(), + ) + .await + .map_err(|_| anyhow!("could not refresh the stored postil login; try again"))? + .map_err(|_| anyhow!("could not refresh the stored postil login; try again"))?; + let status = response.status(); + if status.is_server_error() || matches!(status.as_u16(), 408 | 429) { + anyhow::bail!("could not refresh the stored postil login; try again"); + } + if !status.is_success() { + anyhow::bail!("the stored postil login can no longer be renewed; run `postil login` again"); + } + match tokio::time::timeout(REFRESH_REQUEST_TIMEOUT, bounded_json(response)).await { + Ok(Ok(response)) => Ok(response), + Ok(Err(RefreshResponseError::Invalid)) => { + anyhow::bail!("the postil refresh response was invalid; run `postil login` again") + } + Ok(Err(RefreshResponseError::Transport)) | Err(_) => { + anyhow::bail!("could not refresh the stored postil login; try again") + } + } +} + +enum RefreshResponseError { + Transport, + Invalid, +} + +async fn bounded_json( + mut response: reqwest::Response, +) -> std::result::Result { + let expected_length = response.content_length(); + if expected_length.is_some_and(|length| length > REFRESH_RESPONSE_MAX_BYTES as u64) { + return Err(RefreshResponseError::Invalid); + } + let mut bytes = Vec::new(); + while let Some(chunk) = response + .chunk() + .await + .map_err(|_| RefreshResponseError::Transport)? + { + if bytes.len().saturating_add(chunk.len()) > REFRESH_RESPONSE_MAX_BYTES { + return Err(RefreshResponseError::Invalid); + } + bytes.extend_from_slice(&chunk); + } + if expected_length.is_some_and(|length| length != bytes.len() as u64) { + return Err(RefreshResponseError::Transport); + } + serde_json::from_slice(&bytes).map_err(|_| RefreshResponseError::Invalid) +} + +fn validate_refresh_response(credentials: &Credentials) -> Result<()> { + anyhow::ensure!( + !credentials.token.trim().is_empty() + && credentials + .refresh_token + .as_deref() + .is_some_and(|token| !token.trim().is_empty()) + && !credentials.is_expired() + && credentials.can_refresh(), + "refresh response did not contain usable credentials" + ); + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -248,12 +448,24 @@ mod tests { "status": "approved", "token": "pcli_test-token-not-a-real-secret", "expiresAt": "2999-01-01T00:00:00.000Z", + "refreshToken": "fixture-refresh-not-a-credential", + "refreshExpiresAt": "2999-12-01T00:00:00.000Z", "apiBase": "https://postil.dev/api/inference/v1", "org": {"slug": "runatlas-is", "name": "RunAtlas"}, "model": "z-ai/glm-5.2", }) } + fn legacy_approved_body() -> serde_json::Value { + let mut body = approved_body(); + let object = body + .as_object_mut() + .expect("approved response fixture is an object"); + object.remove("refreshToken"); + object.remove("refreshExpiresAt"); + body + } + fn start_body() -> serde_json::Value { serde_json::json!({ "deviceCode": "test-device-code", @@ -294,6 +506,83 @@ mod tests { assert_eq!(stored.token, "pcli_test-token-not-a-real-secret"); assert_eq!(stored.org, "runatlas-is"); assert_eq!(stored.model, "z-ai/glm-5.2"); + assert_eq!( + stored.refresh_expires_at.as_deref(), + Some("2999-12-01T00:00:00.000Z") + ); + } + + #[tokio::test] + async fn login_waits_for_an_in_progress_stored_login_mutation() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/device/start")) + .respond_with(ResponseTemplate::new(200).set_body_json(start_body())) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/api/cli/device/token")) + .respond_with(ResponseTemplate::new(200).set_body_json(approved_body())) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let held_lock = credentials::CredentialLock::acquire(&credentials_path) + .await + .unwrap(); + let task_path = credentials_path.clone(); + let server_uri = server.uri(); + let login = tokio::spawn(async move { + login_with(&reqwest::Client::new(), &server_uri, None, &task_path).await + }); + + tokio::time::sleep(Duration::from_millis(100)).await; + assert!(!login.is_finished()); + assert!(!credentials_path.exists()); + drop(held_lock); + + let exit = tokio::time::timeout(Duration::from_secs(2), login) + .await + .expect("login should resume after the stored login lock is released") + .unwrap() + .unwrap(); + assert_eq!(exit, 0); + assert!(credentials::read(&credentials_path).unwrap().is_some()); + } + + #[tokio::test] + async fn login_accepts_a_legacy_access_only_approval_response() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/device/start")) + .respond_with(ResponseTemplate::new(200).set_body_json(start_body())) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/api/cli/device/token")) + .respond_with(ResponseTemplate::new(200).set_body_json(legacy_approved_body())) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + assert_eq!( + login_with( + &reqwest::Client::new(), + &server.uri(), + None, + &credentials_path, + ) + .await + .unwrap(), + 0 + ); + + let stored = credentials::read(&credentials_path).unwrap().unwrap(); + assert_eq!(stored.version, 1); + assert!(stored.refresh_token.is_none()); + assert!(stored.refresh_expires_at.is_none()); } #[tokio::test] @@ -407,6 +696,8 @@ mod tests { version: credentials::CREDENTIALS_VERSION, token: "pcli_test-token-not-a-real-secret".to_string(), expires_at: "2999-01-01T00:00:00.000Z".to_string(), + refresh_token: Some("fixture-refresh-not-a-credential".to_string()), + refresh_expires_at: Some("2999-12-01T00:00:00.000Z".to_string()), api_base: "https://postil.dev/api/inference/v1".to_string(), org: "runatlas-is".to_string(), model: "z-ai/glm-5.2".to_string(), @@ -433,4 +724,456 @@ mod tests { assert_eq!(exit, 0); assert!(!credentials_path.exists()); } + + #[tokio::test] + async fn logout_waits_for_an_in_progress_stored_login_mutation() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/logout")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + credentials::write( + &credentials_path, + &stored_credentials("2999-01-01T00:00:00.000Z"), + ) + .unwrap(); + let held_lock = credentials::CredentialLock::acquire(&credentials_path) + .await + .unwrap(); + let task_path = credentials_path.clone(); + let server_uri = server.uri(); + let logout = tokio::spawn(async move { + logout_with(&reqwest::Client::new(), &server_uri, &task_path).await + }); + + tokio::time::sleep(Duration::from_millis(100)).await; + assert!(!logout.is_finished()); + assert!(credentials_path.exists()); + drop(held_lock); + + let exit = tokio::time::timeout(Duration::from_secs(2), logout) + .await + .expect("logout should resume after the stored login lock is released") + .unwrap() + .unwrap(); + assert_eq!(exit, 0); + assert!(!credentials_path.exists()); + } + + fn stored_credentials(expires_at: &str) -> Credentials { + Credentials { + version: credentials::CREDENTIALS_VERSION, + token: "pcli_test-old-access-not-a-real-secret".to_string(), + expires_at: expires_at.to_string(), + refresh_token: Some("fixture-old-refresh-not-a-credential".to_string()), + refresh_expires_at: Some("2999-12-01T00:00:00.000Z".to_string()), + api_base: "https://postil.dev/api/inference/v1".to_string(), + org: "runatlas-is".to_string(), + model: "z-ai/glm-5.2".to_string(), + } + } + + fn write_legacy_credentials(path: &Path, credentials: &Credentials) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, serde_json::to_string(credentials).unwrap()).unwrap(); + } + + fn refresh_body() -> serde_json::Value { + serde_json::json!({ + "token": "pcli_test-new-access-not-a-real-secret", + "expiresAt": "2999-01-01T00:00:00.000Z", + "refreshToken": "fixture-new-refresh-not-a-credential", + "refreshExpiresAt": "2999-12-01T00:00:00.000Z" + }) + } + + #[tokio::test] + async fn refreshes_a_near_expiry_access_token_and_persists_rotation() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(200).set_body_json(refresh_body())) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + credentials::write( + &credentials_path, + &stored_credentials("2020-01-01T00:00:00.000Z"), + ) + .unwrap(); + + let resolved = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .unwrap(); + assert_eq!( + resolved.as_deref(), + Some("pcli_test-new-access-not-a-real-secret") + ); + let stored = credentials::read(&credentials_path).unwrap().unwrap(); + assert_eq!(stored.token, "pcli_test-new-access-not-a-real-secret"); + assert_eq!( + stored.refresh_token.as_deref(), + Some("fixture-new-refresh-not-a-credential") + ); + } + + #[tokio::test] + async fn refreshes_inside_the_proactive_access_margin() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(200).set_body_json(refresh_body())) + .expect(1) + .mount(&server) + .await; + let expires_at = (time::OffsetDateTime::now_utc() + time::Duration::minutes(2)) + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + credentials::write(&credentials_path, &stored_credentials(&expires_at)).unwrap(); + + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .unwrap(); + } + + #[tokio::test] + async fn concurrent_resolvers_share_one_refresh_rotation() { + #[derive(Clone)] + struct DelayedRefreshResponder { + calls: Arc, + } + + impl Respond for DelayedRefreshResponder { + fn respond(&self, _request: &Request) -> ResponseTemplate { + self.calls.fetch_add(1, Ordering::SeqCst); + ResponseTemplate::new(200) + .set_body_json(refresh_body()) + .set_delay(Duration::from_millis(100)) + } + } + + let server = MockServer::start().await; + let responder = DelayedRefreshResponder { + calls: Arc::new(AtomicUsize::new(0)), + }; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(responder.clone()) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + credentials::write( + &credentials_path, + &stored_credentials("2020-01-01T00:00:00.000Z"), + ) + .unwrap(); + let client = reqwest::Client::new(); + let server_uri = server.uri(); + + let (first, second) = tokio::join!( + resolve_stored_token_with(&client, &server_uri, &credentials_path), + resolve_stored_token_with(&client, &server_uri, &credentials_path) + ); + assert_eq!( + first.unwrap().as_deref(), + Some("pcli_test-new-access-not-a-real-secret") + ); + assert_eq!( + second.unwrap().as_deref(), + Some("pcli_test-new-access-not-a-real-secret") + ); + assert_eq!(responder.calls.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn failed_refresh_keeps_the_previous_credential() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(503).set_body_string("not shown")) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .expect_err("a failed refresh must fail closed"); + assert!(error.to_string().contains("try again")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } + + #[tokio::test] + async fn malformed_refresh_response_requires_relogin_without_leaking_it() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with( + ResponseTemplate::new(200).set_body_string("server body must stay private"), + ) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .expect_err("a malformed response must not be accepted"); + assert!(error.to_string().contains("postil login")); + assert!(!error.to_string().contains("server body must stay private")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } + + #[tokio::test] + async fn rejected_refresh_requires_relogin_without_leaking_the_server_body() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(401).set_body_string("refresh replay details")) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .expect_err("a replayed refresh token must require login"); + assert!(error.to_string().contains("postil login")); + assert!(!error.to_string().contains("refresh replay details")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } + + #[tokio::test] + async fn expired_v1_login_requires_relogin() { + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let mut legacy = stored_credentials("2020-01-01T00:00:00.000Z"); + legacy.version = 1; + legacy.refresh_token = None; + legacy.refresh_expires_at = None; + write_legacy_credentials(&credentials_path, &legacy); + + let error = resolve_stored_token_with( + &reqwest::Client::new(), + "http://127.0.0.1:9", + &credentials_path, + ) + .await + .expect_err("expired legacy credentials must require login"); + assert!(error.to_string().contains("postil login")); + } + + #[tokio::test] + async fn unexpired_v1_login_remains_usable_without_a_refresh() { + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let mut legacy = stored_credentials("2999-01-01T00:00:00.000Z"); + legacy.version = 1; + legacy.refresh_token = None; + legacy.refresh_expires_at = None; + write_legacy_credentials(&credentials_path, &legacy); + + let resolved = resolve_stored_token_with( + &reqwest::Client::new(), + "http://127.0.0.1:9", + &credentials_path, + ) + .await + .unwrap(); + assert_eq!( + resolved.as_deref(), + Some("pcli_test-old-access-not-a-real-secret") + ); + } + + #[tokio::test] + async fn expired_refresh_inactivity_requires_relogin() { + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let mut expired = stored_credentials("2999-01-01T00:00:00.000Z"); + expired.refresh_expires_at = Some("2020-01-01T00:00:00.000Z".to_string()); + credentials::write(&credentials_path, &expired).unwrap(); + + let error = resolve_stored_token_with( + &reqwest::Client::new(), + "http://127.0.0.1:9", + &credentials_path, + ) + .await + .expect_err("expired refresh inactivity must require login"); + assert!(error.to_string().contains("postil login")); + } + + #[tokio::test] + async fn logout_sends_refresh_token_even_after_access_expiry() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/logout")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + credentials::write( + &credentials_path, + &stored_credentials("2020-01-01T00:00:00.000Z"), + ) + .unwrap(); + + logout_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .unwrap(); + assert!(!credentials_path.exists()); + let requests = server.received_requests().await.unwrap(); + let request = requests.first().unwrap(); + assert!(request.headers.contains_key("authorization")); + assert!( + std::str::from_utf8(&request.body) + .unwrap() + .contains("refreshToken") + ); + } + + #[tokio::test] + async fn legacy_logout_uses_its_access_bearer_without_a_null_refresh_body() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/logout")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let mut legacy = stored_credentials("2999-01-01T00:00:00.000Z"); + legacy.version = 1; + legacy.refresh_token = None; + legacy.refresh_expires_at = None; + credentials::write(&credentials_path, &legacy).unwrap(); + + assert_eq!( + logout_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .unwrap(), + 0 + ); + assert!(!credentials_path.exists()); + let requests = server.received_requests().await.unwrap(); + assert!(requests.first().unwrap().body.is_empty()); + } + + #[tokio::test] + async fn rate_limited_refresh_keeps_the_previous_credential() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(429).set_body_string("retry later")) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .expect_err("rate-limited refreshes must fail closed without relogin"); + assert!(error.to_string().contains("try again")); + assert!(!error.to_string().contains("run `postil login` again")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } + + #[tokio::test] + async fn request_timeout_refresh_keeps_the_previous_credential() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/api/cli/token/refresh")) + .respond_with(ResponseTemplate::new(408).set_body_string("retry later")) + .mount(&server) + .await; + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = + resolve_stored_token_with(&reqwest::Client::new(), &server.uri(), &credentials_path) + .await + .expect_err("request-timeout refreshes must retain the credential"); + assert!(error.to_string().contains("try again")); + assert!(!error.to_string().contains("run `postil login` again")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } + + #[tokio::test] + async fn truncated_successful_refresh_response_is_retryable_and_preserves_the_credential() { + use std::io::{Read, Write}; + + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = [0_u8; 4_096]; + let _ = stream.read(&mut request); + stream + .write_all( + b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 4096\r\nConnection: close\r\n\r\n{\"token\":\"committed-but-truncated\"", + ) + .unwrap(); + }); + let dir = tempfile::tempdir().unwrap(); + let credentials_path = dir.path().join("postil").join("credentials.json"); + let original = stored_credentials("2020-01-01T00:00:00.000Z"); + credentials::write(&credentials_path, &original).unwrap(); + + let error = resolve_stored_token_with( + &reqwest::Client::new(), + &format!("http://{address}"), + &credentials_path, + ) + .await + .expect_err("a truncated committed response must be retried with the old credential"); + + server.join().unwrap(); + assert!(error.to_string().contains("try again")); + assert!(!error.to_string().contains("run `postil login` again")); + assert_eq!( + credentials::read(&credentials_path).unwrap().unwrap(), + original + ); + } } diff --git a/src/main.rs b/src/main.rs index 9354f36..fbc3c29 100644 --- a/src/main.rs +++ b/src/main.rs @@ -201,6 +201,8 @@ async fn dispatch(cli: Cli) -> anyhow::Result { println!("model.apiBase: {}", cfg.api_base); println!("model.apiFormat: {}", cfg.api_format.as_str()); println!("model.consensus: {}", cfg.consensus); + let login = doctor::login_status(); + println!("login.status: {}", login.detail); Ok(0) } Command::Init { force } => { diff --git a/src/respond.rs b/src/respond.rs index 3f65d96..0206fa0 100644 --- a/src/respond.rs +++ b/src/respond.rs @@ -297,7 +297,7 @@ async fn respond_with( let system = prompt::respond_system_prompt(cfg, current_utc_date); let user = respond_user_prompt(&context, comment); - let client = LlmClient::from_env(cfg)?; + let client = LlmClient::from_env(cfg).await?; client.preflight_respond_plan(cfg, &system, &user)?; let answer = client .answer(cfg, &system, &user, validate_respond_output) diff --git a/src/review.rs b/src/review.rs index b58352b..af42138 100644 --- a/src/review.rs +++ b/src/review.rs @@ -1755,16 +1755,19 @@ async fn review_diff_at( None }; let client = match llm_budget_started_at { - Some(started_at) => LlmClient::from_env_for_remote_review( - cfg, - started_at, - Duration::from_secs(hosted_request_timeout_secs( - deterministic_large_review, - )), - Duration::from_secs(hosted_review_timeout_secs(cfg)), - Duration::from_secs(HOSTED_LLM_TOTAL_TIMEOUT_SECS), - )?, - None => LlmClient::from_env(cfg)?, + Some(started_at) => { + LlmClient::from_env_for_remote_review( + cfg, + started_at, + Duration::from_secs(hosted_request_timeout_secs( + deterministic_large_review, + )), + Duration::from_secs(hosted_review_timeout_secs(cfg)), + Duration::from_secs(HOSTED_LLM_TOTAL_TIMEOUT_SECS), + ) + .await? + } + None => LlmClient::from_env(cfg).await?, }; let planned_batch_count = large_diff_receipt.as_ref().map_or_else( || {