# 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](../code-review-2026-04-22.md)
**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 `MAGNOTIA_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).