diff --git a/server/src/services/settingsService.ts b/server/src/services/settingsService.ts index 7f0580021..52a47c251 100644 --- a/server/src/services/settingsService.ts +++ b/server/src/services/settingsService.ts @@ -140,7 +140,17 @@ export function getUserSettings(userId: number): Record { // an empty string would otherwise shadow the admin/system-wide default and the // user could never fall back to it — even after clearing their own value // (#1634). Non-defaultable keys keep their exact stored value. - const merged: Record = { ...adminDefaults }; + // + // Seed from the admin defaults, but mask any encrypted secret (e.g. a shared + // llm_api_key) exactly like the per-user path above: getUserSettings is the + // client-facing accessor and must never hand back a secret in cleartext — not + // even one inherited from an admin default (the fall-through above widened the + // set of users who inherit it). The server reads the real value via + // getAdminUserDefaults / getDecryptedUserSetting, which don't mask. + const merged: Record = {}; + for (const [key, value] of Object.entries(adminDefaults)) { + merged[key] = MASKED_SETTING_KEYS.has(key) ? (value ? '••••••••' : '') : value; + } for (const [key, value] of Object.entries(userSettings)) { if ( DEFAULTABLE_USER_SETTING_KEY_SET.has(key) && diff --git a/server/tests/unit/services/settingsService.test.ts b/server/tests/unit/services/settingsService.test.ts index 92f92d2a7..6395fec31 100644 --- a/server/tests/unit/services/settingsService.test.ts +++ b/server/tests/unit/services/settingsService.test.ts @@ -152,6 +152,21 @@ describe('getUserSettings', () => { testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'theme', '')").run(user.id); expect(getUserSettings(user.id).theme).toBe(''); }); + + it('SET-SVC-025 — an inherited admin llm_api_key is masked, never returned in cleartext', () => { + const { user } = createUser(testDb); + setAdminDefault('llm_api_key', 'sk-admin-secret'); + // No user row → the value is inherited from the admin default; it is a secret + // and must reach the client masked, not in cleartext. + expect(getUserSettings(user.id).llm_api_key).toBe('••••••••'); + }); + + it('SET-SVC-026 — an empty user llm_api_key falls back to the admin key, still masked', () => { + const { user } = createUser(testDb); + setAdminDefault('llm_api_key', 'sk-admin-secret'); + testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'llm_api_key', '')").run(user.id); + expect(getUserSettings(user.id).llm_api_key).toBe('••••••••'); + }); }); // ── upsertSetting ─────────────────────────────────────────────────────────────