Lumotia/docs/issues/keystore-thread-safety.md

RB-11 MAJOR: keystore::store_api_key is a thread-unsafe safe API

Severity: MAJOR Path: crates/cloud-providers/src/keystore.rs:6-18 Source: 2026-04-22 code review Labels: release-blocker, major, unsafe-api, cloud Status: RESOLVED (2026-04-22)

Resolution

Chose acceptance option 2. The environment-mutation stub is gone; store_api_key now writes into a process-global OnceLock<Mutex<HashMap<String, String>>>, so the safe signature matches the actual safety properties.

Additional details:

• Stored keys now live in-memory only for the life of the process.
• retrieve_api_key checks the in-memory keystore first, then falls back to read-only KON_API_KEY_<PROVIDER> environment variables so externally injected secrets still work.
• Module docs now describe the real tradeoff clearly: safe from any thread, but non-persistent until a proper OS keychain backend lands.

Regression tests:

• stored_key_is_retrievable_without_env_mutation
• providers_do_not_overlap

Problem

store_api_key is declared as a safe pub fn. Its implementation relies on std::env::set_var, which is documented as Undefined Behaviour outside single-threaded initialisation. The file's module comment acknowledges the precondition but the function signature does not enforce it — any caller can invoke it from any thread, and the compiler won't object.

Acceptance

Choose one:

1. Use an OS keychain backend (e.g. keyring crate) so there is no set_var involvement. Preferred — actually secret-safe, cross-platform.
2. Use a process-global OnceLock or Mutex<HashMap> inside the module instead of set_var. Removes the UB, trades persistence.
3. Mark store_api_key as unsafe and document the "call once before threads spawn" contract at the signature level. Ugly but honest.

Whichever path, update the signature and doc comments to match the safety properties actually provided.

Fix scope

Medium. Option 1 is the right long-term answer but adds a dep and platform-specific auth prompts (macOS Keychain asks the user on first access). Option 2 is fastest. Option 3 is cosmetic.

Dependencies

• None — standalone fix.
• Coupled with future BYO LLM endpoint work (storing API keys safely is a prerequisite).