security(desktop): fix API-key encryption using a real secret

CHANGELOG.md

@@ -7,6 +7,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- **Desktop API-key protection now uses a real secret.** The scheme credited as "fixed" for
+  F-05/F-06 (2026-07-29) upgraded the KDF (unsalted SHA-256 → PBKDF2 + random salt) but never
+  addressed the actual finding: its passphrase input —
+  `${appDataPath}|${provider}|WorldScriptStudio|v1` — was built entirely from public/discoverable
+  values, so anyone with read access to the encrypted `<provider>_key.enc.json` file could
+  reconstruct it and decrypt in one PBKDF2 call. API keys now reuse
+  `services/storage/storageEncryptionService.ts`'s real user-passphrase-derived key (the same one
+  protecting IDB at-rest data) whenever at-rest encryption is configured and unlocked; when no
+  passphrase is configured, keys are stored as honest plaintext rather than fake-encrypted.
+  Configured-but-locked reads/writes fail closed (no silent plaintext downgrade), matching the
+  existing IDB protected-write policy. The two obsolete pre-2026-08-13 formats (unsalted and
+  salted-but-public-passphrase) are discarded, not migrated, on next read — same "locked decision"
+  precedent as the original F-05/F-06 fix; the user is notified and re-prompted for the key.
+  `services/fs/fsCore.ts`'s derived-passphrase `encryptText`/`decryptText` were retired outright.
+  Project/settings/snapshot/Codex/RAG/binder-asset data on desktop remains plaintext — that gap
+  is tracked separately and not fixed by this entry. **Review-loop follow-up fixes to the same
+  change:** a `protected-v1` file that fails to decrypt under the *current* key (e.g. after a
+  passphrase rotation, or a transient error resolving the key) is no longer discarded — only
+  payloads positively identified as one of the two obsolete pre-2026-08-13 formats are; a rotation
+  no longer permanently destroys an otherwise-valid saved key. Encrypted payloads now bind
+  `{provider, apiKey}` together (not just the bare key string), so a ciphertext swapped between two
+  providers' files decrypts under the same key but fails the provider check, closing a cross-file
+  substitution gap. **Second round of review-loop follow-up fixes:** a key file containing valid
+  JSON that isn't an object (e.g. the literal `null`) previously threw an uncaught `TypeError` when
+  indexed instead of being treated as unreadable — now guarded explicitly. The "obsolete format,
+  discard" fallback previously deleted *any* payload that didn't match a currently-recognized
+  scheme, without checking it actually matched one of the two legacy shapes — an unexpected future
+  format (e.g. after a rollback) or a merely corrupted current-format file would have been
+  permanently destroyed instead of preserved; now only payloads positively identified as the legacy
+  `{iv, data}` shape (no `scheme` field) are discarded. `AiProviderCard` and `OpenRouterSection`
+  read API keys in a mount-only effect, so a key that read as locked (not "absent") while the
+  encrypted session was still unlocking stayed shown as missing even after the user unlocked —
+  both now re-fetch when `encryptionReady` (threaded from `SettingsViewContext`) changes.
+
 ### Fixed
 
 - **Atomic writes for desktop filesystem storage.** Every `services/fs/*Store.ts` writer

components/settings/AiProviderCard.tsx

