Merge pull request #16 from penso/secrecy

Wrap secrets in secrecy::Secret<String> to prevent leaks

Author: penso

CLAUDE.md

@@ -150,6 +150,42 @@ The `auth` subcommand (`crates/cli/src/auth_commands.rs`) provides:
 - `moltis auth reset-identity` — clear identity and user profile (triggers
   onboarding on next load)
 
+## Sensitive Data Handling
+
+Never use plain `String` for passwords, API keys, tokens, or any secret
+material. Use `secrecy::Secret<String>` instead — it redacts `Debug` output,
+prevents accidental `Display`, and zeroes memory on drop.
+
+```rust
+use secrecy::{ExposeSecret, Secret};
+
+// Store secrets wrapped
+struct Config {
+    api_key: Secret<String>,
+}
+
+// Construct: wrap at the boundary
+let cfg = Config { api_key: Secret::new(raw_key) };
+
+// Use: expose only at the point of consumption
+req.header("Authorization", format!("Bearer {}", cfg.api_key.expose_secret()));
+```
+
+Rules:
+- **Struct fields** holding secrets must be `Secret<String>` (or
+  `Option<Secret<String>>`).
+- **Function parameters** can stay `&str`; call `.expose_secret()` at the call
+  site.
+- **Serde deserialize** works automatically (secrecy's `serde` feature).
+- **Serde serialize** requires a custom helper when round-tripping is needed
+  (config files, token storage). See `serialize_secret` /
+  `serialize_option_secret` in `crates/oauth/src/types.rs`.
+- **Debug impls**: replace `#[derive(Debug)]` with a manual impl that prints
+  `[REDACTED]` for secret fields.
+- **RwLock guards**: when a `RwLock<Option<Secret<String>>>` read guard is
+  followed by a write in the same function, scope the read guard in a block
+  `{ let guard = lock.read().await; ... }` to avoid deadlocks.
+
 ## Provider Implementation Guidelines
 
 ### Async all the way down

Cargo.lock

@@ -71,6 +71,7 @@ fd-lock      = "4"
 genai        = "0.5"
 open         = "5.3"
 rand         = "0.8"
+secrecy      = { features = ["serde"], version = "0.8" }
 sha2         = "0.10"
 teloxide     = { features = ["macros"], version = "0.13" }
 tokio-util   = "0.7"

Cargo.toml

@@ -21,6 +21,7 @@ moltis-config   = { workspace = true }
 moltis-sessions = { workspace = true }
 moltis-skills   = { workspace = true }
 reqwest         = { workspace = true }
+secrecy         = { workspace = true }
 serde           = { workspace = true }
 serde_json      = { workspace = true }
 tokio           = { workspace = true }

crates/agents/Cargo.toml

@@ -1,14 +1,16 @@
 /// OAuth + API key credential management with token refresh, stored per-agent.
+use secrecy::Secret;
+
 pub struct AuthProfile {
     pub provider: String,
     pub credentials: Credentials,
 }
 
 pub enum Credentials {
-    ApiKey(String),
+    ApiKey(Secret<String>),
     OAuth {
-        access_token: String,
-        refresh_token: Option<String>,
+        access_token: Secret<String>,
+        refresh_token: Option<Secret<String>>,
         expires_at: Option<u64>,
     },
 }

crates/agents/src/auth_profiles.rs

@@ -1,13 +1,13 @@
 use std::pin::Pin;
 
-use {async_trait::async_trait, futures::StreamExt, tokio_stream::Stream};
+use {async_trait::async_trait, futures::StreamExt, secrecy::ExposeSecret, tokio_stream::Stream};
 
 use tracing::{debug, trace, warn};
 
 use crate::model::{CompletionResponse, LlmProvider, StreamEvent, ToolCall, Usage};
 
 pub struct AnthropicProvider {
-    api_key: String,
+    api_key: secrecy::Secret<String>,
     model: String,
     base_url: String,
     client: reqwest::Client,
@@ -16,7 +16,7 @@ pub struct AnthropicProvider {
 impl AnthropicProvider {
     pub fn new(api_key: String, model: String, base_url: String) -> Self {
         Self {
-            api_key,
+            api_key: secrecy::Secret::new(api_key),
             model,
             base_url,
             client: reqwest::Client::new(),
@@ -161,7 +161,7 @@ impl LlmProvider for AnthropicProvider {
         let http_resp = self
             .client
             .post(format!("{}/v1/messages", self.base_url))
-            .header("x-api-key", &self.api_key)
+            .header("x-api-key", self.api_key.expose_secret())
             .header("anthropic-version", "2023-06-01")
             .header("content-type", "application/json")
             .json(&body)
@@ -221,7 +221,7 @@ impl LlmProvider for AnthropicProvider {
             let resp = match self
                 .client
                 .post(format!("{}/v1/messages", self.base_url))
-                .header("x-api-key", &self.api_key)
+                .header("x-api-key", self.api_key.expose_secret())
                 .header("anthropic-version", "2023-06-01")
                 .header("content-type", "application/json")
                 .json(&body)

crates/agents/src/providers/anthropic.rs

@@ -12,6 +12,7 @@ use {
     async_trait::async_trait,
     futures::StreamExt,
     moltis_oauth::{OAuthTokens, TokenStore},
+    secrecy::{ExposeSecret, Secret},
     tokio_stream::Stream,
     tracing::{debug, trace, warn},
 };
@@ -149,15 +150,18 @@ impl GitHubCopilotProvider {
                 .unwrap()
                 .as_secs();
             if now + 60 < expires_at {
-                return Ok(copilot_tokens.access_token);
+                return Ok(copilot_tokens.access_token.expose_secret().clone());
             }
         }
 
         // Exchange GitHub token for Copilot API token
         let resp = self
             .client
             .get(COPILOT_TOKEN_URL)
-            .header("Authorization", format!("token {}", tokens.access_token))
+            .header(
+                "Authorization",
+                format!("token {}", tokens.access_token.expose_secret()),
+            )
             .header("Accept", "application/json")
             .header(
                 "User-Agent",
@@ -175,7 +179,7 @@ impl GitHubCopilotProvider {
 
         // Cache the Copilot API token
         let _ = self.token_store.save("github-copilot-api", &OAuthTokens {
-            access_token: copilot_resp.token.clone(),
+            access_token: Secret::new(copilot_resp.token.clone()),
             refresh_token: None,
             expires_at: Some(copilot_resp.expires_at),
         });

crates/agents/src/providers/github_copilot.rs

@@ -9,6 +9,7 @@ use {
     async_trait::async_trait,
     futures::StreamExt,
     moltis_oauth::{OAuthTokens, TokenStore, kimi_headers},
+    secrecy::{ExposeSecret, Secret},
     tokio_stream::Stream,
     tracing::{debug, trace, warn},
 };
@@ -59,17 +60,18 @@ impl KimiCodeProvider {
             if now + REFRESH_THRESHOLD_SECS >= expires_at {
                 if let Some(ref refresh_token) = tokens.refresh_token {
                     debug!("refreshing kimi-code token");
-                    let new_tokens = refresh_access_token(&self.client, refresh_token).await?;
+                    let new_tokens =
+                        refresh_access_token(&self.client, refresh_token.expose_secret()).await?;
                     self.token_store.save(PROVIDER_NAME, &new_tokens)?;
-                    return Ok(new_tokens.access_token);
+                    return Ok(new_tokens.access_token.expose_secret().clone());
                 }
                 return Err(anyhow::anyhow!(
                     "kimi-code token expired and no refresh token available"
                 ));
             }
         }
 
-        Ok(tokens.access_token)
+        Ok(tokens.access_token.expose_secret().clone())
     }
 }
 
@@ -113,8 +115,8 @@ pub async fn refresh_access_token(
     });
 
     Ok(OAuthTokens {
-        access_token: body.access_token,
-        refresh_token: body.refresh_token,
+        access_token: Secret::new(body.access_token),
+        refresh_token: body.refresh_token.map(Secret::new),
         expires_at,
     })
 }