🪦 fix: Add Durable MCP Config Tombstones (#13534)

* fix: add durable MCP config tombstones

* fix: preserve scoped config tombstones

* fix: clean up config tombstone lint

* fix: handle empty model spec skill allowlist

* fix: preserve inactive config tombstones
This commit is contained in:
Danny Avila 2026-06-05 15:05:40 -04:00 committed by GitHub
parent 5118a566df
commit aeb5adff34
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 506 additions and 6 deletions

View file

@ -18,6 +18,7 @@ const handlers = createAdminConfigHandlers({
findConfigByPrincipal: db.findConfigByPrincipal,
upsertConfig: db.upsertConfig,
patchConfigFields: db.patchConfigFields,
tombstoneConfigField: db.tombstoneConfigField,
unsetConfigField: db.unsetConfigField,
deleteConfig: db.deleteConfig,
toggleConfigActive: db.toggleConfigActive,
@ -33,6 +34,7 @@ router.get('/base', handlers.getBaseConfig);
router.get('/:principalType/:principalId', handlers.getConfig);
router.put('/:principalType/:principalId', handlers.upsertConfigOverrides);
router.patch('/:principalType/:principalId/fields', handlers.patchConfigField);
router.post('/:principalType/:principalId/fields/tombstone', handlers.tombstoneConfigField);
router.delete('/:principalType/:principalId/fields', handlers.deleteConfigField);
router.delete('/:principalType/:principalId', handlers.deleteConfigOverrides);
router.patch('/:principalType/:principalId/active', handlers.toggleConfig);

View file

@ -49,6 +49,9 @@ function createHandlers(overrides = {}) {
patchConfigFields: jest
.fn()
.mockResolvedValue({ _id: 'c1', overrides: { registration: { enabled: false } } }),
tombstoneConfigField: jest
.fn()
.mockResolvedValue({ _id: 'c1', tombstones: ['mcpServers.github'] }),
unsetConfigField: jest.fn().mockResolvedValue({ _id: 'c1', overrides: {} }),
deleteConfig: jest.fn().mockResolvedValue({ _id: 'c1' }),
toggleConfigActive: jest.fn().mockResolvedValue({ _id: 'c1', isActive: false }),
@ -377,6 +380,78 @@ describe('createAdminConfigHandlers', () => {
});
});
describe('tombstoneConfigField', () => {
it('writes an explicit tombstone for a valid field path', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { fieldPath: 'mcpServers.github' },
});
const res = mockRes();
await handlers.tombstoneConfigField(req, res);
expect(res.statusCode).toBe(200);
expect(deps.tombstoneConfigField).toHaveBeenCalledWith(
'role',
'admin',
expect.anything(),
'mcpServers.github',
10,
);
});
it('uses the existing config priority when priority is omitted', async () => {
const { handlers, deps } = createHandlers({
findConfigByPrincipal: jest.fn().mockResolvedValue({ _id: 'c1', priority: 42 }),
});
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { fieldPath: 'mcpServers.github' },
});
const res = mockRes();
await handlers.tombstoneConfigField(req, res);
expect(deps.tombstoneConfigField).toHaveBeenCalledWith(
'role',
'admin',
expect.anything(),
'mcpServers.github',
42,
);
});
it('blocks interface permission paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { fieldPath: 'interface.mcpServers.use' },
});
const res = mockRes();
await handlers.tombstoneConfigField(req, res);
expect(res.statusCode).toBe(200);
expect(res.body!.message).toBeDefined();
expect(deps.tombstoneConfigField).not.toHaveBeenCalled();
});
it('rejects unsafe field paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { fieldPath: '__proto__.polluted' },
});
const res = mockRes();
await handlers.tombstoneConfigField(req, res);
expect(res.statusCode).toBe(400);
expect(deps.tombstoneConfigField).not.toHaveBeenCalled();
});
});
describe('patchConfigField', () => {
it('returns 403 when user lacks capability for section', async () => {
const { handlers } = createHandlers({
@ -641,6 +716,13 @@ describe('createAdminConfigHandlers', () => {
query: { fieldPath: 'interface.modelSelect' },
},
},
{
name: 'tombstoneConfigField',
reqOverrides: {
params: { principalType: 'role', principalId: 'admin' },
body: { fieldPath: 'mcpServers.github' },
},
},
{
name: 'deleteConfigOverrides',
reqOverrides: {

View file

@ -82,6 +82,14 @@ export interface AdminConfigDeps {
priority: number,
session?: ClientSession,
) => Promise<IConfig | null>;
tombstoneConfigField: (
principalType: PrincipalType,
principalId: string | Types.ObjectId,
principalModel: PrincipalModel,
fieldPath: string,
priority: number,
session?: ClientSession,
) => Promise<IConfig | null>;
unsetConfigField: (
principalType: PrincipalType,
principalId: string | Types.ObjectId,
@ -163,6 +171,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
findConfigByPrincipal,
upsertConfig,
patchConfigFields,
tombstoneConfigField: writeConfigTombstone,
unsetConfigField,
deleteConfig,
toggleConfigActive,
@ -479,6 +488,80 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
}
}
/**
* POST /:principalType/:principalId/fields/tombstone Suppress an inherited config path.
*/
async function tombstoneConfigField(req: ServerRequest, res: Response) {
try {
const { principalType, principalId } = req.params as {
principalType: string;
principalId: string;
};
if (!validatePrincipalType(principalType)) {
return res.status(400).json({ error: `Invalid principalType: ${principalType}` });
}
const { fieldPath, priority } = req.body as {
fieldPath?: string;
priority?: number;
};
if (!fieldPath || typeof fieldPath !== 'string') {
return res.status(400).json({ error: 'fieldPath is required' });
}
if (priority != null && (typeof priority !== 'number' || priority < 0)) {
return res.status(400).json({ error: 'priority must be a non-negative number' });
}
if (!isValidFieldPath(fieldPath)) {
return res.status(400).json({ error: `Invalid or unsafe field path: ${fieldPath}` });
}
const user = getCapabilityUser(req);
if (!user) {
return res.status(401).json({ error: 'Authentication required' });
}
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 tombstone for interface permission field "${fieldPath}" — use role permissions instead`,
);
return res.status(200).json({ message: 'No actionable field path provided' });
}
const existing =
priority == null
? await findConfigByPrincipal(principalType, principalId, { includeInactive: true })
: null;
const config = await writeConfigTombstone(
principalType,
principalId,
principalModel(principalType),
fieldPath,
priority ?? existing?.priority ?? DEFAULT_PRIORITY,
);
invalidateConfigCaches?.(user.tenantId)?.catch((err) =>
logger.error('[adminConfig] Cache invalidation failed after field tombstone:', err),
);
return res.status(200).json({ config });
} catch (error) {
logger.error('[adminConfig] tombstoneConfigField error:', error);
return res.status(500).json({ error: 'Failed to tombstone config field' });
}
}
/**
* DELETE /:principalType/:principalId/fields?fieldPath=dotted.path
*/
@ -624,6 +707,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
getConfig,
upsertConfigOverrides,
patchConfigField,
tombstoneConfigField,
deleteConfigField,
deleteConfigOverrides,
toggleConfig,

View file

@ -232,8 +232,8 @@ export interface ResolveAgentScopedSkillIdsParams {
* explicit signal from the user or the agent author:
* - Ephemeral agent model-spec `skills` wins when configured:
* `true` = full accessible catalog, string list = scoped allowlist,
* `false` = no skills. Otherwise the skills badge toggle controls
* the full accessible catalog.
* empty list / `false` = no skills. Otherwise the skills badge toggle
* controls the full accessible catalog.
* - Persisted agent the builder's `skills_enabled` master switch.
* Enabled + empty allowlist = full catalog; enabled + non-empty
* allowlist = narrow to those ids; disabled (or undefined) = no skills.
@ -257,6 +257,9 @@ export function resolveAgentScopedSkillIds(
return [];
}
if (agent.skills_enabled === true) {
if (Array.isArray(agent.skills) && agent.skills.length === 0) {
return [];
}
return Array.isArray(agent.skills)
? scopeSkillIds(accessibleSkillIds, agent.skills)
: scopeSkillIds(accessibleSkillIds, undefined);

View file

@ -2,7 +2,11 @@ import { INTERFACE_PERMISSION_FIELDS, PermissionTypes } from 'librechat-data-pro
import { mergeConfigOverrides } from './resolution';
import type { AppConfig, IConfig } from '~/types';
function fakeConfig(overrides: Record<string, unknown>, priority: number): IConfig {
function fakeConfig(
overrides: Record<string, unknown>,
priority: number,
tombstones?: string[],
): IConfig {
return {
_id: 'fake',
principalType: 'role',
@ -10,6 +14,7 @@ function fakeConfig(overrides: Record<string, unknown>, priority: number): IConf
principalModel: 'Role',
priority,
overrides,
tombstones,
isActive: true,
configVersion: 1,
} as unknown as IConfig;
@ -426,6 +431,67 @@ describe('mergeConfigOverrides', () => {
});
expect(result.mcpServers).toBeUndefined();
});
it('applies tombstones after remapping YAML paths to AppConfig paths', () => {
const base = {
mcpConfig: {
github: { type: 'streamable-http', url: 'https://github.example.com' },
slack: { type: 'streamable-http', url: 'https://slack.example.com' },
},
} as unknown as AppConfig;
const result = mergeConfigOverrides(base, [
fakeConfig({}, 10, ['mcpServers.github']),
]) as unknown as Record<string, unknown>;
const mcpConfig = result.mcpConfig as Record<string, unknown>;
expect(mcpConfig.github).toBeUndefined();
expect(mcpConfig.slack).toEqual({
type: 'streamable-http',
url: 'https://slack.example.com',
});
const baseMcpConfig = (base as unknown as Record<string, unknown>).mcpConfig as Record<
string,
unknown
>;
expect(baseMcpConfig.github).toEqual({
type: 'streamable-http',
url: 'https://github.example.com',
});
});
it('lets a higher-priority override recreate a lower-priority tombstoned path', () => {
const base = {
mcpConfig: {
github: { type: 'streamable-http', url: 'https://github.example.com' },
},
} as unknown as AppConfig;
const result = mergeConfigOverrides(base, [
fakeConfig({}, 10, ['mcpServers.github']),
fakeConfig({ mcpServers: { github: { url: 'https://scoped.example.com' } } }, 100),
]) as unknown as Record<string, unknown>;
const mcpConfig = result.mcpConfig as Record<string, unknown>;
expect(mcpConfig.github).toEqual({
url: 'https://scoped.example.com',
});
});
it('lets a higher-priority tombstone suppress a lower-priority override', () => {
const base = {
mcpConfig: {},
} as unknown as AppConfig;
const result = mergeConfigOverrides(base, [
fakeConfig({ mcpServers: { github: { url: 'https://role.example.com' } } }, 10),
fakeConfig({}, 100, ['mcpServers.github']),
]) as unknown as Record<string, unknown>;
const mcpConfig = result.mcpConfig as Record<string, unknown>;
expect(mcpConfig.github).toBeUndefined();
});
});
describe('INTERFACE_PERMISSION_FIELDS', () => {

View file

@ -36,6 +36,50 @@ const OVERRIDE_KEY_MAP: Partial<Record<keyof TCustomConfig, keyof AppConfig>> =
turnstile: 'turnstileConfig',
};
function isSafePath(path: string): boolean {
const segments = path.split('.');
if (
path.length === 0 ||
path.startsWith('.') ||
path.endsWith('.') ||
path.includes('..') ||
segments.some((segment) => segment.length === 0 || UNSAFE_KEYS.has(segment))
) {
return false;
}
return true;
}
function remapOverridePath(path: string): string {
const [first, ...rest] = path.split('.');
const mappedFirst = OVERRIDE_KEY_MAP[first as keyof typeof OVERRIDE_KEY_MAP] ?? first;
return [mappedFirst, ...rest].join('.');
}
function deletePath<T extends AnyObject>(target: T, path: string): T {
if (!isSafePath(path)) {
return target;
}
const segments = path.split('.');
const result = { ...target } as AnyObject;
let cursor: AnyObject = result;
for (let index = 0; index < segments.length - 1; index++) {
const segment = segments[index];
const value = cursor[segment];
if (value == null || typeof value !== 'object' || Array.isArray(value)) {
return result as T;
}
const cloned = { ...(value as AnyObject) };
cursor[segment] = cloned;
cursor = cloned;
}
delete cursor[segments[segments.length - 1]];
return result as T;
}
function mergeArrayByKey(
target: AnyObject[],
source: AnyObject[],
@ -144,6 +188,14 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]):
let merged = { ...baseConfig };
for (const config of sorted) {
if (Array.isArray(config.tombstones)) {
for (const path of config.tombstones) {
if (typeof path === 'string') {
merged = deletePath(merged, remapOverridePath(path));
}
}
}
if (config.overrides && typeof config.overrides === 'object') {
const remapped: AnyObject = {};
for (const [key, value] of Object.entries(config.overrides)) {

View file

@ -26,7 +26,7 @@ beforeEach(async () => {
await mongoose.models.Config.deleteMany({});
});
describe('upsertConfig', () => {
describe('upsertConfig tombstone preservation', () => {
it('creates a new config document', async () => {
const result = await methods.upsertConfig(
PrincipalType.ROLE,
@ -270,6 +270,137 @@ describe('patchConfigFields', () => {
expect(result).toBeTruthy();
expect(result!.principalId).toBe('newrole');
});
it('clears tombstones for patched paths and their ancestors', async () => {
await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
const result = await methods.patchConfigFields(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{ 'mcpServers.github.url': 'https://scoped.example.com' },
10,
);
expect(result!.tombstones).not.toContain('mcpServers.github');
});
it('does not clear a whole-section tombstone when patching a nested path', async () => {
await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers',
10,
);
const result = await methods.patchConfigFields(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{ 'mcpServers.github.url': 'https://scoped.example.com' },
10,
);
expect(result!.tombstones).toContain('mcpServers');
});
});
describe('tombstoneConfigField', () => {
it('adds a tombstone and removes the overridden subtree', async () => {
await methods.upsertConfig(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{
mcpServers: {
github: {
type: 'streamable-http',
url: 'https://github.example.com',
},
slack: {
type: 'streamable-http',
url: 'https://slack.example.com',
},
},
},
10,
);
const result = await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
const overrides = result!.overrides as Record<string, unknown>;
const mcpServers = overrides.mcpServers as Record<string, unknown>;
expect(result!.tombstones).toContain('mcpServers.github');
expect(mcpServers.github).toBeUndefined();
expect(mcpServers.slack).toEqual({
type: 'streamable-http',
url: 'https://slack.example.com',
});
});
it('creates a config if none exists', async () => {
const result = await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
expect(result).toBeTruthy();
expect(result!.tombstones).toContain('mcpServers.github');
});
it('preserves inactive configs when adding a tombstone', async () => {
await methods.upsertConfig(PrincipalType.ROLE, 'admin', PrincipalModel.ROLE, {}, 10);
await methods.toggleConfigActive(PrincipalType.ROLE, 'admin', false);
const result = await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
expect(result!.isActive).toBe(false);
expect(result!.tombstones).toContain('mcpServers.github');
});
});
describe('upsertConfig', () => {
it('preserves tombstones when replacing overrides', async () => {
await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
const result = await methods.upsertConfig(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{ interface: { modelSelect: false } },
10,
);
expect(result!.tombstones).toContain('mcpServers.github');
});
});
describe('unsetConfigField', () => {
@ -297,6 +428,20 @@ describe('unsetConfigField', () => {
const result = await methods.unsetConfigField(PrincipalType.ROLE, 'ghost', 'a.b');
expect(result).toBeNull();
});
it('clears tombstones for the reset path and descendants', async () => {
await methods.tombstoneConfigField(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
'mcpServers.github',
10,
);
const result = await methods.unsetConfigField(PrincipalType.ROLE, 'admin', 'mcpServers.github');
expect(result!.tombstones).not.toContain('mcpServers.github');
});
});
describe('deleteConfig', () => {

View file

@ -1,10 +1,23 @@
import { Types } from 'mongoose';
import { PrincipalType, PrincipalModel } from 'librechat-data-provider';
import { BASE_CONFIG_PRINCIPAL_ID } from '~/admin/capabilities';
import { escapeRegExp } from '~/utils/string';
import type { TCustomConfig } from 'librechat-data-provider';
import type { Model, ClientSession } from 'mongoose';
import type { IConfig } from '~/types';
function getTombstonePathsToClear(fieldPath: string): string[] {
const parts = fieldPath.split('.');
if (parts.length <= 1) {
return [fieldPath];
}
return parts.slice(1).map((_, index) => parts.slice(0, index + 2).join('.'));
}
function getPathAndDescendantsRegex(fieldPath: string): RegExp {
return new RegExp(`^${escapeRegExp(fieldPath)}(?:\\.|$)`);
}
export function createConfigMethods(mongoose: typeof import('mongoose')) {
async function findConfigByPrincipal(
principalType: PrincipalType,
@ -139,6 +152,40 @@ export function createConfigMethods(mongoose: typeof import('mongoose')) {
setPayload[`overrides.${path}`] = value;
}
const tombstonesToClear = [...new Set(Object.keys(fields).flatMap(getTombstonePathsToClear))];
const options = {
upsert: true,
new: true,
setDefaultsOnInsert: true,
...(session ? { session } : {}),
};
const update: Record<string, unknown> = {
$set: setPayload,
$inc: { configVersion: 1 },
};
if (tombstonesToClear.length > 0) {
update.$pull = { tombstones: { $in: tombstonesToClear } };
}
return await Config.findOneAndUpdate(
{ principalType, principalId: principalId.toString() },
update,
options,
);
}
async function tombstoneConfigField(
principalType: PrincipalType,
principalId: string | Types.ObjectId,
principalModel: PrincipalModel,
fieldPath: string,
priority: number,
session?: ClientSession,
): Promise<IConfig | null> {
const Config = mongoose.models.Config as Model<IConfig>;
const options = {
upsert: true,
new: true,
@ -148,7 +195,15 @@ export function createConfigMethods(mongoose: typeof import('mongoose')) {
return await Config.findOneAndUpdate(
{ principalType, principalId: principalId.toString() },
{ $set: setPayload, $inc: { configVersion: 1 } },
{
$set: {
principalModel,
priority,
},
$unset: { [`overrides.${fieldPath}`]: '' },
$addToSet: { tombstones: fieldPath },
$inc: { configVersion: 1 },
},
options,
);
}
@ -168,7 +223,11 @@ export function createConfigMethods(mongoose: typeof import('mongoose')) {
return await Config.findOneAndUpdate(
{ principalType, principalId: principalId.toString() },
{ $unset: { [`overrides.${fieldPath}`]: '' }, $inc: { configVersion: 1 } },
{
$unset: { [`overrides.${fieldPath}`]: '' },
$pull: { tombstones: { $regex: getPathAndDescendantsRegex(fieldPath) } },
$inc: { configVersion: 1 },
},
options,
);
}
@ -206,6 +265,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')) {
getApplicableConfigs,
upsertConfig,
patchConfigFields,
tombstoneConfigField,
unsetConfigField,
deleteConfig,
toggleConfigActive,

View file

@ -30,6 +30,10 @@ const configSchema = new Schema<IConfig>(
type: Schema.Types.Mixed,
default: {},
},
tombstones: {
type: [String],
default: [],
},
isActive: {
type: Boolean,
default: true,

View file

@ -18,6 +18,8 @@ export type Config = {
priority: number;
/** Configuration overrides matching librechat.yaml structure */
overrides: Partial<TCustomConfig>;
/** Dot-paths that suppress inherited config values during resolution */
tombstones?: string[];
/** Whether this config override is currently active */
isActive: boolean;
/** Version number for cache invalidation, auto-increments on overrides change */