Jake
592b894790
Captures the 12 items from docs/code-review-2026-04-22.md that must land before v0.1 ships. One markdown file per issue with: severity, path:line, problem description, acceptance criteria, fix scope, and dependency graph. Split by severity: - 3 CRITICAL: live-session race, migration atomicity, transcript- profile FK - 9 MAJOR: monolith refactor, channel-fatality, capture worker join, runtime capabilities, macOS App Nap, decoder error prop, LLM prompt preflight, keystore thread-safety, hotkey device filter README.md indexes them with a fix-order dependency graph and a fish-shell script for bulk-converting to GitHub issues once `gh` CLI is installed and authed. Deferred step by user decision — markdown tracker is authoritative until then.
30 lines
1.5 KiB
Markdown
30 lines
1.5 KiB
Markdown
# 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
|
|
|
|
## 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).
|