From 648179b21baf404ba6e591522ae2a07f317d8cd8 Mon Sep 17 00:00:00 2001
From: Sephyi <me@sephy.io>
Date: Sun, 19 Apr 2026 18:50:50 +0200
Subject: [PATCH] feat(doctor): verify cloud provider connectivity (not just
 api_key presence)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`commitbee doctor` previously reported OpenAI/Anthropic as "configured"
whenever `config.api_key.is_some()`, without checking that the key was
actually valid or that the provider was reachable. Only Ollama got a
real `provider.verify().await` health check.

Extend the cloud-provider branch of `run_doctor` to:

- call `llm::create_provider(&self.config)` and then `provider.verify()`
  for the configured cloud provider,
- on success print `reachable (key '****<last-4>')` so operators can
  confirm which key is in effect without leaking the full secret,
- on error pretty-print the failure on stderr with the same `ERROR: <e>`
  style used by the Ollama fallback arm,
- preserve the existing "no key" case and report it clearly without
  attempting a network call.

Network failures remain non-fatal — `doctor` is diagnostic, so one
failed provider probe must not abort the rest of the report (git-repo
check still runs).

A small `redact_api_key` helper keeps the secret-redaction logic
UTF-8-safe (tests cover empty, short, normal, and multi-byte keys).

Closes audit entry F-029 from #3.
---
 src/app.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/app.rs b/src/app.rs
index 5ab1fd8..5f60d8c 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -8,6 +8,7 @@ use std::path::PathBuf;
 use console::style;
 use dialoguer::{Confirm, Editor, Input, Select};
 use globset::{Glob, GlobSetBuilder};
+use secrecy::ExposeSecret;
 use tokio::signal;
 use tokio::sync::mpsc;
 use tokio_util::sync::CancellationToken;
@@ -579,11 +580,34 @@ impl App {
                 }
             }
             other => {
-                eprint!("  {} API key: ", other);
-                if self.config.api_key.is_some() {
-                    eprintln!("{}", style("configured").green());
-                } else {
-                    eprintln!("{}", style("MISSING").red().bold());
+                eprint!("  {}: ", other);
+                match self.config.api_key.as_ref() {
+                    None => {
+                        eprintln!("{} (no API key configured)", style("MISSING").red().bold());
+                    }
+                    Some(key) => {
+                        let redacted = redact_api_key(key.expose_secret());
+                        // Build the provider to reach verify(). Construction itself
+                        // can fail (e.g. HTTP client build); keep that non-fatal so
+                        // the rest of `doctor` still runs.
+                        match llm::create_provider(&self.config) {
+                            Err(e) => {
+                                eprintln!("{}: {}", style("ERROR").red().bold(), e);
+                            }
+                            Ok(provider) => match provider.verify().await {
+                                Ok(()) => {
+                                    eprintln!(
+                                        "{} (key '{}')",
+                                        style("reachable").green().bold(),
+                                        redacted
+                                    );
+                                }
+                                Err(e) => {
+                                    eprintln!("{}: {}", style("ERROR").red().bold(), e);
+                                }
+                            },
+                        }
+                    }
                 }
             }
         }
@@ -1499,6 +1523,23 @@ fi
     }
 }
 
+/// Redact an API key for display, preserving only the last 4 characters.
+///
+/// Used by `doctor` to confirm which key is in effect without leaking it into
+/// logs or terminal scrollback. Keys shorter than 8 characters are fully
+/// masked since partial disclosure would be a large fraction of the secret.
+fn redact_api_key(key: &str) -> String {
+    const SUFFIX_LEN: usize = 4;
+    const MIN_LEN_FOR_SUFFIX: usize = 8;
+
+    let char_count = key.chars().count();
+    if char_count < MIN_LEN_FOR_SUFFIX {
+        return "****".to_string();
+    }
+    let tail: String = key.chars().skip(char_count - SUFFIX_LEN).collect();
+    format!("****{tail}")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1524,4 +1565,25 @@ mod tests {
         assert!(result.contains("Make it shorter"));
         assert!(result.contains("Respond with ONLY the refined JSON object."));
     }
+
+    #[test]
+    fn redact_api_key_masks_full_key_when_short() {
+        assert_eq!(redact_api_key(""), "****");
+        assert_eq!(redact_api_key("abc"), "****");
+        assert_eq!(redact_api_key("abcdefg"), "****");
+    }
+
+    #[test]
+    fn redact_api_key_exposes_only_last_four() {
+        assert_eq!(redact_api_key("sk-proj-abcdef1234"), "****1234");
+        assert_eq!(redact_api_key("abcdefgh"), "****efgh");
+    }
+
+    #[test]
+    fn redact_api_key_handles_multibyte_chars() {
+        // Ensure we don't slice through a UTF-8 codepoint boundary.
+        let key = "keyé🔑ABCD";
+        let redacted = redact_api_key(key);
+        assert_eq!(redacted, "****ABCD");
+    }
 }