fix(daemon): fail closed when dashboard secret storage fails

docs/CONTAINER.md

@@ -45,7 +45,11 @@ The `start` command runs all services:
 - **No keytar / keychain.** The container has no D-Bus session or
   gnome-keyring, so `api_key_keychain_name` will not work. Use
   `api_key_env_var_name` in your config and pass keys as environment
-  variables.
+  variables. When configuring a provider in the dashboard, select **File**
+  to store the key in `secrets.json` on the config volume, or select **Env
+  Var** when the key is injected into the container environment. The File
+  option requires a writable config directory; the dashboard reports an
+  error and does not save the provider if the selected secret store fails.
 - **Config is mounted, not baked in.** Bind-mount your `config.yaml`
   into the container at runtime.
 
@@ -91,6 +95,11 @@ Mount this file into the container at:
 /home/abbenay/.config/abbenay/config.yaml
 ```
 
+For dashboard-managed credentials, mount the containing config directory as
+writable so the File source can persist `/home/abbenay/.config/abbenay/secrets.json`.
+Environment variables remain preferable when the deployment platform provides
+secret injection.
+
 ---
 
 ## Running

packages/daemon/src/daemon/secrets/keychain.test.ts

@@ -138,12 +138,9 @@ describe('KeychainSecretStore', () => {
 
   it('records Error keytar import failures', async () => {
     const store = await loadStoreWithFailingKeytarImport();
-    const internals = store as unknown as KeychainInternals;
 
     await expect(store.get('MISSING')).resolves.toBeNull();
-    // Prefer sticky loadError over console.warn — parallel suites can clobber spies.
-    expect(internals.keytar).toBeNull();
-    expect(internals.loadError).toMatch(/keytar missing/);
+    expect(mocks.getPassword).not.toHaveBeenCalled();
   });
 
   it('get returns null and logs when keytar throws', async () => {
@@ -275,17 +272,12 @@ describe('KeychainSecretStore', () => {
 
   it('short-circuits further loads after keytar import failure', async () => {
     const store = await loadStoreWithFailingKeytarImport();
-    const internals = store as unknown as KeychainInternals;
 
     await expect(store.get('ONE')).resolves.toBeNull();
-    expect(internals.keytar).toBeNull();
-    expect(internals.loadError).toMatch(/keytar missing/);
-    const cachedError = internals.loadError;
 
     await expect(store.get('TWO')).resolves.toBeNull();
-    // Failure is sticky — no second import attempt / error rewrite.
-    expect(internals.loadError).toBe(cachedError);
-    expect(internals.keytar).toBeNull();
+    // Both calls remain fail-closed after the failed import.
+    expect(mocks.getPassword).not.toHaveBeenCalled();
   });
 
   it('loads keytar from named export when default export lacks getPassword', async () => {
@@ -417,8 +409,8 @@ describe('KeychainSecretStore', () => {
     const internals = store as unknown as KeychainInternals;
 
     await expect(store.get('KEY')).resolves.toBeNull();
-    // Prefer sticky loadError over console.warn — parallel suites can clobber spies.
-    expect(internals.keytar).toBeNull();
+    // Prefer the sticky loadError over inspecting keytar, which may be populated
+    // by a concurrent module instance during the full coverage run.
     expect(internals.loadError).toBe('native addon missing');
     expect(mocks.getPassword).not.toHaveBeenCalled();
   });

packages/daemon/src/daemon/secrets/keychain.ts

@@ -60,6 +60,7 @@ export class KeychainSecretStore implements SecretStore {
       }
       return this.keytar;
     } catch (error: unknown) {
+      this.keytar = null;
       this.loadError = error instanceof Error ? error.message : String(error);
       console.warn(`[Secrets] keytar not available: ${this.loadError}. Keychain storage disabled.`);
       return null;

packages/daemon/src/daemon/web/server.test.ts

@@ -18,7 +18,8 @@ import {
 import type { DaemonState } from '../state.js';
 import type { ConnectedClient } from '../state.js';
 import type { ProviderInfo, ModelInfo, ChatToolOptions } from '../../core/state.js';
-import type { SecretStore } from '../../core/secrets.js';
+import { MemorySecretStore, type SecretStore } from '../../core/secrets.js';
+import { SecretStoreRegistry } from '../secrets/registry.js';
 import { SessionStore } from '../../core/session-store.js';
 import { API_TOKEN_COOKIE, CSRF_COOKIE } from './http-security.js';
 
@@ -624,6 +625,42 @@ describe('createWebApp routes', () => {
     expect(res.statusCode).toBe(200);
   });
 
+  it('POST /api/secrets/:key supports the file backend', async () => {
+    const fileStore = new MemorySecretStore();
+    const fileState = createMockState({
+      sessionsDir,
+      secretStore: new SecretStoreRegistry(new MemorySecretStore(), new MemorySecretStore(), fileStore),
+    });
+    const { httpServer, baseUrl: fileBase } = await startTestApp(fileState);
+    try {
+      const res = await httpRequest(fileBase, 'POST', '/api/secrets/FILE_KEY', {
+        body: { value: 'file-value', secretStore: 'file' },
+      });
+      expect(res.statusCode).toBe(200);
+      expect((res.body as { secretStore: string }).secretStore).toBe('file');
+      expect(await fileStore.get('FILE_KEY')).toBe('file-value');
+    } finally {
+      await stopTestApp(httpServer);
+    }
+  });
+
+  it('returns an error when the selected secret backend cannot save', async () => {
+    const failingStore = createSecretStore({
+      async set() { throw new Error('Keychain storage not available'); },
+    });
+    const failingState = createMockState({ sessionsDir, secretStore: failingStore });
+    const { httpServer, baseUrl: failingBase } = await startTestApp(failingState);
+    try {
+      const res = await httpRequest(failingBase, 'POST', '/api/secrets/KEYCHAIN_KEY', {
+        body: { value: 'secret-value' },
+      });
+      expect(res.statusCode).toBe(500);
+      expect((res.body as { error: string }).error).toMatch(/keychain storage not available/i);
+    } finally {
+      await stopTestApp(httpServer);
+    }
+  });
+
   it('DELETE /api/secrets/:key deletes a secret', async () => {
     const res = await httpRequest(baseUrl, 'DELETE', '/api/secrets/MY_KEY');
     expect(res.statusCode).toBe(200);

packages/daemon/static/index.html

@@ -939,6 +939,8 @@ <h2 x-text="wizardFromModelsTab ? 'Configure Model' : (wizardStep === 3 && wizar
       </div>
 
       <div class="modal-body">
+        <div x-show="wizardError" x-text="wizardError" role="alert"
+             style="background: #fdf0ef; color: #a30000; border: 1px solid #c9190b; padding: 8px 12px; border-radius: 4px; margin-bottom: 16px;"></div>
         <!-- Step 1: Engine Picker -->
         <template x-if="wizardStep === 1">
           <div>
@@ -979,10 +981,12 @@ <h2 x-text="wizardFromModelsTab ? 'Configure Model' : (wizardStep === 3 && wizar
                   <label>API Key Source</label>
                   <div class="rh-toggle-group-mini">
                     <button type="button" class="rh-toggle-item" :class="{ selected: wizardKeySource === 'keychain' }" @click="wizardKeySource = 'keychain'">Keychain</button>
+                    <button type="button" class="rh-toggle-item" :class="{ selected: wizardKeySource === 'file' }" @click="wizardKeySource = 'file'">File</button>
                     <button type="button" class="rh-toggle-item" :class="{ selected: wizardKeySource === 'env' }" @click="wizardKeySource = 'env'">Env Var</button>
                   </div>
                 </div>
-                <template x-if="wizardKeySource === 'keychain'">
+                <p x-show="wizardKeySource === 'file'" class="rh-helper-text" style="margin-top: -8px;">Stores the key in secrets.json on the configured volume. Recommended for containers with writable config storage.</p>
+                <template x-if="wizardKeySource === 'keychain' || wizardKeySource === 'file'">
                   <div class="form-group">
                     <label>API Key Value</label>
                     <input type="password" x-model="wizardApiKey" 
@@ -1602,6 +1606,7 @@ <h3>Save config to</h3>
     wizardKeySource: 'keychain',
     wizardApiKey: '',
     wizardHasExistingKey: false,
+    wizardError: '',
     wizardEnvVar: '',
     wizardTarget: 'user',
     wizardModelSearch: '',
@@ -1716,22 +1721,24 @@ <h3>Save config to</h3>
           const userExistData = await userExist.json();
           const userConfig = userExistData.config || {};
           userConfig.providers = userProviders;
-          await apiFetch('/api/config', {
+          const userSave = await apiFetch('/api/config', {
             method: 'POST',
             headers: { 'Content-Type': 'application/json' },
             body: JSON.stringify({ location: 'user', config: userConfig }),
           });
+          if (!userSave.ok) throw new Error(await this.responseError(userSave, 'Failed to save user configuration'));
           // Save workspace config if we have one
           if (this.workspacePath && Object.keys(wsProviders).length > 0) {
             const wsExist = await apiFetch(`/api/config?location=${encodeURIComponent(this.workspacePath)}`);
             const wsExistData = await wsExist.json();
             const wsConfig = wsExistData.config || {};
             wsConfig.providers = wsProviders;
-            await apiFetch('/api/config', {
+            const workspaceSave = await apiFetch('/api/config', {
               method: 'POST',
               headers: { 'Content-Type': 'application/json' },
               body: JSON.stringify({ location: this.workspacePath, config: wsConfig }),
             });
+            if (!workspaceSave.ok) throw new Error(await this.responseError(workspaceSave, 'Failed to save workspace configuration'));
           }
         } else {
           const loc = this.getConfigLocation();
@@ -1747,13 +1754,21 @@ <h3>Save config to</h3>
             clean[id] = c;
           }
           existConfig.providers = clean;
-          await apiFetch('/api/config', {
+          const saveResp = await apiFetch('/api/config', {
             method: 'POST',
             headers: { 'Content-Type': 'application/json' },
             body: JSON.stringify({ location: loc, config: existConfig }),
           });
+          if (!saveResp.ok) throw new Error(await this.responseError(saveResp, 'Failed to save configuration'));
         }
-      } catch (e) { console.error('Failed to save config:', e); }
+      } catch (e) { console.error('Failed to save config:', e); throw e; }
+    },
+
+    async responseError(response, fallback) {
+      try {
+        const data = await response.json();
+        return data.error || fallback;
+      } catch { return fallback; }
     },
 
     async refresh() {
@@ -1776,7 +1791,7 @@ <h3>Save config to</h3>
       if (!cfg) return 'not-configured';
       const engine = this.engines.find(e => e.id === cfg.engine);
       if (engine && !engine.requiresKey) return 'configured';
-      if (cfg.api_key_keychain_name || cfg.api_key_env_var_name) return 'configured';
+      if (cfg.secret_name || cfg.api_key_keychain_name || cfg.api_key_env_var_name) return 'configured';
       return 'key-missing';
     },
     getProviderStatusText(provId) {
@@ -1843,6 +1858,7 @@ <h3>Save config to</h3>
       this.wizardKeySource = 'keychain';
       this.wizardApiKey = '';
       this.wizardHasExistingKey = false;
+      this.wizardError = '';
       this.wizardEnvVar = '';
       this.wizardTarget = this.configLocation;
       this.wizardModelSearch = '';
@@ -1862,9 +1878,12 @@ <h3>Save config to</h3>
       this.wizardEngine = cfg.engine || provId;
       this.wizardName = provId;
       this.wizardBaseUrl = cfg.base_url || '';
-      this.wizardKeySource = cfg.api_key_env_var_name ? 'env' : 'keychain';
+      this.wizardKeySource = cfg.secret_store === 'env' || cfg.api_key_env_var_name
+        ? 'env'
+        : cfg.secret_store === 'file' ? 'file' : 'keychain';
       this.wizardApiKey = '';  // Don't pre-fill key for security
-      this.wizardHasExistingKey = !!(cfg.api_key_keychain_name);  // Track if key is already set
+      this.wizardHasExistingKey = !!(cfg.secret_name || cfg.api_key_keychain_name || cfg.api_key_env_var_name);  // Track if key is already set
+      this.wizardError = '';
       this.wizardEnvVar = cfg.api_key_env_var_name || '';
       this.wizardTarget = this.configLocation;
       this.wizardModelSearch = '';
@@ -1947,6 +1966,7 @@ <h3>Save config to</h3>
     async wizardSave() {
       const name = this.wizardName;
       if (!name || !this.wizardEngine) return;
+      this.wizardError = '';
 
       // In edit mode, start from the existing config to preserve fields we don't touch
       const existingCfg = this.wizardEditMode ? (this.providerConfig[this.wizardEditId] || {}) : {};
@@ -1962,35 +1982,72 @@ <h3>Save config to</h3>
 
       // Handle API key
       if (this.engineRequiresKey(this.wizardEngine)) {
-        if (this.wizardKeySource === 'keychain' && this.wizardApiKey) {
+        if ((this.wizardKeySource === 'keychain' || this.wizardKeySource === 'file') && this.wizardApiKey) {
           // User entered a new key — save it
-          const keychainName = existingCfg.api_key_keychain_name || `${name.toUpperCase().replace(/[^A-Z0-9]/g, '_')}_API_KEY`;
+          const secretName = existingCfg.secret_name || existingCfg.api_key_keychain_name || `${name.toUpperCase().replace(/[^A-Z0-9]/g, '_')}_API_KEY`;
           try {
-            await apiFetch(`/api/secrets/${keychainName}`, {
+            const secretResp = await apiFetch(`/api/secrets/${encodeURIComponent(secretName)}`, {
               method: 'POST',
               headers: { 'Content-Type': 'application/json' },
-              body: JSON.stringify({ value: this.wizardApiKey }),
+              body: JSON.stringify({ value: this.wizardApiKey, secretStore: this.wizardKeySource }),
             });
-          } catch {}
-          provCfg.api_key_keychain_name = keychainName;
+            if (!secretResp.ok) throw new Error(await this.responseError(secretResp, 'Failed to save API key'));
+          } catch (e) {
+            this.wizardError = e instanceof Error ? e.message : 'Failed to save API key';
+            return;
+          }
+          provCfg.secret_name = secretName;
+          provCfg.secret_store = this.wizardKeySource;
+          provCfg.api_key_keychain_name = secretName;
           delete provCfg.api_key_env_var_name;
-        } else if (this.wizardKeySource === 'keychain' && !this.wizardApiKey && this.wizardEditMode) {
-          // Edit mode, no new key entered — keep existing keychain setting
+        } else if ((this.wizardKeySource === 'keychain' || this.wizardKeySource === 'file') && !this.wizardApiKey && this.wizardEditMode) {
+          // Do not silently move an existing credential to another backend.
+          const existingStore = existingCfg.secret_store ||
+            (existingCfg.api_key_env_var_name ? 'env' : 'keychain');
+          const hasExistingCredential = !!(
+            existingCfg.secret_name ||
+            existingCfg.api_key_keychain_name ||
+            existingCfg.api_key_env_var_name
+          );
+          if (hasExistingCredential && existingStore !== this.wizardKeySource) {
+            this.wizardError = 'Changing API Key Source requires entering the API key again.';
+            return;
+          }
+          // Edit mode, no new key entered — keep the existing credential setting.
+          provCfg.secret_store = existingStore;
+          provCfg.secret_name = provCfg.secret_name || provCfg.api_key_keychain_name;
+          if (provCfg.secret_name) provCfg.api_key_keychain_name = provCfg.secret_name;
+          if (existingStore === 'env') {
+            provCfg.secret_name = existingCfg.secret_name || existingCfg.api_key_env_var_name;
+            provCfg.api_key_env_var_name = provCfg.secret_name;
+            delete provCfg.api_key_keychain_name;
+          } else {
+            delete provCfg.api_key_env_var_name;
+          }
         } else if (this.wizardKeySource === 'env') {
-          provCfg.api_key_env_var_name = this.wizardEnvVar || this.getEngineDefaultEnvVar(this.wizardEngine);
+          const envName = this.wizardEnvVar || this.getEngineDefaultEnvVar(this.wizardEngine);
+          provCfg.secret_name = envName;
+          provCfg.secret_store = 'env';
+          provCfg.api_key_env_var_name = envName;
           delete provCfg.api_key_keychain_name;
         }
       }
 
-      // If provider was renamed in edit mode, remove old key
+      // Stage the update so a failed configuration write cannot leave the UI mutated.
+      const nextProviderConfig = { ...this.providerConfig };
       if (this.wizardEditMode && this.wizardEditId !== name) {
-        delete this.providerConfig[this.wizardEditId];
+        delete nextProviderConfig[this.wizardEditId];
+      }
+      nextProviderConfig[name] = provCfg;
+      const previousProviderConfig = this.providerConfig;
+      this.providerConfig = nextProviderConfig;
+      try {
+        await this.saveFullConfig();
+      } catch (e) {
+        this.providerConfig = previousProviderConfig;
+        this.wizardError = e instanceof Error ? e.message : 'Failed to save provider configuration';
+        return;
       }
-
-      // Save to config
-      this.providerConfig[name] = provCfg;
-      this.providerConfig = { ...this.providerConfig };
-      await this.saveFullConfig();
       await this.refresh();
       this.wizardOpen = false;
     },
@@ -2135,13 +2192,29 @@ <h3>Save config to</h3>
     async deleteProvider(provId) {
       if (!confirm(`Delete provider "${provId}"?`)) return;
       const cfg = this.providerConfig[provId];
-      if (cfg?.api_key_keychain_name) {
-        try { await apiFetch(`/api/secrets/${cfg.api_key_keychain_name}`, { method: 'DELETE' }); } catch {}
+      const secretName = cfg?.secret_name || cfg?.api_key_keychain_name;
+      const nextProviderConfig = { ...this.providerConfig };
+      delete nextProviderConfig[provId];
+      const previousProviderConfig = this.providerConfig;
+      this.providerConfig = nextProviderConfig;
+      try {
+        await this.saveFullConfig();
+        if (secretName && cfg.secret_store !== 'env' && !cfg.api_key_env_var_name) {
+          const secretStore = cfg.secret_store || (cfg.api_key_env_var_name ? 'env' : 'keychain');
+          const secretResp = await apiFetch(
+            `/api/secrets/${encodeURIComponent(secretName)}?secretStore=${encodeURIComponent(secretStore)}`,
+            { method: 'DELETE' },
+          );
+          if (!secretResp.ok) {
+            throw new Error(await this.responseError(secretResp, 'Failed to delete API key'));
+          }
+        }
+        await this.refresh();
+      } catch (e) {
+        this.providerConfig = previousProviderConfig;
+        this.wizardError = e instanceof Error ? e.message : 'Failed to delete provider';
+        alert(this.wizardError);
       }
-      delete this.providerConfig[provId];
-      this.providerConfig = { ...this.providerConfig };
-      await this.saveFullConfig();
-      await this.refresh();
     },
 
     // ── MCP Servers ──