From 6aa6b84afd09ec8a389053926a98d65b36ac6fd1 Mon Sep 17 00:00:00 2001
From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
Date: Sun, 19 Jul 2026 18:52:48 -0700
Subject: [PATCH 1/3] fix(auth): serialize token refresh and make
 config/session writes atomic

Concurrent hotdata invocations raced on the shared state in ~/.hotdata:

- config.yml updates (e.g. save_current_database on every `databases
  create`) were unlocked read-modify-write cycles ending in a truncating
  fs::write, so parallel commands read half-written YAML ("error parsing
  config file") and dropped each other's entries.
- When the 5-minute access token expired mid-burst, every process spent
  the same refresh token against /o/token/. The server treats reuse of a
  rotated refresh token as compromise and revokes the session; the losers
  then cleared session.json, deleting the winner's fresh session. Under
  sustained concurrency this killed the login session entirely (observed:
  a 128-way burst going 0-for-35k with "session expired or revoked").

Fix:

- Write config.yml, session.json, and database_session.json via
  tempfile-in-same-dir + rename so readers never observe partial files
  (tempfile creates 0600 and rename preserves it for the session files).
- Take a blocking exclusive flock (config.lock) around the config
  read-modify-write helpers.
- Serialize ensure_access_token's refresh/re-mint slow path under
  session.lock, re-checking the cached session after acquiring the lock
  so waiters reuse the winner's token instead of spending the refresh
  token again. The valid-cache fast path stays lock-free. Locks are
  advisory and best-effort: without flock support behavior degrades to
  the previous unserialized writes.

Verified with a 60s create burst at 32-256 concurrent processes: zero
auth/config errors across ~11k creates, where the same run previously
revoked the session.
---
 src/client/database_session.rs | 20 ++++----
 src/client/jwt.rs              | 94 ++++++++++++++++++++++++++++------
 src/config.rs                  | 76 +++++++++++++++++++++++++--
 3 files changed, 160 insertions(+), 30 deletions(-)

diff --git a/src/client/database_session.rs b/src/client/database_session.rs
index b1f6fe6..d62d870 100644
--- a/src/client/database_session.rs
+++ b/src/client/database_session.rs
@@ -50,15 +50,17 @@ pub fn save(session: &DatabaseSession) -> Result<(), String> {
     let json =
         serde_json::to_string_pretty(session).map_err(|e| format!("serialize failed: {e}"))?;
 
-    use std::os::unix::fs::OpenOptionsExt;
-    let mut f = fs::OpenOptions::new()
-        .create(true)
-        .write(true)
-        .truncate(true)
-        .mode(0o600)
-        .open(&path)
-        .map_err(|e| format!("open failed: {e}"))?;
-    f.write_all(json.as_bytes())
+    // Write-to-temp + rename so a concurrent load never reads a half-written
+    // file. tempfile creates with mode 0600 on Unix and the rename preserves
+    // it — the file contains a refresh token, treat it like a credential.
+    let parent = path
+        .parent()
+        .ok_or_else(|| "session path has no parent directory".to_string())?;
+    let mut tmp =
+        tempfile::NamedTempFile::new_in(parent).map_err(|e| format!("open failed: {e}"))?;
+    tmp.write_all(json.as_bytes())
+        .map_err(|e| format!("write failed: {e}"))?;
+    tmp.persist(&path)
         .map_err(|e| format!("write failed: {e}"))?;
     Ok(())
 }
diff --git a/src/client/jwt.rs b/src/client/jwt.rs
index 4db5a74..bdd24c6 100644
--- a/src/client/jwt.rs
+++ b/src/client/jwt.rs
@@ -67,17 +67,18 @@ pub fn save_session(session: &Session) -> Result<(), String> {
     let json =
         serde_json::to_string_pretty(session).map_err(|e| format!("serialize failed: {e}"))?;
 
-    // mode 0600 — session file contains a refresh token, treat it like a
-    // credential on disk.
-    use std::os::unix::fs::OpenOptionsExt;
-    let mut f = fs::OpenOptions::new()
-        .create(true)
-        .write(true)
-        .truncate(true)
-        .mode(0o600)
-        .open(&path)
-        .map_err(|e| format!("open failed: {e}"))?;
-    f.write_all(json.as_bytes())
+    // Write-to-temp + rename so a concurrent load_session never reads a
+    // half-written file. tempfile creates with mode 0600 on Unix and the
+    // rename preserves it — the session file contains a refresh token,
+    // treat it like a credential on disk.
+    let parent = path
+        .parent()
+        .ok_or_else(|| "session path has no parent directory".to_string())?;
+    let mut tmp =
+        tempfile::NamedTempFile::new_in(parent).map_err(|e| format!("open failed: {e}"))?;
+    tmp.write_all(json.as_bytes())
+        .map_err(|e| format!("write failed: {e}"))?;
+    tmp.persist(&path)
         .map_err(|e| format!("write failed: {e}"))?;
     Ok(())
 }
@@ -320,14 +321,35 @@ pub fn ensure_access_token(
         return Ok(session.access_token);
     }
 
-    let now = now_unix();
+    fn usable_token(session: &Session, now: u64) -> Option<String> {
+        (!session.access_token.is_empty()
+            && now + REFRESH_LEEWAY_SECONDS < session.access_expires_at)
+            .then(|| session.access_token.clone())
+    }
+
+    // 1) Cached session is still good — read-only fast path, no lock.
+    if let Some(session) = load_session()
+        && let Some(tok) = usable_token(&session, now_unix())
+    {
+        return Ok(tok);
+    }
 
-    // 1) Cached session is still good.
+    // Refreshing/re-minting rewrites session.json, which every concurrent
+    // `hotdata` process shares. Serialize the slow path across processes so
+    // an expiry mid-burst produces one refresh instead of N racing ones —
+    // the losers of that race saw "session expired or revoked" and their
+    // clear_session() deleted the winner's freshly saved session. Advisory
+    // and best-effort: without flock support this degrades to the old
+    // unserialized behavior.
+    let _lock = config::lock_file("session.lock");
+
+    let now = now_unix();
     if let Some(session) = load_session() {
-        if !session.access_token.is_empty()
-            && now + REFRESH_LEEWAY_SECONDS < session.access_expires_at
-        {
-            return Ok(session.access_token);
+        // Re-check after acquiring the lock: another process may have
+        // refreshed while we waited — reuse its token instead of spending
+        // (and potentially invalidating) the shared refresh token again.
+        if let Some(tok) = usable_token(&session, now) {
+            return Ok(tok);
         }
 
         // 2) Access expired but refresh might still work.
@@ -1112,6 +1134,44 @@ mod tests {
         assert_eq!(token, "cached-jwt");
     }
 
+    #[test]
+    fn ensure_concurrent_expiry_refreshes_once_and_shares_result() {
+        // Regression: when the access token expired mid-burst, N concurrent
+        // processes all spent the same refresh token. The winner saved a new
+        // session; the losers got a rejection, surfaced "session expired or
+        // revoked", and their clear_session() deleted the winner's fresh
+        // session. Under the session lock exactly one refresh runs and the
+        // waiters reuse its result.
+        let (_tmp, _guard) = with_temp_config_dir();
+        let mut server = mockito::Server::new();
+        let refresh_mock = server
+            .mock("POST", "/o/token/")
+            .match_body(mockito::Matcher::UrlEncoded(
+                "grant_type".into(),
+                "refresh_token".into(),
+            ))
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"access_token":"refreshed-jwt","expires_in":300,"refresh_token":"r2"}"#)
+            .expect(1)
+            .create();
+
+        save_session(&cached_session(-10, 86400)).unwrap();
+        let profile = mock_profile(&server.url());
+
+        let handles: Vec<_> = (0..8)
+            .map(|_| {
+                let profile = profile.clone();
+                std::thread::spawn(move || ensure_access_token(&profile, None).unwrap())
+            })
+            .collect();
+        for h in handles {
+            assert_eq!(h.join().unwrap(), "refreshed-jwt");
+        }
+        refresh_mock.assert();
+        assert_eq!(load_session().unwrap().access_token, "refreshed-jwt");
+    }
+
     #[test]
     fn ensure_clears_session_when_refresh_dies_with_no_fallback() {
         let (_tmp, _guard) = with_temp_config_dir();
diff --git a/src/config.rs b/src/config.rs
index 4bd9d43..c30a61d 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,9 +1,12 @@
 use crossterm::style::Stylize;
 use directories::UserDirs;
+use nix::fcntl::{Flock, FlockArg};
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::env;
 use std::fs;
+use std::fs::File;
+use std::io::Write;
 use std::ops::Deref;
 use std::path::PathBuf;
 
@@ -109,10 +112,41 @@ pub struct ConfigFile {
 }
 
 fn write_config(config_path: &std::path::Path, content: &str) -> Result<(), String> {
-    if let Some(parent) = config_path.parent() {
-        fs::create_dir_all(parent).map_err(|e| format!("error creating config directory: {e}"))?;
-    }
-    fs::write(config_path, content).map_err(|e| format!("error writing config file: {e}"))
+    let parent = config_path
+        .parent()
+        .ok_or("config path has no parent directory")?;
+    fs::create_dir_all(parent).map_err(|e| format!("error creating config directory: {e}"))?;
+    // Write-to-temp + rename so concurrent readers never observe a
+    // truncated or half-written file (a plain fs::write truncates first,
+    // and parallel invocations were hitting "error parsing config file").
+    let mut tmp = tempfile::NamedTempFile::new_in(parent)
+        .map_err(|e| format!("error writing config file: {e}"))?;
+    tmp.write_all(content.as_bytes())
+        .map_err(|e| format!("error writing config file: {e}"))?;
+    tmp.persist(config_path)
+        .map_err(|e| format!("error writing config file: {e}"))?;
+    Ok(())
+}
+
+/// Exclusive advisory lock on `<config_dir>/<name>`, blocking until granted;
+/// released on drop. Serializes read-modify-write cycles on shared on-disk
+/// state (config.yml updates, session refresh) across concurrent `hotdata`
+/// processes. Best-effort by design: on filesystems without flock support
+/// callers proceed unlocked, which is the pre-lock behavior.
+pub(crate) fn lock_file(name: &str) -> Option<Flock<File>> {
+    let dir = config_dir().ok()?;
+    fs::create_dir_all(&dir).ok()?;
+    let f = fs::OpenOptions::new()
+        .create(true)
+        .write(true)
+        .truncate(false)
+        .open(dir.join(name))
+        .ok()?;
+    Flock::lock(f, FlockArg::LockExclusive).ok()
+}
+
+fn lock_config() -> Option<Flock<File>> {
+    lock_file("config.lock")
 }
 
 /// Wipe the workspace cache for a profile. Paired with
@@ -120,6 +154,7 @@ fn write_config(config_path: &std::path::Path, content: &str) -> Result<(), Stri
 /// on-disk state that login populates.
 pub fn clear_workspaces(profile: &str) -> Result<(), String> {
     let config_path = config_path()?;
+    let _lock = lock_config();
 
     if !config_path.exists() {
         return Ok(());
@@ -141,6 +176,7 @@ pub fn clear_workspaces(profile: &str) -> Result<(), String> {
 
 pub fn save_workspaces(profile: &str, workspaces: Vec<WorkspaceEntry>) -> Result<(), String> {
     let config_path = config_path()?;
+    let _lock = lock_config();
 
     let mut config_file: ConfigFile = if config_path.exists() {
         let content = fs::read_to_string(&config_path)
@@ -166,6 +202,7 @@ pub fn save_workspaces(profile: &str, workspaces: Vec<WorkspaceEntry>) -> Result
 
 pub fn save_default_workspace(profile: &str, workspace: WorkspaceEntry) -> Result<(), String> {
     let config_path = config_path()?;
+    let _lock = lock_config();
 
     let mut config_file: ConfigFile = if config_path.exists() {
         let content = fs::read_to_string(&config_path)
@@ -194,6 +231,7 @@ pub fn save_current_database(
     database_id: &str,
 ) -> Result<(), String> {
     let config_path = config_path()?;
+    let _lock = lock_config();
 
     let mut config_file: ConfigFile = if config_path.exists() {
         let content = fs::read_to_string(&config_path)
@@ -234,6 +272,7 @@ pub fn load_current_database(profile: &str, workspace_id: &str) -> Option<String
 
 pub fn clear_current_database(profile: &str, workspace_id: &str) -> Result<(), String> {
     let config_path = config_path()?;
+    let _lock = lock_config();
 
     if !config_path.exists() {
         return Ok(());
@@ -419,6 +458,35 @@ mod tests {
         assert_eq!(staging.workspaces[0].public_id, "ws-staging");
     }
 
+    #[test]
+    fn concurrent_saves_keep_all_entries_and_parse_cleanly() {
+        // Regression: parallel `hotdata` invocations doing read-modify-write
+        // on config.yml used to tear the file (readers hit "error parsing
+        // config file") and drop each other's entries. The config lock +
+        // atomic rename must keep every writer's entry.
+        let (_tmp, _guard) = with_temp_config_dir();
+        let threads: Vec<_> = (0..8)
+            .map(|i| {
+                std::thread::spawn(move || {
+                    save_current_database("default", &format!("ws-{i}"), &format!("db-{i}"))
+                        .unwrap()
+                })
+            })
+            .collect();
+        for t in threads {
+            t.join().unwrap();
+        }
+
+        let profile = load("default").unwrap();
+        for i in 0..8 {
+            assert_eq!(
+                profile.current_databases.get(&format!("ws-{i}")),
+                Some(&format!("db-{i}")),
+                "entry ws-{i} lost by a concurrent writer"
+            );
+        }
+    }
+
     #[test]
     fn legacy_api_key_in_yaml_is_ignored_on_load() {
         // Older configs (pre-jwt-branch) had `api_key: hd_xxx` written

From e7e42ea4cf86c0b4d63726ddd46e36eb4aa3a878 Mon Sep 17 00:00:00 2001
From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
Date: Sun, 19 Jul 2026 19:57:29 -0700
Subject: [PATCH 2/3] =?UTF-8?q?fix(auth):=20address=20review=20findings=20?=
 =?UTF-8?q?=E2=80=94=20timeout=20token=20client,=20typed=20refresh=20error?=
 =?UTF-8?q?s,=20locked=20config=20primitive?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review follow-ups on the concurrency fix (PR #230):

- Token endpoints (jwt.rs mints/refresh, database_session.rs refresh) now
  use raw_http::build_http_client() (300s timeout + keepalive) instead of
  bare reqwest Client::new(), which has no timeout. With the refresh now
  running under session.lock, a stalled /o/token/ would otherwise hang the
  lock holder — and with it every concurrent hotdata invocation.
- refresh() returns a typed RefreshError: Rejected (server refused the
  grant → clear the session and fall through to re-mint, as before) vs
  Transport (request never completed → keep the session, which was not
  consumed, and surface the real connection error instead of forcing a
  re-login).
- A failed save_session after a successful refresh now prints a warning
  instead of being silently dropped — an unpersisted rotation sends the
  next process back to the spent refresh token.
- lock_file() warns on unexpected failures (perms, I/O) instead of
  silently degrading; flock-unsupported filesystems still degrade silently.
- All five config read-modify-write functions now go through one locked
  update_config() primitive, so future mutations can't forget the lock.
- The tempfile+rename block is extracted to util::atomic_write(path,
  bytes, mode); modes are explicit — 0600 for both session files, 0644
  for config.yml (preserving its pre-atomic-write mode).

New tests: transport-level refresh failure keeps the session; config.yml
stays 0644.
---
 src/client/database_session.rs |  23 +---
 src/client/jwt.rs              | 115 +++++++++++-----
 src/config.rs                  | 237 ++++++++++++++++-----------------
 src/util.rs                    |  23 ++++
 4 files changed, 228 insertions(+), 170 deletions(-)

diff --git a/src/client/database_session.rs b/src/client/database_session.rs
index d62d870..f9430ab 100644
--- a/src/client/database_session.rs
+++ b/src/client/database_session.rs
@@ -12,7 +12,6 @@ use crate::config;
 use crate::util;
 use serde::{Deserialize, Serialize};
 use std::fs;
-use std::io::Write;
 use std::path::PathBuf;
 use std::time::{SystemTime, UNIX_EPOCH};
 
@@ -44,25 +43,13 @@ pub fn load() -> Option<DatabaseSession> {
 
 pub fn save(session: &DatabaseSession) -> Result<(), String> {
     let path = session_path().ok_or_else(|| "no database session path available".to_string())?;
-    if let Some(parent) = path.parent() {
-        fs::create_dir_all(parent).map_err(|e| format!("mkdir failed: {e}"))?;
-    }
     let json =
         serde_json::to_string_pretty(session).map_err(|e| format!("serialize failed: {e}"))?;
 
-    // Write-to-temp + rename so a concurrent load never reads a half-written
-    // file. tempfile creates with mode 0600 on Unix and the rename preserves
-    // it — the file contains a refresh token, treat it like a credential.
-    let parent = path
-        .parent()
-        .ok_or_else(|| "session path has no parent directory".to_string())?;
-    let mut tmp =
-        tempfile::NamedTempFile::new_in(parent).map_err(|e| format!("open failed: {e}"))?;
-    tmp.write_all(json.as_bytes())
-        .map_err(|e| format!("write failed: {e}"))?;
-    tmp.persist(&path)
-        .map_err(|e| format!("write failed: {e}"))?;
-    Ok(())
+    // Atomic replace so a concurrent load never reads a half-written file;
+    // 0600 because the file contains a refresh token — treat it like a
+    // credential on disk.
+    util::atomic_write(&path, json.as_bytes(), 0o600)
 }
 
 #[allow(dead_code)] // Reserved for flows that re-use a cached database session.
@@ -106,7 +93,7 @@ pub fn refresh(api_url: &str, refresh_token: &str) -> Result<DatabaseSession, St
         "refresh_token": redact(refresh_token),
     });
 
-    let client = reqwest::blocking::Client::new();
+    let client = crate::client::raw_http::build_http_client();
     let req = client.post(&url).json(&body);
     let (status, body_text) =
         util::send_debug_with_redaction(&client, req, Some(&body_log), &["token", "refresh_token"])
diff --git a/src/client/jwt.rs b/src/client/jwt.rs
index bdd24c6..5329eb1 100644
--- a/src/client/jwt.rs
+++ b/src/client/jwt.rs
@@ -18,11 +18,12 @@
 //! persisted to the session file — it stays in the main config or the
 //! `HOTDATA_API_KEY` env var and is only used transiently to mint.
 
+use crate::client::raw_http::build_http_client;
 use crate::config;
 use crate::util;
+use crossterm::style::Stylize;
 use serde::{Deserialize, Serialize};
 use std::fs;
-use std::io::Write;
 use std::path::PathBuf;
 use std::time::{SystemTime, UNIX_EPOCH};
 
@@ -61,26 +62,13 @@ pub fn load_session() -> Option<Session> {
 
 pub fn save_session(session: &Session) -> Result<(), String> {
     let path = session_path().ok_or_else(|| "no session path available".to_string())?;
-    if let Some(parent) = path.parent() {
-        fs::create_dir_all(parent).map_err(|e| format!("mkdir failed: {e}"))?;
-    }
     let json =
         serde_json::to_string_pretty(session).map_err(|e| format!("serialize failed: {e}"))?;
 
-    // Write-to-temp + rename so a concurrent load_session never reads a
-    // half-written file. tempfile creates with mode 0600 on Unix and the
-    // rename preserves it — the session file contains a refresh token,
-    // treat it like a credential on disk.
-    let parent = path
-        .parent()
-        .ok_or_else(|| "session path has no parent directory".to_string())?;
-    let mut tmp =
-        tempfile::NamedTempFile::new_in(parent).map_err(|e| format!("open failed: {e}"))?;
-    tmp.write_all(json.as_bytes())
-        .map_err(|e| format!("write failed: {e}"))?;
-    tmp.persist(&path)
-        .map_err(|e| format!("write failed: {e}"))?;
-    Ok(())
+    // Atomic replace so a concurrent load_session never reads a half-written
+    // file; 0600 because the session file contains a refresh token — treat it
+    // like a credential on disk.
+    util::atomic_write(&path, json.as_bytes(), 0o600)
 }
 
 pub fn clear_session() {
@@ -181,7 +169,7 @@ pub fn exchange_cli_register_code(
         "code_verifier": util::mask_credential(code_verifier),
     });
 
-    let client = reqwest::blocking::Client::new();
+    let client = build_http_client();
     let req = client.post(&url).json(&body);
     let (status, body_text) =
         util::send_debug_with_redaction(&client, req, Some(&body_log), &["token"])
@@ -218,7 +206,7 @@ pub fn mint_from_pkce_code(
         ("client_id", CLIENT_ID),
     ];
 
-    let client = reqwest::blocking::Client::new();
+    let client = build_http_client();
     let req = client.post(&url).form(&params);
     let body_log = redacted_form_body(&params);
     let (status, body_text) =
@@ -244,7 +232,7 @@ pub fn mint_from_api_token(
         ("client_id", CLIENT_ID),
     ];
 
-    let client = reqwest::blocking::Client::new();
+    let client = build_http_client();
     let req = client.post(&url).form(&params);
     let body_log = redacted_form_body(&params);
     let (status, body_text) =
@@ -260,8 +248,32 @@ pub fn mint_from_api_token(
     Ok(session_from_response(body, None, "api_token"))
 }
 
+/// Why a refresh attempt failed. The distinction decides what happens to the
+/// on-disk session: a rejection means the server saw the grant and refused
+/// it, so the session is dead; a transport failure means the request never
+/// completed and the stored refresh token was not consumed.
+#[derive(Debug)]
+pub enum RefreshError {
+    /// The request never completed (DNS, connect, timeout).
+    Transport(String),
+    /// The server answered and refused the grant (or returned an unusable
+    /// body).
+    Rejected(String),
+}
+
+impl std::fmt::Display for RefreshError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            RefreshError::Transport(msg) | RefreshError::Rejected(msg) => f.write_str(msg),
+        }
+    }
+}
+
 /// Refresh an existing session via the refresh-token grant.
-pub fn refresh(profile: &config::ProfileConfig, session: &Session) -> Result<Session, String> {
+pub fn refresh(
+    profile: &config::ProfileConfig,
+    session: &Session,
+) -> Result<Session, RefreshError> {
     let url = format!("{}/o/token/", oauth_base(profile));
     let params = [
         ("grant_type", "refresh_token"),
@@ -269,17 +281,19 @@ pub fn refresh(profile: &config::ProfileConfig, session: &Session) -> Result<Ses
         ("client_id", CLIENT_ID),
     ];
 
-    let client = reqwest::blocking::Client::new();
+    let client = build_http_client();
     let req = client.post(&url).form(&params);
     let body_log = redacted_form_body(&params);
     let (status, body_text) =
         util::send_debug_with_redaction(&client, req, Some(&body_log), TOKEN_REDACT_KEYS)
-            .map_err(|e| format!("connection error: {e}"))?;
+            .map_err(|e| RefreshError::Transport(format!("connection error: {e}")))?;
     if !status.is_success() {
-        return Err(format!("refresh failed: HTTP {status}: {body_text}"));
+        return Err(RefreshError::Rejected(format!(
+            "refresh failed: HTTP {status}: {body_text}"
+        )));
     }
-    let body: TokenResponse =
-        serde_json::from_str(&body_text).map_err(|e| format!("malformed token response: {e}"))?;
+    let body: TokenResponse = serde_json::from_str(&body_text)
+        .map_err(|e| RefreshError::Rejected(format!("malformed token response: {e}")))?;
     Ok(session_from_response(
         body,
         // Rotation is off server-side, so the same refresh token
@@ -357,13 +371,30 @@ pub fn ensure_access_token(
             match refresh(profile, &session) {
                 Ok(new_session) => {
                     let tok = new_session.access_token.clone();
-                    let _ = save_session(&new_session);
+                    if let Err(e) = save_session(&new_session) {
+                        // Not fatal for this invocation — the token is
+                        // valid — but the next process can't see it and
+                        // will spend the refresh token again, so make the
+                        // persistence failure visible.
+                        eprintln!(
+                            "{}",
+                            format!("warning: could not persist refreshed session: {e}").yellow()
+                        );
+                    }
                     return Ok(tok);
                 }
-                Err(_) => {
-                    // Refresh rejected — fall through to re-mint.
+                Err(RefreshError::Rejected(_)) => {
+                    // The server refused the grant: the session is dead.
+                    // Fall through to re-mint.
                     clear_session();
                 }
+                Err(RefreshError::Transport(e)) => {
+                    // The request never completed (network down, DNS,
+                    // timeout): the stored session was not consumed and is
+                    // likely still good, so keep it and surface the real
+                    // failure instead of forcing a re-login.
+                    return Err(e);
+                }
             }
         }
     }
@@ -822,7 +853,10 @@ mod tests {
         };
         let err = refresh(&profile, &session).unwrap_err();
         m.assert();
-        assert!(err.contains("400"), "got: {err}");
+        assert!(
+            matches!(&err, RefreshError::Rejected(msg) if msg.contains("400")),
+            "got: {err}"
+        );
     }
 
     // --- ensure_access_token: each branch of the decision table ---
@@ -1134,6 +1168,25 @@ mod tests {
         assert_eq!(token, "cached-jwt");
     }
 
+    #[test]
+    fn ensure_keeps_session_when_refresh_fails_at_transport_level() {
+        // A transport failure (network down, DNS, timeout) means the refresh
+        // token was never consumed — the session must survive so a working
+        // network can still use it, and the caller gets the real connection
+        // error instead of "session expired or revoked" + forced re-login.
+        let (_tmp, _guard) = with_temp_config_dir();
+        save_session(&cached_session(-10, 86400)).unwrap();
+
+        // Nothing listens on port 1 — connection refused, not an HTTP reply.
+        let profile = mock_profile("http://127.0.0.1:1");
+        let err = ensure_access_token(&profile, None).unwrap_err();
+        assert!(err.contains("connection error"), "got: {err}");
+        assert!(
+            load_session().is_some(),
+            "session must not be cleared on a transport failure"
+        );
+    }
+
     #[test]
     fn ensure_concurrent_expiry_refreshes_once_and_shares_result() {
         // Regression: when the access token expired mid-burst, N concurrent
diff --git a/src/config.rs b/src/config.rs
index c30a61d..f179789 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,12 +1,12 @@
 use crossterm::style::Stylize;
 use directories::UserDirs;
+use nix::errno::Errno;
 use nix::fcntl::{Flock, FlockArg};
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::env;
 use std::fs;
 use std::fs::File;
-use std::io::Write;
 use std::ops::Deref;
 use std::path::PathBuf;
 
@@ -112,117 +112,124 @@ pub struct ConfigFile {
 }
 
 fn write_config(config_path: &std::path::Path, content: &str) -> Result<(), String> {
-    let parent = config_path
-        .parent()
-        .ok_or("config path has no parent directory")?;
-    fs::create_dir_all(parent).map_err(|e| format!("error creating config directory: {e}"))?;
-    // Write-to-temp + rename so concurrent readers never observe a
-    // truncated or half-written file (a plain fs::write truncates first,
-    // and parallel invocations were hitting "error parsing config file").
-    let mut tmp = tempfile::NamedTempFile::new_in(parent)
-        .map_err(|e| format!("error writing config file: {e}"))?;
-    tmp.write_all(content.as_bytes())
-        .map_err(|e| format!("error writing config file: {e}"))?;
-    tmp.persist(config_path)
-        .map_err(|e| format!("error writing config file: {e}"))?;
-    Ok(())
+    // Atomic replace so concurrent readers never observe a truncated or
+    // half-written file (a plain fs::write truncates first, and parallel
+    // invocations were hitting "error parsing config file"). 0644 keeps the
+    // pre-atomic-write mode: config.yml holds no credentials.
+    crate::util::atomic_write(config_path, content.as_bytes(), 0o644)
+        .map_err(|e| format!("error writing config file: {e}"))
 }
 
 /// Exclusive advisory lock on `<config_dir>/<name>`, blocking until granted;
 /// released on drop. Serializes read-modify-write cycles on shared on-disk
 /// state (config.yml updates, session refresh) across concurrent `hotdata`
-/// processes. Best-effort by design: on filesystems without flock support
-/// callers proceed unlocked, which is the pre-lock behavior.
+/// processes. Best-effort by design: callers proceed unlocked when no lock
+/// can be taken — silently where flock simply isn't supported (some network
+/// mounts), with a warning for unexpected failures so broken locking doesn't
+/// masquerade as working.
 pub(crate) fn lock_file(name: &str) -> Option<Flock<File>> {
-    let dir = config_dir().ok()?;
-    fs::create_dir_all(&dir).ok()?;
-    let f = fs::OpenOptions::new()
+    fn warn(name: &str, stage: &str, detail: impl std::fmt::Display) {
+        eprintln!(
+            "{}",
+            format!(
+                "warning: could not acquire {name} ({stage}: {detail}); proceeding without the lock"
+            )
+            .yellow()
+        );
+    }
+
+    let dir = match config_dir() {
+        Ok(dir) => dir,
+        Err(e) => {
+            warn(name, "config dir", e);
+            return None;
+        }
+    };
+    if let Err(e) = fs::create_dir_all(&dir) {
+        warn(name, "mkdir", e);
+        return None;
+    }
+    let f = match fs::OpenOptions::new()
         .create(true)
         .write(true)
         .truncate(false)
         .open(dir.join(name))
-        .ok()?;
-    Flock::lock(f, FlockArg::LockExclusive).ok()
-}
-
-fn lock_config() -> Option<Flock<File>> {
-    lock_file("config.lock")
-}
-
-/// Wipe the workspace cache for a profile. Paired with
-/// `jwt::clear_session()` in `commands::auth::logout` — together they reset the
-/// on-disk state that login populates.
-pub fn clear_workspaces(profile: &str) -> Result<(), String> {
-    let config_path = config_path()?;
-    let _lock = lock_config();
-
-    if !config_path.exists() {
-        return Ok(());
-    }
-
-    let content =
-        fs::read_to_string(&config_path).map_err(|e| format!("error reading config file: {e}"))?;
-    let mut config_file: ConfigFile =
-        serde_yaml::from_str(&content).map_err(|e| format!("error parsing config file: {e}"))?;
-
-    if let Some(entry) = config_file.profiles.get_mut(profile) {
-        entry.workspaces.clear();
+    {
+        Ok(f) => f,
+        Err(e) => {
+            warn(name, "open", e);
+            return None;
+        }
+    };
+    match Flock::lock(f, FlockArg::LockExclusive) {
+        Ok(lock) => Some(lock),
+        // No flock support on this filesystem — degrade silently to the
+        // pre-lock behavior.
+        Err((_, Errno::ENOTSUP | Errno::ENOLCK)) => None,
+        Err((_, e)) => {
+            warn(name, "flock", e);
+            None
+        }
     }
-
-    let content = serde_yaml::to_string(&config_file)
-        .map_err(|e| format!("error serializing config: {e}"))?;
-    write_config(&config_path, &content)
 }
 
-pub fn save_workspaces(profile: &str, workspaces: Vec<WorkspaceEntry>) -> Result<(), String> {
+/// Locked read-modify-write on config.yml: takes the config lock, parses the
+/// current file, applies `f`, and writes the result back atomically. Every
+/// config mutation must go through here so none can skip the lock. When the
+/// file doesn't exist yet, `create` picks between starting from an empty
+/// config (true) and a no-op (false).
+fn update_config(create: bool, f: impl FnOnce(&mut ConfigFile)) -> Result<(), String> {
     let config_path = config_path()?;
-    let _lock = lock_config();
+    let _lock = lock_file("config.lock");
 
     let mut config_file: ConfigFile = if config_path.exists() {
         let content = fs::read_to_string(&config_path)
             .map_err(|e| format!("error reading config file: {e}"))?;
         serde_yaml::from_str(&content).map_err(|e| format!("error parsing config file: {e}"))?
-    } else {
+    } else if create {
         ConfigFile {
             profiles: HashMap::new(),
         }
+    } else {
+        return Ok(());
     };
 
-    config_file
-        .profiles
-        .entry(profile.to_string())
-        .or_default()
-        .workspaces = workspaces;
+    f(&mut config_file);
 
     let content = serde_yaml::to_string(&config_file)
         .map_err(|e| format!("error serializing config: {e}"))?;
-
     write_config(&config_path, &content)
 }
 
-pub fn save_default_workspace(profile: &str, workspace: WorkspaceEntry) -> Result<(), String> {
-    let config_path = config_path()?;
-    let _lock = lock_config();
-
-    let mut config_file: ConfigFile = if config_path.exists() {
-        let content = fs::read_to_string(&config_path)
-            .map_err(|e| format!("error reading config file: {e}"))?;
-        serde_yaml::from_str(&content).map_err(|e| format!("error parsing config file: {e}"))?
-    } else {
-        ConfigFile {
-            profiles: HashMap::new(),
+/// Wipe the workspace cache for a profile. Paired with
+/// `jwt::clear_session()` in `commands::auth::logout` — together they reset the
+/// on-disk state that login populates.
+pub fn clear_workspaces(profile: &str) -> Result<(), String> {
+    update_config(false, |config_file| {
+        if let Some(entry) = config_file.profiles.get_mut(profile) {
+            entry.workspaces.clear();
         }
-    };
+    })
+}
 
-    let entry = config_file.profiles.entry(profile.to_string()).or_default();
-    entry
-        .workspaces
-        .retain(|w| w.public_id != workspace.public_id);
-    entry.workspaces.insert(0, workspace);
+pub fn save_workspaces(profile: &str, workspaces: Vec<WorkspaceEntry>) -> Result<(), String> {
+    update_config(true, move |config_file| {
+        config_file
+            .profiles
+            .entry(profile.to_string())
+            .or_default()
+            .workspaces = workspaces;
+    })
+}
 
-    let content = serde_yaml::to_string(&config_file)
-        .map_err(|e| format!("error serializing config: {e}"))?;
-    write_config(&config_path, &content)
+pub fn save_default_workspace(profile: &str, workspace: WorkspaceEntry) -> Result<(), String> {
+    update_config(true, move |config_file| {
+        let entry = config_file.profiles.entry(profile.to_string()).or_default();
+        entry
+            .workspaces
+            .retain(|w| w.public_id != workspace.public_id);
+        entry.workspaces.insert(0, workspace);
+    })
 }
 
 pub fn save_current_database(
@@ -230,29 +237,14 @@ pub fn save_current_database(
     workspace_id: &str,
     database_id: &str,
 ) -> Result<(), String> {
-    let config_path = config_path()?;
-    let _lock = lock_config();
-
-    let mut config_file: ConfigFile = if config_path.exists() {
-        let content = fs::read_to_string(&config_path)
-            .map_err(|e| format!("error reading config file: {e}"))?;
-        serde_yaml::from_str(&content).map_err(|e| format!("error parsing config file: {e}"))?
-    } else {
-        ConfigFile {
-            profiles: HashMap::new(),
-        }
-    };
-
-    config_file
-        .profiles
-        .entry(profile.to_string())
-        .or_default()
-        .current_databases
-        .insert(workspace_id.to_string(), database_id.to_string());
-
-    let content = serde_yaml::to_string(&config_file)
-        .map_err(|e| format!("error serializing config: {e}"))?;
-    write_config(&config_path, &content)
+    update_config(true, |config_file| {
+        config_file
+            .profiles
+            .entry(profile.to_string())
+            .or_default()
+            .current_databases
+            .insert(workspace_id.to_string(), database_id.to_string());
+    })
 }
 
 pub fn load_current_database(profile: &str, workspace_id: &str) -> Option<String> {
@@ -271,25 +263,11 @@ pub fn load_current_database(profile: &str, workspace_id: &str) -> Option<String
 }
 
 pub fn clear_current_database(profile: &str, workspace_id: &str) -> Result<(), String> {
-    let config_path = config_path()?;
-    let _lock = lock_config();
-
-    if !config_path.exists() {
-        return Ok(());
-    }
-
-    let content =
-        fs::read_to_string(&config_path).map_err(|e| format!("error reading config file: {e}"))?;
-    let mut config_file: ConfigFile =
-        serde_yaml::from_str(&content).map_err(|e| format!("error parsing config file: {e}"))?;
-
-    if let Some(entry) = config_file.profiles.get_mut(profile) {
-        entry.current_databases.remove(workspace_id);
-    }
-
-    let content = serde_yaml::to_string(&config_file)
-        .map_err(|e| format!("error serializing config: {e}"))?;
-    write_config(&config_path, &content)
+    update_config(false, |config_file| {
+        if let Some(entry) = config_file.profiles.get_mut(profile) {
+            entry.current_databases.remove(workspace_id);
+        }
+    })
 }
 
 /// Global API key override set via --api-key flag.
@@ -458,6 +436,23 @@ mod tests {
         assert_eq!(staging.workspaces[0].public_id, "ws-staging");
     }
 
+    #[test]
+    fn config_file_stays_world_readable() {
+        // The atomic-write path must not silently flip config.yml from the
+        // fs::write-era 0644 to tempfile's 0600 — it holds no credentials
+        // and other tooling may read it.
+        use std::os::unix::fs::PermissionsExt;
+        let (_tmp, _guard) = with_temp_config_dir();
+        save_workspaces("default", vec![ws("ws-1", "WS")]).unwrap();
+
+        let mode = fs::metadata(config_path().unwrap())
+            .unwrap()
+            .permissions()
+            .mode()
+            & 0o777;
+        assert_eq!(mode, 0o644);
+    }
+
     #[test]
     fn concurrent_saves_keep_all_entries_and_parse_cleanly() {
         // Regression: parallel `hotdata` invocations doing read-modify-write
diff --git a/src/util.rs b/src/util.rs
index 15a9f6f..8c0a0e8 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -1,6 +1,29 @@
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::time::Duration;
 
+/// Atomically replace the file at `path` with `bytes`: write to a tempfile in
+/// the same directory, chmod it to `mode`, then rename over the destination.
+/// Concurrent readers never observe a truncated or half-written file. Note
+/// that rename replaces the destination entry itself — a symlinked
+/// destination becomes a regular file.
+pub fn atomic_write(path: &std::path::Path, bytes: &[u8], mode: u32) -> Result<(), String> {
+    use std::io::Write;
+    use std::os::unix::fs::PermissionsExt;
+
+    let parent = path.parent().ok_or("path has no parent directory")?;
+    std::fs::create_dir_all(parent).map_err(|e| format!("mkdir failed: {e}"))?;
+    let mut tmp =
+        tempfile::NamedTempFile::new_in(parent).map_err(|e| format!("open failed: {e}"))?;
+    tmp.write_all(bytes)
+        .map_err(|e| format!("write failed: {e}"))?;
+    tmp.as_file()
+        .set_permissions(std::fs::Permissions::from_mode(mode))
+        .map_err(|e| format!("chmod failed: {e}"))?;
+    tmp.persist(path)
+        .map_err(|e| format!("write failed: {e}"))?;
+    Ok(())
+}
+
 /// Create a steady-ticking spinner with a cyan glyph and the given message.
 /// Writes to stderr so stdout (json/yaml output) stays clean.
 pub fn spinner(msg: &str) -> indicatif::ProgressBar {

From d6d29f12491098da700cd0fd3ec8e64dd90aa014 Mon Sep 17 00:00:00 2001
From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
Date: Sun, 19 Jul 2026 20:22:31 -0700
Subject: [PATCH 3/3] test(auth): cover persist-failure and lock-degrade paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- ensure_returns_token_even_when_session_persist_fails: a read-only
  config dir must not fail the refresh — the caller still gets the
  token, the on-disk session stays stale, and the failed lock
  acquisition exercises the unlocked degrade.
- lock_file_degrades_to_none_when_config_dir_unavailable: pointing the
  config dir at a regular file must yield None (proceed unlocked), not
  an error or a hang.
---
 src/client/jwt.rs | 34 ++++++++++++++++++++++++++++++++++
 src/config.rs     | 19 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/src/client/jwt.rs b/src/client/jwt.rs
index 5329eb1..25cb74c 100644
--- a/src/client/jwt.rs
+++ b/src/client/jwt.rs
@@ -1187,6 +1187,40 @@ mod tests {
         );
     }
 
+    #[test]
+    fn ensure_returns_token_even_when_session_persist_fails() {
+        // A refreshed token must reach the caller even when session.json
+        // can't be written (read-only config dir): the persistence failure
+        // is warned about, not fatal. The lock acquisition also fails here
+        // (session.lock can't be created), exercising the unlocked degrade.
+        use std::os::unix::fs::PermissionsExt;
+        let (_tmp, _guard) = with_temp_config_dir();
+        let mut server = mockito::Server::new();
+        let m = server
+            .mock("POST", "/o/token/")
+            .match_body(mockito::Matcher::UrlEncoded(
+                "grant_type".into(),
+                "refresh_token".into(),
+            ))
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(r#"{"access_token":"refreshed-jwt","expires_in":300,"refresh_token":"r2"}"#)
+            .create();
+
+        save_session(&cached_session(-10, 86400)).unwrap();
+        let profile = mock_profile(&server.url());
+
+        let dir = config::config_dir().unwrap();
+        fs::set_permissions(&dir, fs::Permissions::from_mode(0o555)).unwrap();
+        let result = ensure_access_token(&profile, None);
+        fs::set_permissions(&dir, fs::Permissions::from_mode(0o755)).unwrap();
+
+        m.assert();
+        assert_eq!(result.unwrap(), "refreshed-jwt");
+        // The on-disk session couldn't be updated — still the old one.
+        assert_eq!(load_session().unwrap().access_token, "cached-jwt");
+    }
+
     #[test]
     fn ensure_concurrent_expiry_refreshes_once_and_shares_result() {
         // Regression: when the access token expired mid-burst, N concurrent
diff --git a/src/config.rs b/src/config.rs
index f179789..8b2fb36 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -436,6 +436,25 @@ mod tests {
         assert_eq!(staging.workspaces[0].public_id, "ws-staging");
     }
 
+    #[test]
+    fn lock_file_degrades_to_none_when_config_dir_unavailable() {
+        // Locking is best-effort: when the config dir can't even be
+        // created (here: the path is a regular file), lock_file must
+        // degrade to None — warn and proceed unlocked — not error or hang.
+        let (tmp, _guard) = with_temp_config_dir();
+        let not_a_dir = tmp.path().join("not-a-dir");
+        fs::write(&not_a_dir, "x").unwrap();
+        // SAFETY: serialized via with_temp_config_dir's ENV_LOCK guard.
+        unsafe {
+            std::env::set_var("HOTDATA_CONFIG_DIR", &not_a_dir);
+        }
+        let lock = lock_file("config.lock");
+        unsafe {
+            std::env::set_var("HOTDATA_CONFIG_DIR", tmp.path());
+        }
+        assert!(lock.is_none());
+    }
+
     #[test]
     fn config_file_stays_world_readable() {
         // The atomic-write path must not silently flip config.yml from the