Skip to content

Commit 6a50268

Browse files
committed
feat(credentials): default to age-encrypted file backend, keychain opt-in (eliminates macOS prompt)
1 parent 1fe3d81 commit 6a50268

10 files changed

Lines changed: 1658 additions & 196 deletions

File tree

Cargo.lock

Lines changed: 519 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ colored = "2"
5959
# Secure storage
6060
keyring = "2"
6161
secrecy = { version = "0.10", features = ["serde"] }
62+
# age-encryption.org v1 (X25519 + ChaCha20-Poly1305). Canonical Rust
63+
# implementation of the age spec, actively maintained by the age project.
64+
# Backs the DEFAULT local encrypted-file credential backend so CLX never
65+
# touches the macOS keychain unless the user explicitly opts in.
66+
age = "0.11"
6267

6368
# Traits and utilities
6469
trait-variant = "0.1"

crates/clx-core/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ chrono.workspace = true
2121
reqwest.workspace = true
2222
keyring.workspace = true
2323
secrecy.workspace = true
24+
# Default local encrypted-file credential backend (age-encryption.org v1).
25+
# Never prompts, headless-safe, cross-platform identical. The macOS keychain
26+
# is opt-in only; see credentials/backend.rs.
27+
age.workspace = true
2428
sqlite-vec.workspace = true
2529
trait-variant.workspace = true
2630
url.workspace = true

crates/clx-core/src/config/mod.rs

Lines changed: 114 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,76 @@ pub struct Config {
255255
/// Opt-in. Default values preserve 0.7.x behavior (no auto-summary fires).
256256
#[serde(default)]
257257
pub memory: MemoryConfig,
258+
259+
/// Credential storage backend selection.
260+
///
261+
/// Default is the local age-encrypted file (`backend: file`), which
262+
/// NEVER touches the macOS keychain and never prompts. Set
263+
/// `backend: keychain` (or `CLX_CREDENTIALS_BACKEND=keychain`) to opt
264+
/// into the system keychain.
265+
#[serde(default)]
266+
pub credentials: CredentialsConfig,
267+
}
268+
269+
/// Credential storage backend configuration.
270+
///
271+
/// ```yaml
272+
/// credentials:
273+
/// backend: file # default; local age-encrypted file, never prompts
274+
/// # backend: keychain # opt-in; macOS Keychain (may prompt on adhoc binaries)
275+
/// ```
276+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
277+
pub struct CredentialsConfig {
278+
/// `file` (default) or `keychain`.
279+
#[serde(default)]
280+
pub backend: crate::credentials::CredentialBackendKind,
281+
}
282+
283+
impl Default for CredentialsConfig {
284+
fn default() -> Self {
285+
Self {
286+
// The default MUST be the file backend so a fresh user never sees
287+
// a single macOS keychain prompt.
288+
backend: crate::credentials::CredentialBackendKind::File,
289+
}
290+
}
291+
}
292+
293+
impl Config {
294+
/// Resolve the effective credential backend, honoring the
295+
/// `CLX_CREDENTIALS_BACKEND` env override (highest precedence) over the
296+
/// `credentials.backend` config value (default `file`).
297+
///
298+
/// An unknown env value is a hard error so a typo can never silently
299+
/// fall back to the prompting keychain.
300+
pub fn credential_backend_kind(
301+
&self,
302+
) -> crate::Result<crate::credentials::CredentialBackendKind> {
303+
if let Ok(v) = std::env::var("CLX_CREDENTIALS_BACKEND")
304+
&& !v.trim().is_empty()
305+
{
306+
return crate::credentials::CredentialBackendKind::parse(&v)
307+
.map_err(|e| crate::Error::Config(e.to_string()));
308+
}
309+
Ok(self.credentials.backend)
310+
}
311+
312+
/// Build a `CredentialStore` from this config (single config-aware
313+
/// constructor). Every production callsite uses this so the user's
314+
/// backend selection (default `file`) is honored uniformly.
315+
pub fn credential_store(&self) -> crate::Result<crate::credentials::CredentialStore> {
316+
Ok(crate::credentials::CredentialStore::from_config(
317+
self.credential_backend_kind()?,
318+
))
319+
}
320+
321+
/// Same as [`Config::credential_store`] but with the process-scoped read
322+
/// cache enabled (long-lived MCP server).
323+
pub fn credential_store_cached(&self) -> crate::Result<crate::credentials::CredentialStore> {
324+
Ok(crate::credentials::CredentialStore::from_config_cached(
325+
self.credential_backend_kind()?,
326+
))
327+
}
258328
}
259329

