From 8ed059b8d360f2bf599d524f9249d42a1fedb2c2 Mon Sep 17 00:00:00 2001
From: Josh Wright <josh@thewrightserver.net>
Date: Fri, 9 Oct 2026 15:33:34 -0400
Subject: [PATCH] Restrict fleet settings reads to administrators (Refs #99)

---
 src/lib/server/auth/access.ts  | 5 ++---
 tests/unit/auth-access.test.ts | 8 ++++++--
 tests/unit/auth-guard.test.ts  | 9 +++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/lib/server/auth/access.ts b/src/lib/server/auth/access.ts
index ae9f895..7a81ffe 100644
--- a/src/lib/server/auth/access.ts
+++ b/src/lib/server/auth/access.ts
@@ -25,11 +25,10 @@ export function routePermission(routeId: string | null, method: string): RoutePe
     if ((routeId === '/api/auth/login' || routeId === '/api/auth/setup') && method === 'POST') return 'public';
     if (routeId === '/account' || ['/api/auth/me', '/api/auth/logout', '/api/auth/password'].includes(routeId)) return 'authenticated';
 
-    if (within(routeId, '/api/users') || within(routeId, '/settings') || within(routeId, '/api/auth')
+    if (within(routeId, '/api/users') || within(routeId, '/settings') || within(routeId, '/api/settings') || within(routeId, '/api/auth')
         || within(routeId, '/api/server-keys') || within(routeId, '/server-keys') || within(routeId, '/keys')) return 'admin';
 
-    if (!reading && ['/api/hosts', '/api/settings', '/api/game-files'].some(base => within(routeId, base))) return 'admin';
-    if (within(routeId, '/api/settings/retention')) return 'admin';
+    if (!reading && ['/api/hosts', '/api/game-files'].some(base => within(routeId, base))) return 'admin';
     if (routeId === '/api/provisioning/[id]/override') return 'admin';
     // These endpoints mix operational actions with global policy and schedule
     // changes. Per-incident/job actions remain available to operators.
diff --git a/tests/unit/auth-access.test.ts b/tests/unit/auth-access.test.ts
index dded8be..4977de4 100644
--- a/tests/unit/auth-access.test.ts
+++ b/tests/unit/auth-access.test.ts
@@ -39,7 +39,11 @@ describe('management route permissions', () => {
         expect(allows('admin', route, method)).toBe(true);
     });
 
-    it.each(['/api/users', '/api/users/[id]', '/api/users/[id]/password', '/settings', '/settings/users', '/server-keys', '/keys', '/api/server-keys', '/api/provisioning/[id]/override'])('reserves all methods on %s for administrators', route => {
+    it.each([
+        '/api/users', '/api/users/[id]', '/api/users/[id]/password', '/settings', '/settings/users',
+        '/api/settings', '/api/settings/retention', '/api/settings/retention/preview',
+        '/server-keys', '/keys', '/api/server-keys', '/api/provisioning/[id]/override'
+    ])('reserves all methods on %s for administrators', route => {
         for (const method of ['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE']) {
             expect(routePermission(route, method)).toBe('admin');
             expect(allows('viewer', route, method)).toBe(false);
@@ -51,7 +55,7 @@ describe('management route permissions', () => {
     it.each([
         ['/api/hosts', 'POST'], ['/api/hosts/[id]', 'PATCH'], ['/api/hosts/[id]', 'DELETE'], ['/api/hosts/[id]/enrollment', 'POST'],
         ['/api/hosts/[id]/credentials', 'DELETE'], ['/api/hosts/[id]/credentials', 'POST'],
-        ['/api/hosts/[id]/operations', 'POST'], ['/api/settings', 'PUT'], ['/api/game-files', 'POST'],
+        ['/api/hosts/[id]/operations', 'POST'], ['/api/game-files', 'POST'],
         ['/api/game-files/[id]/download', 'POST'], ['/api/game-files/[id]/upload/files/[...path]', 'PUT'],
         ['/api/game-files/[id]/upload/complete', 'POST'], ['/api/maintenance', 'POST'], ['/api/watchdog', 'POST'], ['/api/plutonium', 'POST']
     ])('protects host, game-file, and global policy mutations at %s', (route, method) => {
diff --git a/tests/unit/auth-guard.test.ts b/tests/unit/auth-guard.test.ts
index e95f28e..70bb2bf 100644
--- a/tests/unit/auth-guard.test.ts
+++ b/tests/unit/auth-guard.test.ts
@@ -142,6 +142,15 @@ describe('browser authentication guard', () => {
         if (expectedStatus === 200) { expect(call.resolve).toHaveBeenCalledOnce(); expect(call.event.locals.user).toEqual(session.user); }
         else expect(call.resolve).not.toHaveBeenCalled();
     });
+    it.each(['GET', 'HEAD'])('restricts global settings %s requests to administrators', async method => {
+        for (const role of ['viewer', 'operator', 'admin'] as const) {
+            signIn(role);
+            const call = requestFor('/api/settings', { method });
+            expect((await call.invoke()).status).toBe(role === 'admin' ? 200 : 403);
+            if (role === 'admin') expect(call.resolve).toHaveBeenCalledOnce();
+            else expect(call.resolve).not.toHaveBeenCalled();
+        }
+    });
     it.each(['GET', 'HEAD'])('restricts raw beta file %s requests to operators and administrators', async method => {
         for (const role of ['viewer', 'operator', 'admin'] as const) {
             signIn(role);