🚫 refactor: Remove Interface Config from Override Processing (#12473)

Add INTERFACE_PERMISSION_FIELDS set defining the interface fields that seed role permissions at startup (prompts, agents, marketplace, etc.). These fields are now stripped from DB config overrides in the merge layer because updateInterfacePermissions() only runs at boot — DB overrides for these fields create a client/server permission mismatch. Pure UI fields (endpointsMenu, modelSelect, parameters, presets, sidePanel, customWelcome, etc.) continue to work in overrides as before. YAML startup path is completely unaffected.
2026-04-03 14:27:20 +02:00 · 2026-03-31 11:07:31 -04:00 · 2026-03-31 11:07:31 -04:00 · c0ce7fee91
commit c0ce7fee91
parent 3d1b883e9d
7 changed files with 607 additions and 20 deletions
--- a/packages/api/src/admin/config.handler.spec.ts
+++ b/packages/api/src/admin/config.handler.spec.ts
@ -48,7 +48,7 @@ function createHandlers(overrides = {}) {
    }),
    patchConfigFields: jest
      .fn()
-      .mockResolvedValue({ _id: 'c1', overrides: { interface: { endpointsMenu: false } } }),
+      .mockResolvedValue({ _id: 'c1', overrides: { registration: { enabled: false } } }),
    unsetConfigField: jest.fn().mockResolvedValue({ _id: 'c1', overrides: {} }),
    deleteConfig: jest.fn().mockResolvedValue({ _id: 'c1' }),
    toggleConfigActive: jest.fn().mockResolvedValue({ _id: 'c1', isActive: false }),
@ -169,6 +169,93 @@ describe('createAdminConfigHandlers', () => {

      expect(res.statusCode).toBe(400);
    });