260330
/// Retention policy for storage tables.
@@ -1669,8 +1739,11 @@ impl Config {
16691739
Ok(crate::llm::LlmClient::Ollama(backend))
16701740
}
16711741
ProviderConfig::AzureOpenai(c) => {
1672-
let secret =
1673-
resolve_azure_credential(name, c).map_err(LlmConfigError::ProviderInit)?;
1742+
let kind = self
1743+
.credential_backend_kind()
1744+
.map_err(|e| LlmConfigError::ProviderInit(e.to_string()))?;
1745+
let secret = resolve_azure_credential(name, c, kind)
1746+
.map_err(LlmConfigError::ProviderInit)?;
16741747
let backend = crate::llm::AzureOpenAIBackend::new(c, secret)
16751748
.map_err(|e| LlmConfigError::ProviderInit(e.to_string()))?;
16761749
Ok(crate::llm::LlmClient::Azure(backend))
@@ -1700,18 +1773,20 @@ pub enum LlmConfigError {
17001773

17011774
/// Resolve an Azure provider's API key.
17021775
///
1703-
/// Resolution order:
1704-
/// 1. Env var named in `cfg.api_key_env` (if set and non-empty).
1705-
/// 2. `CredentialStore` entry keyed `"<provider_name>-api-key"`.
1706-
/// (Hyphen, not colon, because `CredentialStore` rejects colons in user
1707-
/// keys. Earlier 0.6.0 used a colon, which made the entry impossible to
1708-
/// write via `clx credentials set`. See RUSTSEC-style note: contract
1709-
/// mismatch fixed in 0.6.1.)
1710-
/// 3. File at `cfg.api_key_file` (Unix: must be mode 0600).
1711-
/// 4. Error.
1776+
/// Resolution order (the critical correctness requirement):
1777+
/// 1. Env var named in `cfg.api_key_env` (if set and non-empty) -> 0 prompts.
1778+
/// 2. The selected `CredentialBackend` entry keyed `"<provider_name>-api-key"`.
1779+
/// With the DEFAULT backend (`file`) this is the local age-encrypted file
1780+
/// and NEVER touches the keychain / prompts. The keychain is consulted
1781+
/// here ONLY if the user explicitly set `credentials.backend: keychain`.
1782+
/// (Hyphen, not colon: `CredentialStore` rejects colons in user keys.)
1783+
/// 3. File at `cfg.api_key_file` (Unix: must be mode 0600) -> 0 prompts.
1784+
/// 4. Error (with an actionable, one-time message). NEVER a keychain
1785+
/// fallback under the default backend -- that was the entire bug.
17121786
fn resolve_azure_credential(
17131787
provider_name: &str,
17141788
cfg: &AzureOpenAIConfig,
1789+
backend_kind: crate::credentials::CredentialBackendKind,
17151790
) -> Result<secrecy::SecretString, String> {
17161791
use secrecy::SecretString;
17171792

@@ -1723,18 +1798,21 @@ fn resolve_azure_credential(
17231798
return Ok(SecretString::new(v.into()));
17241799
}
17251800

1726-
// 2. CredentialStore (system keychain)
1727-
let store = crate::credentials::CredentialStore::new();
1801+
// 2. selected backend (file by default; keychain ONLY if opted in)
1802+
let store = crate::credentials::CredentialStore::from_config(backend_kind);
17281803
let key = format!("{provider_name}-api-key");
17291804
match store.get(&key) {
17301805
Ok(Some(v)) => return Ok(SecretString::new(v.into())),
1731-
Ok(None) => {} // fall through
1806+
Ok(None) => {} // fall through to api_key_file (NOT to the keychain)
17321807
Err(e) => {
1733-
// headless Linux without D-Bus, etc. Log and fall through.
1808+
// File backend IO error / headless keychain unavailable. Log and
1809+
// fall through to api_key_file. We never silently retry a
1810+
// different store (reintroducing prompts).
17341811
tracing::warn!(
17351812
provider = %provider_name,
1813+
backend = %backend_kind_label(backend_kind),
17361814
error = %e,
1737-
"keychain unavailable, falling back to file credential"
1815+
"credential backend unavailable, falling back to api_key_file"
17381816
);
17391817
}
17401818
}
@@ -1746,11 +1824,21 @@ fn resolve_azure_credential(
17461824

17471825
Err(format!(
17481826
"no credentials available for provider '{provider_name}' \
1749-
(checked env var, keychain key '{key}', and api_key_file). \
1750-
Run: clx credentials set {provider_name}-api-key '<your-key>'"
1827+
(checked env var, {} backend key '{key}', and api_key_file). \
1828+
Run: clx credentials set {provider_name}-api-key '<your-key>' \
1829+
(or `clx credentials migrate` if the secret is only in the old \
1830+
macOS keychain).",
1831+
backend_kind_label(backend_kind)
17511832
))
17521833
}
17531834

1835+
fn backend_kind_label(kind: crate::credentials::CredentialBackendKind) -> &'static str {
1836+
match kind {
1837+
crate::credentials::CredentialBackendKind::File => "file",
1838+
crate::credentials::CredentialBackendKind::Keychain => "keychain",
1839+
}
1840+
}
1841+
17541842
#[cfg(unix)]
17551843
fn read_file_credential(path: &std::path::Path) -> Result<String, String> {
17561844
use std::os::unix::fs::PermissionsExt;
@@ -3271,8 +3359,14 @@ ollama:
32713359
eprintln!("skipping: keychain access hangs on GitHub Actions macOS runners");
32723360
return;
32733361
}
3274-
use crate::credentials::{CredentialError, CredentialStore};
3275-
let store = CredentialStore::with_service("clx-test-keyfmt");
3362+
use std::sync::Arc;
3363+
3364+
use crate::credentials::{AgeFileBackend, CredentialError, CredentialStore};
3365+
// Use the default (file) backend on a tempdir: deterministic and
3366+
// headless-safe, never touches the keychain.
3367+
let tmp = tempfile::tempdir().unwrap();
3368+
let store =
3369+
CredentialStore::with_backend(Arc::new(AgeFileBackend::with_dir(tmp.path()).unwrap()));
32763370
let provider = "azure-regression-test-keyfmt";
32773371

32783372
// 1. Hyphen-format key MUST NOT be rejected by the validator.

0 commit comments

Comments
 (0)