fix: keep an explicit schedules disable from becoming an opt-in

`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.
This commit is contained in:
Danny Avila 2026-08-18 09:15:19 -04:00
parent f3f4cd1823
commit 208f218b7c
2 changed files with 113 additions and 22 deletions

View file

@ -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({

View file

@ -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<string, unknown>).use === false ? false : value;
}
/** Applies {@link normalizeRuntimeInterfaceValue} to a bare `interface.<field>` 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<IConfig[]>;
findConfigByPrincipal: (
@ -617,7 +646,8 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
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>)) {
for (const [field, rawVal] of Object.entries(iface as Record<string, unknown>)) {
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');