+
+    it('strips permission fields from interface overrides but keeps UI fields', async () => {
+      const { handlers, deps } = createHandlers({
+        upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }),
+      });
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          overrides: {
+            interface: { endpointsMenu: false, prompts: false, agents: { use: false } },
+          },
+        },
+      });
+      const res = mockRes();
+
+      await handlers.upsertConfigOverrides(req, res);
+
+      expect(res.statusCode).toBe(201);
+      const savedOverrides = deps.upsertConfig.mock.calls[0][3];
+      expect(savedOverrides.interface).toEqual({ endpointsMenu: false });
+    });
+
+    it('preserves UI sub-keys in composite permission fields like mcpServers', async () => {
+      const { handlers, deps } = createHandlers({
+        upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }),
+      });
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          overrides: {
+            interface: {
+              mcpServers: {
+                use: true,
+                create: false,
+                placeholder: 'Search MCP...',
+                trustCheckbox: { label: 'Trust' },
+              },
+            },
+          },
+        },
+      });
+      const res = mockRes();
+
+      await handlers.upsertConfigOverrides(req, res);
+
+      expect(res.statusCode).toBe(201);
+      const savedOverrides = deps.upsertConfig.mock.calls[0][3];
+      const mcp = (savedOverrides as Record<string, unknown>).interface as Record<string, unknown>;
+      expect((mcp.mcpServers as Record<string, unknown>).placeholder).toBe('Search MCP...');
+      expect((mcp.mcpServers as Record<string, unknown>).trustCheckbox).toEqual({ label: 'Trust' });
+      expect((mcp.mcpServers as Record<string, unknown>).use).toBeUndefined();
+      expect((mcp.mcpServers as Record<string, unknown>).create).toBeUndefined();
+    });
+
+    it('strips peoplePicker permission sub-keys in upsert', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          overrides: {
+            interface: { peoplePicker: { users: false, groups: true, roles: true } },
+          },
+        },
+      });
+      const res = mockRes();
+
+      await handlers.upsertConfigOverrides(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.upsertConfig).not.toHaveBeenCalled();
+    });
+
+    it('returns 200 with message when only permission fields in interface', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: { overrides: { interface: { prompts: false, agents: false } } },
+      });
+      const res = mockRes();
+
+      await handlers.upsertConfigOverrides(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.upsertConfig).not.toHaveBeenCalled();
+    });
  });

  describe('deleteConfigField', () => {
@ -189,6 +276,87 @@ describe('createAdminConfigHandlers', () => {
      );
    });

+    it('allows deleting mcpServers UI sub-key paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        query: { fieldPath: 'interface.mcpServers.placeholder' },
+      });
+      const res = mockRes();
+
+      await handlers.deleteConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(deps.unsetConfigField).toHaveBeenCalledWith(
+        'role',
+        'admin',
+        'interface.mcpServers.placeholder',
+      );
+    });
+
+    it('blocks deleting mcpServers permission sub-key paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        query: { fieldPath: 'interface.mcpServers.use' },
+      });
+      const res = mockRes();
+
+      await handlers.deleteConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.unsetConfigField).not.toHaveBeenCalled();
+    });
+
+    it('blocks deleting peoplePicker permission sub-key paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        query: { fieldPath: 'interface.peoplePicker.users' },
+      });
+      const res = mockRes();
+
+      await handlers.deleteConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.unsetConfigField).not.toHaveBeenCalled();
+    });
+
+    it('returns 200 no-op for interface permission field path', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        query: { fieldPath: 'interface.prompts' },
+      });
+      const res = mockRes();
+
+      await handlers.deleteConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.unsetConfigField).not.toHaveBeenCalled();
+    });
+
+    it('allows deleting interface UI field paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        query: { fieldPath: 'interface.endpointsMenu' },
+      });
+      const res = mockRes();
+
+      await handlers.deleteConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(deps.unsetConfigField).toHaveBeenCalledWith(
+        'role',
+        'admin',
+        'interface.endpointsMenu',
+      );
+    });
+
    it('returns 400 when fieldPath query param is missing', async () => {
      const { handlers } = createHandlers();
      const req = mockReq({
@ -224,7 +392,110 @@ describe('createAdminConfigHandlers', () => {
      });
      const req = mockReq({
        params: { principalType: 'role', principalId: 'admin' },
-        body: { entries: [{ fieldPath: 'interface.endpointsMenu', value: false }] },
+        body: { entries: [{ fieldPath: 'registration.enabled', value: false }] },
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(403);
+    });
+
+    it('strips interface permission field entries but keeps UI field entries', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          entries: [
+            { fieldPath: 'interface.endpointsMenu', value: false },
+            { fieldPath: 'interface.prompts', value: false },
+          ],
+        },
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      const patchedFields = deps.patchConfigFields.mock.calls[0][3];
+      expect(patchedFields['interface.endpointsMenu']).toBe(false);
+      expect(patchedFields['interface.prompts']).toBeUndefined();
+    });
+
+    it('blocks peoplePicker permission sub-key paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          entries: [{ fieldPath: 'interface.peoplePicker.users', value: false }],
+        },
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.patchConfigFields).not.toHaveBeenCalled();
+    });
+
+    it('allows mcpServers UI sub-key paths but blocks permission sub-key paths', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: {
+          entries: [
+            { fieldPath: 'interface.mcpServers.placeholder', value: 'Search...' },
+            { fieldPath: 'interface.mcpServers.use', value: true },
+          ],
+        },
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      const patchedFields = deps.patchConfigFields.mock.calls[0][3];
+      expect(patchedFields['interface.mcpServers.placeholder']).toBe('Search...');
+      expect(patchedFields['interface.mcpServers.use']).toBeUndefined();
+    });
+
+    it('returns 200 with message when all entries are permission fields', async () => {
+      const { handlers, deps } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: { entries: [{ fieldPath: 'interface.prompts', value: false }] },
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(200);
+      expect(res.body!.message).toBeDefined();
+      expect(deps.patchConfigFields).not.toHaveBeenCalled();
+    });
+
+    it('returns 401 when unauthenticated even if all entries are permission fields', async () => {
+      const { handlers } = createHandlers();
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: { entries: [{ fieldPath: 'interface.prompts', value: false }] },
+        user: undefined,
+      });
+      const res = mockRes();
+
+      await handlers.patchConfigField(req, res);
+
+      expect(res.statusCode).toBe(401);
+    });
+
+    it('returns 403 when unauthorized even if all entries are permission fields', async () => {
+      const { handlers } = createHandlers({
+        hasConfigCapability: jest.fn().mockResolvedValue(false),
+      });
+      const req = mockReq({
+        params: { principalType: 'role', principalId: 'admin' },
+        body: { entries: [{ fieldPath: 'interface.prompts', value: false }] },
      });
      const res = mockRes();

--- a/packages/api/src/admin/config.ts
+++ b/packages/api/src/admin/config.ts
@ -1,5 +1,10 @@
 import { logger } from '@librechat/data-schemas';
-import { PrincipalType, PrincipalModel } from 'librechat-data-provider';
+import {
+  PrincipalType,
+  PrincipalModel,
+  INTERFACE_PERMISSION_FIELDS,
+  PERMISSION_SUB_KEYS,
+} from 'librechat-data-provider';
 import type { TCustomConfig } from 'librechat-data-provider';
 import type { AppConfig, ConfigSection, IConfig } from '@librechat/data-schemas';
 import type { Types, ClientSession } from 'mongoose';
@ -26,6 +31,33 @@ export function getTopLevelSection(fieldPath: string): string {
  return fieldPath.split('.')[0];
 }

+/**
+ * Returns true if `fieldPath` targets an interface permission field or permission sub-key.
+ *
+ * - `"interface.prompts"` → true (boolean permission field)
+ * - `"interface.agents.use"` → true (permission sub-key)
+ * - `"interface.mcpServers"` → true (entire composite field)
+ * - `"interface.mcpServers.use"` → true (permission sub-key)
+ * - `"interface.mcpServers.placeholder"` → false (UI-only sub-key)
+ * - `"interface.peoplePicker.users"` → true (all peoplePicker sub-keys are permissions)
+ * - `"interface.endpointsMenu"` → false (UI-only field)
+ */
+function isInterfacePermissionPath(fieldPath: string): boolean {
+  const parts = fieldPath.split('.');
+  if (parts[0] !== 'interface' || parts.length < 2) {
+    return false;
+  }
+  if (!INTERFACE_PERMISSION_FIELDS.has(parts[1])) {
+    return false;
+  }
+  // "interface.<permField>" with no sub-key → permission (blocks the whole field)
+  if (parts.length === 2) {
+    return true;
+  }
+  // "interface.<permField>.<subKey>" → only block if sub-key is a permission bit
+  return PERMISSION_SUB_KEYS.has(parts[2]);
+}
+
 export interface AdminConfigDeps {
  listAllConfigs: (filter?: { isActive?: boolean }, session?: ClientSession) => Promise<IConfig[]>;
  findConfigByPrincipal: (
@ -262,24 +294,64 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
        return res.status(403).json({ error: 'Insufficient permissions' });
      }

-      const overrideSections = Object.keys(overrides);
-      if (overrideSections.length > 0) {
-        const allowed = await Promise.all(
-          overrideSections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')),
-        );
-        const denied = overrideSections.find((_, i) => !allowed[i]);
-        if (denied) {
-          return res.status(403).json({
-            error: `Insufficient permissions for config section: ${denied}`,
-          });
+      let filteredOverrides = overrides;
+      const iface = (overrides as Record<string, unknown>).interface;
+      if (iface != null && typeof iface === 'object' && !Array.isArray(iface)) {
+        const filteredIface: Record<string, unknown> = {};
+        for (const [field, val] of Object.entries(iface as Record<string, unknown>)) {
+          if (!INTERFACE_PERMISSION_FIELDS.has(field)) {
+            filteredIface[field] = val;
+          } else if (val != null && typeof val === 'object' && !Array.isArray(val)) {
+            // Composite permission field (e.g. mcpServers): strip permission
+            // sub-keys but preserve UI-only sub-keys like placeholder/trustCheckbox.
+            const uiOnly: Record<string, unknown> = {};
+            for (const [sub, subVal] of Object.entries(val as Record<string, unknown>)) {
+              if (!PERMISSION_SUB_KEYS.has(sub)) {
+                uiOnly[sub] = subVal;
+              } else {
+                logger.warn(
+                  `[adminConfig] Stripping interface permission sub-field "${field}.${sub}" — use role permissions instead`,
+                );
+              }
+            }
+            if (Object.keys(uiOnly).length > 0) {
+              filteredIface[field] = uiOnly;
+            }
+          } else {
+            logger.warn(
+              `[adminConfig] Stripping interface permission field "${field}" — use role permissions instead`,
+            );
+          }
        }
+        filteredOverrides = { ...(overrides as Record<string, unknown>) } as Partial<TCustomConfig>;
+        if (Object.keys(filteredIface).length > 0) {
+          (filteredOverrides as Record<string, unknown>).interface = filteredIface;
+        } else {
+          delete (filteredOverrides as Record<string, unknown>).interface;
+        }
+      }
+
+      const overrideSections = Object.keys(filteredOverrides);
+
+      if (overrideSections.length === 0) {
+        return res.status(200).json({ message: 'No actionable override sections provided' });
+      }
+
+      const allowed = await Promise.all(
+        overrideSections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')),
+      );
+      const denied = overrideSections.find((_, i) => !allowed[i]);
+      if (denied) {
+        return res.status(403).json({
+          error: `Insufficient permissions for config section: ${denied}`,
+        });
      }

      const config = await upsertConfig(
        principalType,
        principalId,
        principalModel(principalType),
-        overrides,
+        filteredOverrides,
        priority ?? DEFAULT_PRIORITY,
      );

@ -339,8 +411,25 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
        return res.status(401).json({ error: 'Authentication required' });
      }

+      const validEntries = entries.filter((entry) => {
+        if (isInterfacePermissionPath(entry.fieldPath)) {
+          logger.warn(
+            `[adminConfig] Stripping interface permission field "${entry.fieldPath}" — use role permissions instead`,
+          );
+          return false;
+        }
+        return true;
+      });
+
+      if (validEntries.length === 0) {
+        if (!(await hasConfigCapability(user, null, 'manage'))) {
+          return res.status(403).json({ error: 'Insufficient permissions' });
+        }
+        return res.status(200).json({ message: 'No actionable field entries provided' });
+      }
+
      if (!(await hasConfigCapability(user, null, 'manage'))) {
-        const sections = [...new Set(entries.map((e) => getTopLevelSection(e.fieldPath)))];
+        const sections = [...new Set(validEntries.map((e) => getTopLevelSection(e.fieldPath)))];
        const allowed = await Promise.all(
          sections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')),
        );
@ -354,7 +443,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {

      const seen = new Set<string>();
      const fields: Record<string, unknown> = {};
-      for (const entry of entries) {
+      for (const entry of validEntries) {
        if (seen.has(entry.fieldPath)) {
          return res.status(400).json({ error: `Duplicate fieldPath: ${entry.fieldPath}` });
        }
@ -414,12 +503,20 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
      }

      const section = getTopLevelSection(fieldPath);
+
      if (!(await hasConfigCapability(user, section as ConfigSection, 'manage'))) {
        return res.status(403).json({
          error: `Insufficient permissions for config section: ${section}`,
        });
      }

+      if (isInterfacePermissionPath(fieldPath)) {
+        logger.warn(
+          `[adminConfig] Ignoring delete for interface permission field "${fieldPath}" — use role permissions instead`,
+        );
+        return res.status(200).json({ message: 'No actionable field path provided' });
+      }
+
      const config = await unsetConfigField(principalType, principalId, fieldPath);
      if (!config) {
        return res.status(404).json({ error: 'Config not found' });