diff --git a/backend/src/__tests__/settings-routes.test.ts b/backend/src/__tests__/settings-routes.test.ts index d0a1da28..5618e811 100644 --- a/backend/src/__tests__/settings-routes.test.ts +++ b/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(); + }); }); }); diff --git a/backend/src/routes/settings.ts b/backend/src/routes/settings.ts index 0663c4d5..5cbe45fa 100644 --- a/backend/src/routes/settings.ts +++ b/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 => { 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 = {}; + for (const [key, value] of Object.entries(all)) { + if (ALLOWED_SETTING_KEYS.has(key)) settings[key] = value; } res.json(settings); } catch (error) {