fix(settings): serialize GET /api/settings from an allowlist (#1299)

The settings read returned the entire global_settings map minus a small
denylist of auth keys. Other subsystems persist their config in the same
table, so cloud backup values (endpoint, bucket, access key) were returned
to any authenticated user, including read-only roles, bypassing the
redaction the dedicated cloud-backup endpoint applies.

Project the response from the operational allowlist instead, so only the
keys the settings UI reads are returned and any key written to
global_settings by another subsystem is excluded by default. Remove the
now-unused denylist constant.

backend/src/__tests__/settings-routes.test.ts

@@ -55,10 +55,67 @@ describe('GET /api/settings', () => {
     expect(res.body.auth_jwt_secret).toBeUndefined();
   });
 
-  it('allows non-admin users to read settings', async () => {
-    // Settings is read-only for non-admins; write is admin-gated separately.
-    const res = await request(app).get('/api/settings').set('Cookie', viewerCookie);
-    expect(res.status).toBe(200);
+  describe('credential projection (leak prevention)', () => {
+    // Seeded leak-probe values are reset afterward so the shared test DB does
+    // not carry them into later suites (matches the cleanup convention used by
+    // the mesh_auto_recreate write test below).
+    afterAll(() => {
+      const db = DatabaseService.getInstance();
+      for (const k of [
+        'cloud_backup_access_key',
+        'cloud_backup_secret_key',
+        'cloud_backup_endpoint',
+        'cloud_backup_bucket',
+        'some_future_setting',
+      ]) {
+        db.updateGlobalSetting(k, '');
+      }
+      db.updateGlobalSetting('trivy_auto_update', '0');
+      db.updateGlobalSetting('mesh_auto_recreate', '0');
+    });
+
+    it('never returns credentials or other non-allowlisted keys, even to admins', async () => {
+      // Cloud backup config is stored in global_settings by the cloud-backup
+      // route (access key plaintext, secret key encrypted). The generic settings
+      // GET projects an allowlist, so these credentials and any other key written
+      // to the table are never disclosed here.
+      const db = DatabaseService.getInstance();
+      db.updateGlobalSetting('cloud_backup_access_key', 'AKIA-must-not-leak');
+      db.updateGlobalSetting('cloud_backup_secret_key', 'cipher-must-not-leak');
+      db.updateGlobalSetting('cloud_backup_endpoint', 'https://s3.example.com');
+      db.updateGlobalSetting('cloud_backup_bucket', 'private-bucket');
+      db.updateGlobalSetting('trivy_auto_update', '1');
+      // A key that is neither allowlisted nor obviously sensitive: the allowlist
+      // must still exclude it. This locks the fail-closed contract that is the
+      // reason the GET uses an allowlist rather than a denylist.
+      db.updateGlobalSetting('some_future_setting', 'should-not-appear');
+      db.updateGlobalSetting('host_cpu_limit', '80');
+      db.updateGlobalSetting('mesh_auto_recreate', '1');
+
+      const res = await request(app).get('/api/settings').set('Cookie', adminCookie);
+      expect(res.status).toBe(200);
+      expect(res.body.cloud_backup_access_key).toBeUndefined();
+      expect(res.body.cloud_backup_secret_key).toBeUndefined();
+      expect(res.body.cloud_backup_endpoint).toBeUndefined();
+      expect(res.body.cloud_backup_bucket).toBeUndefined();
+      expect(res.body.trivy_auto_update).toBeUndefined();
+      expect(res.body.some_future_setting).toBeUndefined();
+      // Allowlisted keys still come through. Two structurally different keys (a
+      // numeric and an enum-shaped one) prove the projection passes the whole
+      // allowlist, not just one key.
+      expect(res.body.host_cpu_limit).toBe('80');
+      expect(res.body.mesh_auto_recreate).toBe('1');
+    });
+
+    it('allows non-admin users to read settings without leaking credentials', async () => {
+      // Settings is read-only for non-admins; write is admin-gated separately.
+      // The leak matters most here: a viewer must never receive backup creds.
+      DatabaseService.getInstance().updateGlobalSetting('cloud_backup_access_key', 'AKIA-viewer-must-not-see');
+      const res = await request(app).get('/api/settings').set('Cookie', viewerCookie);
+      expect(res.status).toBe(200);
+      expect(res.body.cloud_backup_access_key).toBeUndefined();
+      expect(res.body.auth_password_hash).toBeUndefined();
+    });
   });
 });
 

backend/src/routes/settings.ts

@@ -4,12 +4,12 @@ import { DatabaseService } from '../services/DatabaseService';
 import { authMiddleware } from '../middleware/auth';
 import { requireAdmin } from '../middleware/tierGates';
 
-// Keys that contain auth credentials; never exposed to the frontend or
-// writable via the settings API.
-const PRIVATE_SETTINGS_KEYS = new Set(['auth_username', 'auth_password_hash', 'auth_jwt_secret']);
-
-// Strict allowlist of keys writable via the settings API. Prevents
-// overwriting auth credentials through a misconfigured key.
+// Strict allowlist of keys readable and writable via the generic settings
+// API. This is the single source of truth for what the endpoint exposes:
+// reads project only these keys, so secrets written to global_settings by
+// other subsystems (the cloud_backup_* credentials stored by the cloud-backup
+// route, the auth_* login secrets) are never returned here; writes are
+// rejected for anything outside the list.
 const ALLOWED_SETTING_KEYS = new Set([
   'host_cpu_limit',
   'host_ram_limit',
@@ -47,9 +47,14 @@ export const settingsRouter = Router();
 
 settingsRouter.get('/', authMiddleware, async (_req: Request, res: Response): Promise<void> => {
   try {
-    const settings = { ...DatabaseService.getInstance().getGlobalSettings() };
-    for (const key of PRIVATE_SETTINGS_KEYS) {
-      delete settings[key];
+    const all = DatabaseService.getInstance().getGlobalSettings();
+    // Project only allowlisted operational keys. A denylist would leak every
+    // future sensitive key written to global_settings by default (e.g. the
+    // cloud_backup_* credentials the cloud-backup route stores here); the
+    // allowlist fails closed.
+    const settings: Record<string, string> = {};
+    for (const [key, value] of Object.entries(all)) {
+      if (ALLOWED_SETTING_KEYS.has(key)) settings[key] = value;
     }
     res.json(settings);
   } catch (error) {