mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-05 07:17:18 +02:00
🎭 fix: Set Explicit Permission Defaults for USER Role in roleDefaults (#12308)
* fix: set explicit permission defaults for USER role in roleDefaults
Previously several permission types for the USER role had empty
objects in roleDefaults, causing the getPermissionValue fallback to
resolve SHARE/CREATE via the zod schema defaults on fresh installs.
This silently granted users MCP server creation ability and left
share permissions ambiguous.
Sets explicit defaults for all multi-field permission types:
- PROMPTS/AGENTS: USE and CREATE true, SHARE false
- MCP_SERVERS: USE true, CREATE/SHARE false
- REMOTE_AGENTS: all false
Adds regression tests covering the exact reported scenarios (fresh
install with `agents: { use: true }`, restart preserving admin-panel
overrides) and structural guards against future permission schema
expansions missing explicit USER defaults.
Closes #12306.
* fix: guard MCP_SERVERS.CREATE against configDefaults fallback + add migration
The roleDefaults fix alone was insufficient: loadDefaultInterface propagates
configDefaults.mcpServers.create=true as tier-1 in getPermissionValue, overriding
the roleDefault of false. This commit:
- Adds conditional guards for MCP_SERVERS.CREATE and REMOTE_AGENTS.CREATE matching
the existing AGENTS/PROMPTS pattern (only include CREATE when explicitly configured
in yaml OR on fresh install)
- Uses raw interfaceConfig for MCP_SERVERS.CREATE tier-1 instead of loadedInterface
(which includes configDefaults fallback)
- Adds one-time migration backfill: corrects existing MCP_SERVERS.CREATE=true for
USER role in DB when no explicit yaml config is present
- Adds restart-scenario and migration regression tests for MCP_SERVERS
- Cleans up roles.spec.ts: for..of loops, Permissions[] typing, Set for lookups,
removes unnecessary aliases, improves JSDoc for exclusion list
- Fixes misleading test name for agents regression test
- Removes redundant not.toHaveProperty assertions after strict toEqual
* fix: use raw interfaceConfig for REMOTE_AGENTS.CREATE tier-1 (consistency)
Aligns REMOTE_AGENTS.CREATE with the MCP_SERVERS.CREATE fix — reads from
raw interfaceConfig instead of loadedInterface to prevent a future
configDefaults fallback from silently overriding the roleDefault.
This commit is contained in:
Danny Avila
GitHub
parent
9cb5ac63f8
commit
b189972381
4 changed files with 510 additions and 19 deletions
|
|
@ -398,7 +398,7 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[PermissionTypes.FILE_CITATIONS]: { [Permissions.USE]: true },
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
|
|
@ -555,7 +555,7 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[PermissionTypes.FILE_CITATIONS]: { [Permissions.USE]: true },
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
|
|
@ -699,7 +699,7 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[PermissionTypes.FILE_CITATIONS]: { [Permissions.USE]: true },
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
|
|
@ -848,7 +848,7 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[PermissionTypes.FILE_CITATIONS]: { [Permissions.USE]: true },
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
|
|
@ -1002,7 +1002,7 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[PermissionTypes.FILE_CITATIONS]: { [Permissions.USE]: true },
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
|
|
@ -2194,4 +2194,302 @@ describe('updateInterfacePermissions - permissions', () => {
|
|||
[Permissions.SHARE_PUBLIC]: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('should populate all default agent permissions on fresh install with object use config (regression: #12306)', async () => {
|
||||
const config = {
|
||||
interface: {
|
||||
agents: { use: true },
|
||||
},
|
||||
};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
const adminCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.ADMIN,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.AGENTS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
});
|
||||
|
||||
expect(adminCall[1][PermissionTypes.AGENTS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: true,
|
||||
[Permissions.SHARE_PUBLIC]: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should preserve admin-panel changes to USER agents.CREATE across restart (regression: #12306 restart)', async () => {
|
||||
mockGetRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.AGENTS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const config = {
|
||||
interface: {
|
||||
agents: { use: true },
|
||||
},
|
||||
};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.AGENTS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should preserve all admin-panel changes when agents is not in yaml config (regression: #12306 restart)', async () => {
|
||||
mockGetRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.AGENTS]: {
|
||||
[Permissions.USE]: false,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
[PermissionTypes.PROMPTS]: {
|
||||
[Permissions.USE]: false,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const config = {};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
|
||||
expect(userCall[1]).not.toHaveProperty(PermissionTypes.AGENTS);
|
||||
expect(userCall[1]).not.toHaveProperty(PermissionTypes.PROMPTS);
|
||||
});
|
||||
|
||||
it('should not grant USER share for prompts when only use is configured (regression: #12306)', async () => {
|
||||
const config = {
|
||||
interface: {
|
||||
prompts: { use: true },
|
||||
},
|
||||
};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
const adminCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.ADMIN,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.PROMPTS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
});
|
||||
|
||||
expect(adminCall[1][PermissionTypes.PROMPTS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: true,
|
||||
[Permissions.SHARE_PUBLIC]: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should not grant USER create for mcpServers when only use is configured (regression: #12306)', async () => {
|
||||
const config = {
|
||||
interface: {
|
||||
mcpServers: { use: true },
|
||||
},
|
||||
};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
const adminCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.ADMIN,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.MCP_SERVERS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: false,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
});
|
||||
|
||||
expect(adminCall[1][PermissionTypes.MCP_SERVERS]).toEqual({
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: true,
|
||||
[Permissions.SHARE_PUBLIC]: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should preserve existing MCP_SERVERS permissions on restart when mcpServers not in yaml config (regression: #12306 restart)', async () => {
|
||||
mockGetRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const config = {};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.MCP_SERVERS]).toEqual({
|
||||
[Permissions.CREATE]: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('should migrate existing MCP_SERVERS.CREATE=true to false for USER when no explicit config (regression: #12306 migration)', async () => {
|
||||
mockGetRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
[PermissionTypes.AGENTS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const config = {};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
const adminCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.ADMIN,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.MCP_SERVERS]).toEqual({
|
||||
[Permissions.CREATE]: false,
|
||||
});
|
||||
expect(userCall[1]).not.toHaveProperty(PermissionTypes.AGENTS);
|
||||
|
||||
expect(adminCall[1]).not.toHaveProperty(PermissionTypes.MCP_SERVERS);
|
||||
expect(adminCall[1]).not.toHaveProperty(PermissionTypes.AGENTS);
|
||||
});
|
||||
|
||||
it('should NOT migrate MCP_SERVERS.CREATE when yaml explicitly sets create: true (regression: #12306 migration)', async () => {
|
||||
mockGetRoleByName.mockResolvedValue({
|
||||
permissions: {
|
||||
[PermissionTypes.MCP_SERVERS]: {
|
||||
[Permissions.USE]: true,
|
||||
[Permissions.CREATE]: true,
|
||||
[Permissions.SHARE]: false,
|
||||
[Permissions.SHARE_PUBLIC]: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const config = {
|
||||
interface: {
|
||||
mcpServers: { use: true, create: true },
|
||||
},
|
||||
};
|
||||
const configDefaults = { interface: {} } as TConfigDefaults;
|
||||
const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
|
||||
const appConfig = { config, interfaceConfig } as unknown as AppConfig;
|
||||
|
||||
await updateInterfacePermissions({
|
||||
appConfig,
|
||||
getRoleByName: mockGetRoleByName,
|
||||
updateAccessPermissions: mockUpdateAccessPermissions,
|
||||
});
|
||||
|
||||
const userCall = mockUpdateAccessPermissions.mock.calls.find(
|
||||
(call) => call[0] === SystemRoles.USER,
|
||||
);
|
||||
|
||||
expect(userCall[1][PermissionTypes.MCP_SERVERS][Permissions.CREATE]).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -352,11 +352,19 @@ export async function updateInterfacePermissions({
|
|||
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.USE],
|
||||
defaults.mcpServers?.use,
|
||||
),
|
||||
[Permissions.CREATE]: getPermissionValue(
|
||||
loadedInterface.mcpServers?.create,
|
||||
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CREATE],
|
||||
defaults.mcpServers?.create,
|
||||
),
|
||||
...((typeof interfaceConfig?.mcpServers === 'object' &&
|
||||
'create' in interfaceConfig.mcpServers) ||
|
||||
!existingPermissions?.[PermissionTypes.MCP_SERVERS]
|
||||
? {
|
||||
[Permissions.CREATE]: getPermissionValue(
|
||||
typeof interfaceConfig?.mcpServers === 'object'
|
||||
? interfaceConfig.mcpServers.create
|
||||
: undefined,
|
||||
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CREATE],
|
||||
defaults.mcpServers?.create,
|
||||
),
|
||||
}
|
||||
: {}),
|
||||
...((typeof interfaceConfig?.mcpServers === 'object' &&
|
||||
('share' in interfaceConfig.mcpServers || 'public' in interfaceConfig.mcpServers)) ||
|
||||
!existingPermissions?.[PermissionTypes.MCP_SERVERS]
|
||||
|
|
@ -380,11 +388,19 @@ export async function updateInterfacePermissions({
|
|||
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.USE],
|
||||
defaults.remoteAgents?.use,
|
||||
),
|
||||
[Permissions.CREATE]: getPermissionValue(
|
||||
loadedInterface.remoteAgents?.create,
|
||||
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.CREATE],
|
||||
defaults.remoteAgents?.create,
|
||||
),
|
||||
...((typeof interfaceConfig?.remoteAgents === 'object' &&
|
||||
'create' in interfaceConfig.remoteAgents) ||
|
||||
!existingPermissions?.[PermissionTypes.REMOTE_AGENTS]
|
||||
? {
|
||||
[Permissions.CREATE]: getPermissionValue(
|
||||
typeof interfaceConfig?.remoteAgents === 'object'
|
||||
? interfaceConfig.remoteAgents.create
|
||||
: undefined,
|
||||
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.CREATE],
|
||||
defaults.remoteAgents?.create,
|
||||
),
|
||||
}
|
||||
: {}),
|
||||
...((typeof interfaceConfig?.remoteAgents === 'object' &&
|
||||
('share' in interfaceConfig.remoteAgents || 'public' in interfaceConfig.remoteAgents)) ||
|
||||
!existingPermissions?.[PermissionTypes.REMOTE_AGENTS]
|
||||
|
|
@ -511,6 +527,31 @@ export async function updateInterfacePermissions({
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One-time migration: correct MCP_SERVERS.CREATE for USER role.
|
||||
* Before the explicit roleDefaults fix, Zod schema defaults resolved CREATE to true
|
||||
* for all roles. ADMIN should keep CREATE: true, but USER should have CREATE: false
|
||||
* unless explicitly configured otherwise in librechat.yaml.
|
||||
*/
|
||||
if (roleName === SystemRoles.USER) {
|
||||
const existingMcpPerms = existingPermissions?.[PermissionTypes.MCP_SERVERS];
|
||||
const mcpCreateExplicit =
|
||||
typeof interfaceConfig?.mcpServers === 'object' && 'create' in interfaceConfig.mcpServers;
|
||||
if (
|
||||
existingMcpPerms?.[Permissions.CREATE] === true &&
|
||||
!mcpCreateExplicit &&
|
||||
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CREATE] === false
|
||||
) {
|
||||
logger.debug(
|
||||
`Role '${roleName}': Migrating MCP_SERVERS.CREATE from true to false (Zod default correction)`,
|
||||
);
|
||||
permissionsToUpdate[PermissionTypes.MCP_SERVERS] = {
|
||||
...permissionsToUpdate[PermissionTypes.MCP_SERVERS],
|
||||
[Permissions.CREATE]: false,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Update permissions if any need updating
|
||||
if (Object.keys(permissionsToUpdate).length > 0) {
|
||||
await updateAccessPermissions(roleName, permissionsToUpdate, existingRole);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue