From 208f218b7c9d4d5ff7cd2da780821f55123ffa6c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 18 Aug 2026 09:15:19 -0400 Subject: [PATCH] fix: keep an explicit schedules disable from becoming an opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `use` is two things at once for a dual-purpose runtime interface field: a permission bit, which DB overrides strip, and the runtime disable signal that `getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an object, and `getLimits` treats any object without `use: false` as enabled — so an admin override written to stop scheduled billing for a role or user started it instead. Collapse an explicit disable to the boolean form before the strip, on both paths that accept it: the `interface.schedules` field patch, which admitted the object wholesale because bare runtime paths deliberately bypass the permission gate, and the overrides merge, which reached the composite-field branch and kept `maxPerUser`. Objects that only narrow limits are untouched, so a principal can still be given a smaller cap. Both regressions fail without the normalizer. --- packages/api/src/admin/config.handler.spec.ts | 56 +++++++++++++ packages/api/src/admin/config.ts | 79 +++++++++++++------ 2 files changed, 113 insertions(+), 22 deletions(-) diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index 7d44f65a12..2b037bda76 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -521,6 +521,25 @@ describe('createAdminConfigHandlers', () => { expect(savedOverrides.interface).toEqual({ modelSelect: false }); }); + it('collapses an explicit schedules disable to the boolean form in overrides', async () => { + const { handlers, deps } = createHandlers({ + upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + overrides: { interface: { schedules: { use: false, maxPerUser: 2 } } }, + }, + }); + 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({ schedules: false }); + }); + it('preserves skillSync sections in admin overrides', async () => { const { handlers, deps } = createHandlers({ upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }), @@ -1046,6 +1065,43 @@ describe('createAdminConfigHandlers', () => { expect(patchedFields['interface.prompts']).toBeUndefined(); }); + /** `use` is both a permission bit and the runtime disable for dual-purpose fields. + * Stripping it alone leaves an object, which `getLimits` reads as ENABLED — so an + * override meant to stop scheduled billing would start it. */ + it('collapses an explicit schedules disable to the boolean form in patches', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'interface.schedules', value: { use: false, maxPerUser: 2 } }], + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const patchedFields = deps.patchConfigFields.mock.calls[0][3]; + expect(patchedFields['interface.schedules']).toBe(false); + }); + + it('keeps a schedules object that only narrows limits', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'interface.schedules', value: { maxPerUser: 2 } }], + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const patchedFields = deps.patchConfigFields.mock.calls[0][3]; + expect(patchedFields['interface.schedules']).toEqual({ maxPerUser: 2 }); + }); + it('preserves skillSync field entries in patches', async () => { const { handlers, deps } = createHandlers(); const req = mockReq({ diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index 0d8799aafb..979aaafa19 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -137,6 +137,35 @@ function isInterfacePermissionPath(fieldPath: string): boolean { return PERMISSION_SUB_KEYS.has(parts[2]); } +/** + * Collapses an explicit disable on a dual-purpose runtime interface field (e.g. `schedules`) + * to its boolean form. + * + * For these fields `use` is BOTH a permission bit — stripped from DB overrides — and the + * runtime disable signal. Stripping it alone would leave `{ maxPerUser: 2 }`, which + * `getLimits` reads as ENABLED because an object opts in unless it sets `use: false`. An + * override written to stop scheduled billing for a principal would therefore start it. + * Other object forms are left alone so a principal can still narrow limits. + */ +function normalizeRuntimeInterfaceValue(field: string, value: unknown): unknown { + if (!RUNTIME_CONFIG_INTERFACE_FIELDS.has(field)) { + return value; + } + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + return value; + } + return (value as Record).use === false ? false : value; +} + +/** Applies {@link normalizeRuntimeInterfaceValue} to a bare `interface.` patch. */ +function normalizeInterfaceFieldPatch(fieldPath: string, value: unknown): unknown { + const parts = fieldPath.split('.'); + if (parts[0] !== 'interface' || parts.length !== 2) { + return value; + } + return normalizeRuntimeInterfaceValue(parts[1], value); +} + export interface AdminConfigDeps { listAllConfigs: (filter?: { isActive?: boolean }, session?: ClientSession) => Promise; findConfigByPrincipal: ( @@ -617,7 +646,8 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { const iface = (overrides as Record).interface; if (iface != null && typeof iface === 'object' && !Array.isArray(iface)) { const filteredIface: Record = {}; - for (const [field, val] of Object.entries(iface as Record)) { + for (const [field, rawVal] of Object.entries(iface as Record)) { + const val = normalizeRuntimeInterfaceValue(field, rawVal); if (!INTERFACE_PERMISSION_FIELDS.has(field)) { filteredIface[field] = val; } else if (val != null && typeof val === 'object' && !Array.isArray(val)) { @@ -803,27 +833,32 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(401).json({ error: 'Authentication required' }); } - const validEntries = entries.filter((entry) => { - if (isBaseOnlyFieldPath(entry.fieldPath)) { - logger.warn( - `[adminConfig] Stripping base-only config field "${entry.fieldPath}" - configure it in librechat.yaml instead`, - ); - return false; - } - if (BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(getTopLevelSection(entry.fieldPath))) { - logger.warn( - `[adminConfig] Stripping dedicated tenant-wide config field "${entry.fieldPath}" from the generic config API`, - ); - return false; - } - if (isInterfacePermissionPath(entry.fieldPath)) { - logger.warn( - `[adminConfig] Stripping interface permission field "${entry.fieldPath}" — use role permissions instead`, - ); - return false; - } - return true; - }); + const validEntries = entries + .map((entry) => ({ + ...entry, + value: normalizeInterfaceFieldPatch(entry.fieldPath, entry.value), + })) + .filter((entry) => { + if (isBaseOnlyFieldPath(entry.fieldPath)) { + logger.warn( + `[adminConfig] Stripping base-only config field "${entry.fieldPath}" - configure it in librechat.yaml instead`, + ); + return false; + } + if (BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(getTopLevelSection(entry.fieldPath))) { + logger.warn( + `[adminConfig] Stripping dedicated tenant-wide config field "${entry.fieldPath}" from the generic config API`, + ); + return false; + } + if (isInterfacePermissionPath(entry.fieldPath)) { + logger.warn( + `[adminConfig] Stripping interface permission field "${entry.fieldPath}" — use role permissions instead`, + ); + return false; + } + return true; + }); const hasBroadManage = await hasConfigCapability(user, null, 'manage');