diff --git a/packages/api/src/app/permissions.spec.ts b/packages/api/src/app/permissions.spec.ts index 7ab7e0d0d1..106ebbb50b 100644 --- a/packages/api/src/app/permissions.spec.ts +++ b/packages/api/src/app/permissions.spec.ts @@ -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); + }); }); diff --git a/packages/api/src/app/permissions.ts b/packages/api/src/app/permissions.ts index 3638bdc0bb..5a557adfcf 100644 --- a/packages/api/src/app/permissions.ts +++ b/packages/api/src/app/permissions.ts @@ -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); diff --git a/packages/data-provider/src/roles.spec.ts b/packages/data-provider/src/roles.spec.ts new file mode 100644 index 0000000000..60dac5ab50 --- /dev/null +++ b/packages/data-provider/src/roles.spec.ts @@ -0,0 +1,132 @@ +import { Permissions, PermissionTypes, permissionsSchema } from './permissions'; +import { SystemRoles, roleDefaults } from './roles'; + +const RESOURCE_MANAGEMENT_FIELDS: Permissions[] = [ + Permissions.CREATE, + Permissions.SHARE, + Permissions.SHARE_PUBLIC, +]; + +/** + * Permission types where CREATE/SHARE/SHARE_PUBLIC must default to false for USER. + * MEMORIES is excluded: its CREATE/READ/UPDATE apply to the user's own private data. + * AGENTS/PROMPTS are excluded: CREATE=true is intentional (users own their agents/prompts). + * Add new types here if they gate shared/multi-user resources. + */ +const RESOURCE_PERMISSION_TYPES: PermissionTypes[] = [ + PermissionTypes.MCP_SERVERS, + PermissionTypes.REMOTE_AGENTS, +]; + +describe('roleDefaults', () => { + describe('USER role', () => { + const userPerms = roleDefaults[SystemRoles.USER].permissions; + + it('should have explicit values for every field in every multi-field permission type', () => { + const schemaShape = permissionsSchema.shape; + + for (const [permType, subSchema] of Object.entries(schemaShape)) { + const fieldNames = Object.keys(subSchema.shape); + if (fieldNames.length <= 1) { + continue; + } + + const userValues = + userPerms[permType as PermissionTypes] as Record; + + for (const field of fieldNames) { + expect({ + permType, + field, + value: userValues[field], + }).toEqual( + expect.objectContaining({ + permType, + field, + value: expect.any(Boolean), + }), + ); + } + } + }); + + it('should never grant CREATE, SHARE, or SHARE_PUBLIC by default for resource-management types', () => { + for (const permType of RESOURCE_PERMISSION_TYPES) { + const permissions = userPerms[permType] as Record; + for (const field of RESOURCE_MANAGEMENT_FIELDS) { + if (permissions[field] === undefined) { + continue; + } + expect({ + permType, + field, + value: permissions[field], + }).toEqual( + expect.objectContaining({ + permType, + field, + value: false, + }), + ); + } + } + }); + + it('should cover every permission type that has CREATE, SHARE, or SHARE_PUBLIC fields', () => { + const schemaShape = permissionsSchema.shape; + const restrictedSet = new Set(RESOURCE_PERMISSION_TYPES); + + for (const [permType, subSchema] of Object.entries(schemaShape)) { + const fieldNames = Object.keys(subSchema.shape); + const hasResourceFields = fieldNames.some((f) => RESOURCE_MANAGEMENT_FIELDS.includes(f as Permissions)); + if (!hasResourceFields) { + continue; + } + + const isTracked = + restrictedSet.has(permType) || + permType === PermissionTypes.MEMORIES || + permType === PermissionTypes.PROMPTS || + permType === PermissionTypes.AGENTS; + + expect({ + permType, + tracked: isTracked, + }).toEqual( + expect.objectContaining({ + permType, + tracked: true, + }), + ); + } + }); + }); + + describe('ADMIN role', () => { + const adminPerms = roleDefaults[SystemRoles.ADMIN].permissions; + + it('should have explicit values for every field in every permission type', () => { + const schemaShape = permissionsSchema.shape; + + for (const [permType, subSchema] of Object.entries(schemaShape)) { + const fieldNames = Object.keys(subSchema.shape); + const adminValues = + adminPerms[permType as PermissionTypes] as Record; + + for (const field of fieldNames) { + expect({ + permType, + field, + value: adminValues[field], + }).toEqual( + expect.objectContaining({ + permType, + field, + value: expect.any(Boolean), + }), + ); + } + } + }); + }); +}); diff --git a/packages/data-provider/src/roles.ts b/packages/data-provider/src/roles.ts index b494ee5817..1ba7a8cce2 100644 --- a/packages/data-provider/src/roles.ts +++ b/packages/data-provider/src/roles.ts @@ -180,10 +180,20 @@ export const roleDefaults = defaultRolesSchema.parse({ [SystemRoles.USER]: { name: SystemRoles.USER, permissions: { - [PermissionTypes.PROMPTS]: {}, + [PermissionTypes.PROMPTS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, [PermissionTypes.BOOKMARKS]: {}, [PermissionTypes.MEMORIES]: {}, - [PermissionTypes.AGENTS]: {}, + [PermissionTypes.AGENTS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, [PermissionTypes.MULTI_CONVO]: {}, [PermissionTypes.TEMPORARY_CHAT]: {}, [PermissionTypes.RUN_CODE]: {}, @@ -198,8 +208,18 @@ export const roleDefaults = defaultRolesSchema.parse({ }, [PermissionTypes.FILE_SEARCH]: {}, [PermissionTypes.FILE_CITATIONS]: {}, - [PermissionTypes.MCP_SERVERS]: {}, - [PermissionTypes.REMOTE_AGENTS]: {}, + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: false, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, + [PermissionTypes.REMOTE_AGENTS]: { + [Permissions.USE]: false, + [Permissions.CREATE]: false, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, }, }, });