@@ -89,6 +89,10 @@ interface AiProviderCardProps {
   // instead of requiring desktop when the user has separately configured OLLAMA_ORIGINS. Default
   // false so callers that don't pass it (e.g. older tests) keep today's desktop-only behavior.
   browserOllamaEnabled?: boolean;
+  // QNBS-v3: at-rest-encrypted API keys read as null while the session is locked (fail-closed, not
+  // "no key saved") — this must be in the key-fetch effect's deps so a mount-time-locked read gets
+  // retried once the session unlocks, instead of leaving the input permanently blank until reload.
+  encryptionReady?: boolean;
 }
 
 interface LocalDiagnosticState {
@@ -191,6 +195,7 @@ export const AiProviderCard: FC<AiProviderCardProps> = ({
   onProviderChange,
   onModelSelect,
   browserOllamaEnabled = false,
+  encryptionReady,
 }) => {
   const { t } = useTranslation();
   const provider = advancedAi.provider;
@@ -297,6 +302,7 @@ export const AiProviderCard: FC<AiProviderCardProps> = ({
   }, [provider, probeWebGpu]);
 
   useEffect(() => {
+    void encryptionReady;
     storageService
       .getApiKey('openai')
       .then((k) => setOpenaiKey(k ?? ''))
@@ -309,7 +315,7 @@ export const AiProviderCard: FC<AiProviderCardProps> = ({
       .getApiKey('anthropic')
       .then((k) => setAnthropicKey(k ?? ''))
       .catch(() => {});
-  }, []);
+  }, [encryptionReady]);
 
   // QNBS-v3: save/clear via storageService, matching every other provider's key persistence.
   const handleSaveGrokKey = useCallback(async () => {

components/settings/AiSections.tsx

@@ -49,7 +49,7 @@ const isCustomOllamaModel = (model: string) =>
   model.startsWith('ollama/') && !KNOWN_OLLAMA_MODELS.has(model);
 
 export const AiSection: FC = () => {
-  const { t, settings, handleSettingChange } = useSettingsViewContext();
+  const { t, settings, handleSettingChange, encryptionReady } = useSettingsViewContext();
   // QNBS-v3: Issue 10 — gate at the parent level so useAdaptiveAi hook + device profiling
   //          never run when the feature flag is off (saves GPU queries + IDB reads on every settings open)
   const adaptiveAiEnabled = useAppSelector(selectEnableAdaptiveAiEngine);
@@ -71,6 +71,7 @@ export const AiSection: FC = () => {
       <AiProviderCard
         advancedAi={settings.advancedAi}
         browserOllamaEnabled={browserOllamaEnabled}
+        encryptionReady={encryptionReady}
         onAdvancedAiPatch={(patch) =>
           handleSettingChange('advancedAi', { ...settings.advancedAi, ...patch })
         }

components/settings/OpenRouterSection.tsx

@@ -7,6 +7,7 @@
 import type { FC } from 'react';
 import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { useAppDispatch, useAppSelector } from '../../app/hooks';
+import { useSettingsViewContext } from '../../contexts/SettingsViewContext';
 import { settingsActions } from '../../features/settings/settingsSlice';
 import { statusActions } from '../../features/status/statusSlice';
 import { useTranslation } from '../../hooks/useTranslation';
@@ -110,6 +111,7 @@ const CircuitBreakerStatus: FC<{ t: ReturnType<typeof useTranslation>['t'] }> =
 export const OpenRouterSection: FC = () => {
   const { t } = useTranslation();
   const dispatch = useAppDispatch();
+  const { encryptionReady } = useSettingsViewContext();
   const openRouterSettings = useAppSelector((s) => s.settings.openRouter);
   const aiMode = useAppSelector((s) => s.settings.aiMode);
   const privacy = useAppSelector((s) => s.settings.privacy);
@@ -166,8 +168,11 @@ export const OpenRouterSection: FC = () => {
   // afterwards and overwrite the user's newer key state with a stale value.
   const keyOverriddenRef = useRef(false);
 
-  // Load stored key status on mount.
+  // QNBS-v3: re-runs when encryptionReady changes, not just on mount — a mount-time-locked read
+  // (fail-closed null, not "no key saved") must be retried once the session unlocks, instead of
+  // leaving the key permanently shown as missing until this component remounts/the page reloads.
   useEffect(() => {
+    void encryptionReady;
     let cancelled = false;
     storageService
       .getApiKey('openrouter')
@@ -184,7 +189,7 @@ export const OpenRouterSection: FC = () => {
     return () => {
       cancelled = true;
     };
-  }, []);
+  }, [encryptionReady]);
 
   // QNBS-v3: Monotonic guard so concurrent catalog fetches are last-wins — a slower or failing
   // response can never overwrite the result of a newer request, and a response that resolves after

docs/SECURITY-THREAT-MODEL.md

@@ -1,7 +1,7 @@
 # Security Threat Model
 
 **Version:** 1.0.0  
-**Date:** 2026-06-05 (baseline); desktop-crypto mitigation row updated 2026-07-29 (v1.24.2, F-05/F-06)  
+**Date:** 2026-06-05 (baseline); desktop-crypto mitigation row updated 2026-08-13 (F-05/F-06 superseded — see below)  
 **Status:** v1.24.2 baseline
 
 This document provides a formal STRIDE threat analysis for WorldScript Studio, mapping threats to mitigations and code locations.
@@ -39,7 +39,7 @@ This document provides a formal STRIDE threat analysis for WorldScript Studio, m
 | Threat | Mitigation | Code Location |
 |--------|------------|-------------|
 | API key leakage via logs | StructuredLogger sanitization; never log keys | `services/logger.ts:sanitizeLogContext()` |
-| Desktop API key exposure via local file-read access | AES-256-GCM with a PBKDF2-derived key (600 000 iterations, SHA-256, random 32-byte salt per encryption) — fixed 2026-07-29; the prior scheme derived the key from a single unsalted SHA-256 digest of publicly-derivable material (own file's parent path + provider name from the filename + a hardcoded literal), so anyone with read access to `config/<provider>_key.enc.json` could reconstruct the key in one hash operation (F-05/F-06). No migration path for pre-fix files by design — a legacy (unsalted) payload is discarded and the user is prompted to re-enter the key. | `services/fs/fsCore.ts:deriveFileSystemCryptoKey()`, `services/fs/settingsFsStore.ts:getApiKey()` |
+| Desktop API key exposure via local file-read access | **Fixed 2026-08-13 (supersedes F-05/F-06).** API keys are now protected by the real user at-rest-encryption passphrase (the same `CryptoKey` protecting IDB data — `services/storage/storageEncryptionService.ts`) whenever one is configured and unlocked, AEAD-encrypting `{provider, apiKey}` together so a ciphertext swapped between two providers' files fails the provider check on decrypt. **The two prior F-05/F-06 schemes (both retired, not migrated)**: (1) pre-2026-07-29, unsalted single-SHA-256 of publicly-derivable material; (2) 2026-07-29–2026-08-13, salted PBKDF2 — but of that *same* publicly-derivable material (`${appDataPath}\|${provider}\|WorldScriptStudio\|v1`), so it hardened against rainbow tables without addressing the actual finding (anyone with file-read access could reconstruct the identical string). Neither is migrated — the file is discarded and the user re-prompted for the key. **Honest residual gap**: with no at-rest passphrase configured (the default), API keys are stored as plaintext, not fake-encrypted — this is a disclosed, deliberate tradeoff, not an oversight. | `services/storage/storageEncryptionService.ts`, `services/fs/settingsFsStore.ts:getApiKey()`/`saveApiKey()` |
 | Manuscript data in IndexedDB | AES-256-GCM at-rest encryption | `services/storage/storageEncryptionService.ts` |
 | Voice audio to cloud | Web Speech API consent gate | `components/voice/VoicePrivacyConsentModal.tsx` |
 | DuckDB analytics unencrypted (SEC-6) | **Bounded by design, with one prose column now encrypted:** most persisted fields are local metadata only (titles, loglines, character names, word counts, embeddings) and **nothing leaves the device**. The one column that genuinely holds literal manuscript prose, `codex_mentions.excerpt`, is now cell-level encrypted (AES-256-GCM via `services/duckdb/duckdbEncryption.ts`, reusing the IDB at-rest encryption key) whenever `enableIdbAtRestEncryption` is active: `duckdbCodexWrite()` writes ciphertext into `excerpt_enc BLOB` and nulls the plaintext `excerpt` column; `services/duckdb/codexExcerptEncryptionMigration.ts` backfills any pre-existing plaintext rows once encryption is unlocked. Gated by `enableDuckDbAnalytics` **and** the Settings → Privacy "Analytics" opt-out (`isAnalyticsPersistenceAllowed` in `app/listenerMiddleware.ts`); turning the toggle off stops all DuckDB writes + inference telemetry. Full OPFS file-level encryption remains **infeasible** — DuckDB-WASM owns the OPFS file handle directly, so there is no app-level interception point; the other metadata columns stay intentionally plaintext (bounded-exposure design). | `app/listenerMiddleware.ts:isAnalyticsPersistenceAllowed`, `services/duckdb/duckdbAnalytics.ts:duckdbCodexWrite()`, `services/duckdb/duckdbEncryption.ts`, `services/duckdb/codexExcerptEncryptionMigration.ts` |
@@ -124,9 +124,12 @@ Goal: Intercept/decrypt collaboration traffic
 ```
 Goal: Recover a user's cloud-provider API key from the Tauri desktop install
 ├─ OR: Read config/<provider>_key.enc.json directly (local process / malware with user-level FS access)
-│  └─ Mitigation: AES-256-GCM with PBKDF2-derived key (600k iter, random 32-byte salt per file) —
-│     reading the ciphertext no longer reveals the key material; the pre-2026-07-29 scheme derived
-│     the key from data an attacker with file-read access already had (F-05/F-06, fixed)
+│  └─ Mitigation (when at-rest encryption is configured+unlocked): AES-256-GCM under the real,
+│     user-chosen passphrase-derived key — not derivable from file contents or path. When no
+│     passphrase is configured (default), the file is honestly plaintext, not fake-encrypted — a
+│     disclosed tradeoff, not a bypass of this mitigation. Both pre-2026-08-13 schemes (F-05/F-06)
+│     derived their key from data an attacker with file-read access already had; retired, not
+│     migrated (superseded 2026-08-13)
 ├─ OR: Read the IDB-at-rest passphrase sentinel (enableIdbAtRestEncryption)
 │  └─ Mitigation: same PBKDF2 + non-extractable-key pattern; session-scoped in-memory key, never
 │     persisted to disk (`services/storage/storageEncryptionService.ts`)

services/fs/fsCore.ts

@@ -168,13 +168,10 @@ export function decompressData<T>(raw: string): T {
   return JSON.parse(raw) as T;
 }
 
-// --- Crypto helpers ---
-// QNBS-v3: PBKDF2-SHA-256 (600k iter, OWASP 2024 minimum) + random 32-byte salt, mirroring storageEncryptionService.ts#deriveKey, replacing a prior unsalted-SHA-256-of-public-material scheme (F-05/F-06); legacy (no `salt` field) payloads are treated as unreadable, not migrated — see decryptText below.
+// --- Base64 codec helpers ---
+// QNBS-v3: previously private to a now-removed derived-passphrase crypto scheme (F-05/F-06 fix, then found still-insecure — see settingsFsStore.ts header comment); API-key protection now reuses storageEncryptionService.ts's real passphrase-derived key directly, and these two helpers stay, exported, as the JSON-safe codec for its Uint8Array ciphertext.
 
-const PBKDF2_ITERATIONS = 600_000; // OWASP 2024 minimum for PBKDF2-HMAC-SHA-256
-const SALT_BYTE_LENGTH = 32;
-
-function bytesToBase64(bytes: Uint8Array): string {
+export function bytesToBase64(bytes: Uint8Array): string {
   let bin = '';
   for (let i = 0; i < bytes.byteLength; i++) {
     bin += String.fromCharCode(bytes[i]!);
@@ -183,7 +180,7 @@ function bytesToBase64(bytes: Uint8Array): string {
 }
 
 // QNBS-v3: explicit Uint8Array<ArrayBuffer> return type — a bare `Uint8Array` annotation widens to `Uint8Array<ArrayBufferLike>` (includes SharedArrayBuffer), rejected by crypto.subtle as a BufferSource; same pattern as libraryBackupService.ts#copyToFixedBuffer.
-function base64ToBytes(b64: string): Uint8Array<ArrayBuffer> {
+export function base64ToBytes(b64: string): Uint8Array<ArrayBuffer> {
   const bin = atob(b64);
   const out = new Uint8Array(bin.length);
   for (let i = 0; i < bin.length; i++) {
@@ -192,66 +189,6 @@ function base64ToBytes(b64: string): Uint8Array<ArrayBuffer> {
   return out;
 }
 
-async function deriveFileSystemCryptoKey(
-  secretMaterial: string,
-  salt: Uint8Array,
-): Promise<CryptoKey> {
-  const encoder = new TextEncoder();
-  const keyMaterial = await crypto.subtle.importKey(
-    'raw',
-    encoder.encode(secretMaterial),
-    { name: 'PBKDF2' },
-    false,
-    ['deriveBits', 'deriveKey'],
-  );
-  return crypto.subtle.deriveKey(
-    { name: 'PBKDF2', salt: new Uint8Array(salt), iterations: PBKDF2_ITERATIONS, hash: 'SHA-256' },
-    keyMaterial,
-    { name: 'AES-GCM', length: 256 },
-    // QNBS-v3: extractable: false — key cannot leave the WebCrypto context.
-    false,
-    ['encrypt', 'decrypt'],
-  );
-}
-
-export interface EncryptedFsPayload {
-  iv: string;
-  salt: string;
-  data: string;
-}
-
-export async function encryptText(
-  value: string,
-  secretMaterial: string,
-): Promise<EncryptedFsPayload> {
-  const salt = crypto.getRandomValues(new Uint8Array(SALT_BYTE_LENGTH));
-  const key = await deriveFileSystemCryptoKey(secretMaterial, salt);
-  const iv = crypto.getRandomValues(new Uint8Array(12));
-  const encoded = new TextEncoder().encode(value);
-  const encrypted = await crypto.subtle.encrypt({ name: 'AES-GCM', iv }, key, encoded);
-  return {
-    iv: bytesToBase64(iv),
-    salt: bytesToBase64(salt),
-    data: bytesToBase64(new Uint8Array(encrypted)),
-  };
-}
-
-export async function decryptText(
-  payload: { iv: string; salt?: string; data: string },
-  secretMaterial: string,
-): Promise<string> {
-  if (!payload.salt) {
-    // QNBS-v3: pre-2026-07-29 payloads have no salt field (unsalted single-SHA-256 scheme, F-05) — not migrated by design (locked decision); the caller treats this as "no key available".
-    throw new Error('Legacy unsalted key payload is no longer supported; re-enter the API key.');
-  }
-  const salt = base64ToBytes(payload.salt);
-  const key = await deriveFileSystemCryptoKey(secretMaterial, salt);
-  const iv = base64ToBytes(payload.iv);
-  const encrypted = base64ToBytes(payload.data);
-  const decrypted = await crypto.subtle.decrypt({ name: 'AES-GCM', iv }, key, encrypted);
-  return new TextDecoder().decode(decrypted);
-}
-
 // --- Path sanitization helpers ---
 
 const stripControlChars = (value: string): string => {