🛡️ fix: Preserve CREATE/SHARE/SHARE_PUBLIC Permissions with Boolean Config (#11647)

* 🔧 refactor: Update permissions handling in updateInterfacePermissions function

- Removed explicit SHARE and SHARE_PUBLIC permissions for PROMPTS when prompts are true, simplifying the permission logic.
- Adjusted the permissions structure to conditionally include SHARE and SHARE_PUBLIC based on the type of interface configuration, enhancing maintainability and clarity in permission management.
- Updated related tests to reflect the changes in permission handling for consistency and accuracy.

* 🔧 refactor: Enhance permission configuration in updateInterfacePermissions

- Introduced a new `create` property in the permission configuration object to improve flexibility in permission management.
- Updated helper functions to accommodate the new `create` property, ensuring backward compatibility with existing boolean configurations.
- Adjusted default values for prompts and agents to include the new `create` property, enhancing the overall permission structure.

* 🧪 test: Add regression tests for SHARE/SHARE_PUBLIC permission handling

- Introduced tests to ensure existing SHARE and SHARE_PUBLIC values are preserved when using boolean configuration for agents.
- Added validation to confirm that SHARE and SHARE_PUBLIC are included in the update payload when using object configuration, enhancing the accuracy of permission management.
- These tests address potential regressions and improve the robustness of the permission handling logic in the updateInterfacePermissions function.

* fix: accessing undefined regex

- Moved the creation of the domainSeparatorRegex to the beginning of the loadToolDefinitionsWrapper function for improved clarity and performance.
- Removed redundant regex initialization within the function's loop, enhancing code efficiency and maintainability.

* 🧪 test: Enhance regression tests for SHARE/SHARE_PUBLIC permission handling

- Added a new test to ensure that SHARE and SHARE_PUBLIC permissions are preserved when using object configuration without explicit share/public keys.
- Updated existing tests to validate the inclusion of SHARE and SHARE_PUBLIC in the update payload when using object configuration, improving the robustness of permission management.
- Adjusted the updateInterfacePermissions function to conditionally include SHARE and SHARE_PUBLIC based on the presence of share/public keys in the configuration, enhancing clarity and maintainability.

* 🔧 refactor: Update permission handling in updateInterfacePermissions

- Simplified the logic for including CREATE, SHARE, and SHARE_PUBLIC permissions in the update payload based on the presence of corresponding keys in the configuration object.
- Adjusted tests to reflect the changes, ensuring that only the USE permission is updated when existing permissions are present, preserving the database values for CREATE, SHARE, and SHARE_PUBLIC.
- Enhanced clarity in comments to better explain the permission management logic.
This commit is contained in:
Danny Avila 2026-02-05 15:06:53 +01:00 committed by GitHub
parent 24625f5693
commit 8cf5ae7e79
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 306 additions and 77 deletions

View file

@ -568,6 +568,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
const definitions = [];
const allowedDomains = appConfig?.actions?.allowedDomains;
const domainSeparatorRegex = new RegExp(actionDomainSeparator, 'g');
for (const action of actionSets) {
const domain = await domainParser(action.metadata.domain, true);
@ -590,7 +591,6 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
const { functionSignatures } = openapiToFunction(validationResult.spec, true);
const domainSeparatorRegex = new RegExp(actionDomainSeparator, 'g');
for (const sig of functionSignatures) {
const toolName = `${sig.name}${actionDelimiter}${normalizedDomain}`;
if (!actionToolNames.some((name) => name.replace(domainSeparatorRegex, '_') === toolName)) {

View file

@ -962,9 +962,7 @@ describe('updateInterfacePermissions - permissions', () => {
const expectedPermissionsForUser = {
[PermissionTypes.PROMPTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: false,
[Permissions.SHARE_PUBLIC]: false,
// CREATE/SHARE/SHARE_PUBLIC not included since prompts: true is boolean and PROMPTS already exists
}, // Explicitly configured
// All other permissions that don't exist in the database
[PermissionTypes.MEMORIES]: {
@ -1003,9 +1001,7 @@ describe('updateInterfacePermissions - permissions', () => {
const expectedPermissionsForAdmin = {
[PermissionTypes.PROMPTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true,
[Permissions.SHARE_PUBLIC]: true,
// CREATE/SHARE/SHARE_PUBLIC not included since prompts: true is boolean and PROMPTS already exists
}, // Explicitly configured
// All other permissions that don't exist in the database
[PermissionTypes.MEMORIES]: {
@ -1460,11 +1456,9 @@ describe('updateInterfacePermissions - permissions', () => {
);
// Explicitly configured permissions should be updated
// CREATE/SHARE/SHARE_PUBLIC not included since prompts: true is boolean and PROMPTS already exists
expect(userCall[1][PermissionTypes.PROMPTS]).toEqual({
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: false,
[Permissions.SHARE_PUBLIC]: false,
});
expect(userCall[1][PermissionTypes.BOOKMARKS]).toEqual({ [Permissions.USE]: true });
expect(userCall[1][PermissionTypes.MARKETPLACE]).toEqual({ [Permissions.USE]: true });
@ -1785,12 +1779,9 @@ describe('updateInterfacePermissions - permissions', () => {
);
// Memory permissions should be updated even though they already exist
expect(userCall[1][PermissionTypes.MEMORIES]).toEqual(expectedMemoryPermissions);
// Prompts should be updated (explicitly configured)
// Prompts should be updated (explicitly configured) - CREATE/SHARE/SHARE_PUBLIC not included since prompts: true is boolean and PROMPTS already exists
expect(userCall[1][PermissionTypes.PROMPTS]).toEqual({
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: false,
[Permissions.SHARE_PUBLIC]: false,
});
// Bookmarks should be updated (explicitly configured)
expect(userCall[1][PermissionTypes.BOOKMARKS]).toEqual({ [Permissions.USE]: true });
@ -1801,11 +1792,9 @@ describe('updateInterfacePermissions - permissions', () => {
);
// Memory permissions should be updated even though they already exist
expect(adminCall[1][PermissionTypes.MEMORIES]).toEqual(expectedMemoryPermissions);
// CREATE/SHARE/SHARE_PUBLIC not included since prompts: true is boolean and PROMPTS already exists
expect(adminCall[1][PermissionTypes.PROMPTS]).toEqual({
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true,
[Permissions.SHARE_PUBLIC]: true,
});
expect(adminCall[1][PermissionTypes.BOOKMARKS]).toEqual({ [Permissions.USE]: true });
@ -1821,4 +1810,199 @@ describe('updateInterfacePermissions - permissions', () => {
}),
});
});
it('should preserve existing SHARE/SHARE_PUBLIC values when using boolean config (regression test)', async () => {
// This test ensures that when `agents: true` (boolean) is configured,
// existing SHARE and SHARE_PUBLIC permissions are NOT reset to defaults.
// Mock existing permissions where SHARE and SHARE_PUBLIC were enabled by user
mockGetRoleByName.mockResolvedValue({
permissions: {
[PermissionTypes.AGENTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true, // User enabled this via admin panel
[Permissions.SHARE_PUBLIC]: true, // User enabled this via admin panel
},
[PermissionTypes.PROMPTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true,
[Permissions.SHARE_PUBLIC]: true,
},
},
});
// Config uses boolean (not object), simulating `agents: true` in librechat.yaml
const config = {
interface: {
agents: true, // Boolean config - should only update USE, not reset SHARE/SHARE_PUBLIC
prompts: true, // Boolean config - should only update USE, not reset SHARE/SHARE_PUBLIC
},
};
const configDefaults = {
interface: {
agents: true,
prompts: true,
},
} 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,
);
// CRITICAL: When using boolean config and permissions already exist,
// only USE should be updated. CREATE, SHARE, and SHARE_PUBLIC should NOT be in the update payload.
// This means they will be preserved in the database (not reset to defaults).
expect(userCall[1][PermissionTypes.AGENTS]).toEqual({
[Permissions.USE]: true,
// CREATE, SHARE, and SHARE_PUBLIC intentionally omitted - preserves existing DB values
});
expect(userCall[1][PermissionTypes.AGENTS]).not.toHaveProperty(Permissions.CREATE);
expect(userCall[1][PermissionTypes.AGENTS]).not.toHaveProperty(Permissions.SHARE);
expect(userCall[1][PermissionTypes.AGENTS]).not.toHaveProperty(Permissions.SHARE_PUBLIC);
expect(userCall[1][PermissionTypes.PROMPTS]).toEqual({
[Permissions.USE]: true,
// CREATE, SHARE, and SHARE_PUBLIC intentionally omitted - preserves existing DB values
});
expect(userCall[1][PermissionTypes.PROMPTS]).not.toHaveProperty(Permissions.CREATE);
expect(userCall[1][PermissionTypes.PROMPTS]).not.toHaveProperty(Permissions.SHARE);
expect(userCall[1][PermissionTypes.PROMPTS]).not.toHaveProperty(Permissions.SHARE_PUBLIC);
});
it('should include SHARE/SHARE_PUBLIC when using object config (explicit configuration)', async () => {
// When using object config like `agents: { share: true }`, SHARE/SHARE_PUBLIC SHOULD be updated
mockGetRoleByName.mockResolvedValue({
permissions: {
[PermissionTypes.AGENTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: false,
[Permissions.SHARE_PUBLIC]: false,
},
},
});
const config = {
interface: {
agents: {
use: true,
share: true, // Explicitly setting SHARE
public: true, // Explicitly setting SHARE_PUBLIC
},
},
};
const configDefaults = {
interface: {
agents: {
use: true,
share: false,
public: false,
},
},
} 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,
);
// When object config is used with explicit share/public, they SHOULD be included
expect(userCall[1][PermissionTypes.AGENTS]).toHaveProperty(Permissions.SHARE, true);
expect(userCall[1][PermissionTypes.AGENTS]).toHaveProperty(Permissions.SHARE_PUBLIC, true);
});
it('should preserve SHARE/SHARE_PUBLIC when using object config without share/public keys', async () => {
// When using object config like `agents: { use: true, create: false }` WITHOUT share/public,
// existing SHARE and SHARE_PUBLIC should be preserved (not reset to defaults)
mockGetRoleByName.mockResolvedValue({
permissions: {
[PermissionTypes.AGENTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true, // User enabled this via admin panel
[Permissions.SHARE_PUBLIC]: true, // User enabled this via admin panel
},
[PermissionTypes.PROMPTS]: {
[Permissions.USE]: true,
[Permissions.CREATE]: true,
[Permissions.SHARE]: true,
[Permissions.SHARE_PUBLIC]: true,
},
},
});
const config = {
interface: {
agents: {
use: true,
create: false, // Only setting use and create, NOT share/public
},
prompts: {
use: true,
create: false, // Only setting use and create, NOT share/public
},
},
};
const configDefaults = {
interface: {
agents: {
use: true,
create: true,
share: false,
public: false,
},
prompts: {
use: true,
create: true,
share: false,
public: false,
},
},
} 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,
);
// AGENTS: use and create should be updated, but SHARE/SHARE_PUBLIC should NOT be in payload
expect(userCall[1][PermissionTypes.AGENTS]).toEqual({
[Permissions.USE]: true,
[Permissions.CREATE]: false,
});
expect(userCall[1][PermissionTypes.AGENTS]).not.toHaveProperty(Permissions.SHARE);
expect(userCall[1][PermissionTypes.AGENTS]).not.toHaveProperty(Permissions.SHARE_PUBLIC);
// PROMPTS: same behavior
expect(userCall[1][PermissionTypes.PROMPTS]).toEqual({
[Permissions.USE]: true,
[Permissions.CREATE]: false,
});
expect(userCall[1][PermissionTypes.PROMPTS]).not.toHaveProperty(Permissions.SHARE);
expect(userCall[1][PermissionTypes.PROMPTS]).not.toHaveProperty(Permissions.SHARE_PUBLIC);
});
});

View file

@ -146,21 +146,28 @@ export async function updateInterfacePermissions({
};
// Helper to extract value from boolean or object config
const getConfigUse = (
config: boolean | { use?: boolean; share?: boolean; public?: boolean } | undefined,
) => (typeof config === 'boolean' ? config : config?.use);
const getConfigShare = (
config: boolean | { use?: boolean; share?: boolean; public?: boolean } | undefined,
) => (typeof config === 'boolean' ? undefined : config?.share);
const getConfigPublic = (
config: boolean | { use?: boolean; share?: boolean; public?: boolean } | undefined,
) => (typeof config === 'boolean' ? undefined : config?.public);
type PermissionConfig =
| boolean
| { use?: boolean; create?: boolean; share?: boolean; public?: boolean }
| undefined;
const getConfigUse = (config: PermissionConfig) =>
typeof config === 'boolean' ? config : config?.use;
const getConfigCreate = (config: PermissionConfig) =>
typeof config === 'boolean' ? undefined : config?.create;
const getConfigShare = (config: PermissionConfig) =>
typeof config === 'boolean' ? undefined : config?.share;
const getConfigPublic = (config: PermissionConfig) =>
typeof config === 'boolean' ? undefined : config?.public;
// Get default use values (for backward compat when config is boolean)
// Get default values (for backward compat when config is boolean)
const promptsDefaultUse =
typeof defaults.prompts === 'boolean' ? defaults.prompts : defaults.prompts?.use;
const agentsDefaultUse =
typeof defaults.agents === 'boolean' ? defaults.agents : defaults.agents?.use;
const promptsDefaultCreate =
typeof defaults.prompts === 'object' ? defaults.prompts?.create : undefined;
const agentsDefaultCreate =
typeof defaults.agents === 'object' ? defaults.agents?.create : undefined;
const promptsDefaultShare =
typeof defaults.prompts === 'object' ? defaults.prompts?.share : undefined;
const agentsDefaultShare =
@ -177,21 +184,32 @@ export async function updateInterfacePermissions({
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.USE],
promptsDefaultUse,
),
[Permissions.CREATE]: getPermissionValue(
undefined,
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.CREATE],
true,
),
[Permissions.SHARE]: getPermissionValue(
getConfigShare(loadedInterface.prompts),
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.SHARE],
promptsDefaultShare,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
getConfigPublic(loadedInterface.prompts),
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.SHARE_PUBLIC],
promptsDefaultPublic,
),
...((typeof interfaceConfig?.prompts === 'object' && 'create' in interfaceConfig.prompts) ||
!existingPermissions?.[PermissionTypes.PROMPTS]
? {
[Permissions.CREATE]: getPermissionValue(
getConfigCreate(loadedInterface.prompts),
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.CREATE],
promptsDefaultCreate ?? true,
),
}
: {}),
...((typeof interfaceConfig?.prompts === 'object' &&
('share' in interfaceConfig.prompts || 'public' in interfaceConfig.prompts)) ||
!existingPermissions?.[PermissionTypes.PROMPTS]
? {
[Permissions.SHARE]: getPermissionValue(
getConfigShare(loadedInterface.prompts),
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.SHARE],
promptsDefaultShare,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
getConfigPublic(loadedInterface.prompts),
defaultPerms[PermissionTypes.PROMPTS]?.[Permissions.SHARE_PUBLIC],
promptsDefaultPublic,
),
}
: {}),
},
[PermissionTypes.BOOKMARKS]: {
[Permissions.USE]: getPermissionValue(
@ -242,21 +260,32 @@ export async function updateInterfacePermissions({
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.USE],
agentsDefaultUse,
),
[Permissions.CREATE]: getPermissionValue(
undefined,
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.CREATE],
true,
),
[Permissions.SHARE]: getPermissionValue(
getConfigShare(loadedInterface.agents),
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.SHARE],
agentsDefaultShare,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
getConfigPublic(loadedInterface.agents),
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.SHARE_PUBLIC],
agentsDefaultPublic,
),
...((typeof interfaceConfig?.agents === 'object' && 'create' in interfaceConfig.agents) ||
!existingPermissions?.[PermissionTypes.AGENTS]
? {
[Permissions.CREATE]: getPermissionValue(
getConfigCreate(loadedInterface.agents),
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.CREATE],
agentsDefaultCreate ?? true,
),
}
: {}),
...((typeof interfaceConfig?.agents === 'object' &&
('share' in interfaceConfig.agents || 'public' in interfaceConfig.agents)) ||
!existingPermissions?.[PermissionTypes.AGENTS]
? {
[Permissions.SHARE]: getPermissionValue(
getConfigShare(loadedInterface.agents),
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.SHARE],
agentsDefaultShare,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
getConfigPublic(loadedInterface.agents),
defaultPerms[PermissionTypes.AGENTS]?.[Permissions.SHARE_PUBLIC],
agentsDefaultPublic,
),
}
: {}),
},
[PermissionTypes.TEMPORARY_CHAT]: {
[Permissions.USE]: getPermissionValue(
@ -328,16 +357,22 @@ export async function updateInterfacePermissions({
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CREATE],
defaults.mcpServers?.create,
),
[Permissions.SHARE]: getPermissionValue(
loadedInterface.mcpServers?.share,
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.SHARE],
defaults.mcpServers?.share,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
loadedInterface.mcpServers?.public,
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.SHARE_PUBLIC],
defaults.mcpServers?.public,
),
...((typeof interfaceConfig?.mcpServers === 'object' &&
('share' in interfaceConfig.mcpServers || 'public' in interfaceConfig.mcpServers)) ||
!existingPermissions?.[PermissionTypes.MCP_SERVERS]
? {
[Permissions.SHARE]: getPermissionValue(
loadedInterface.mcpServers?.share,
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.SHARE],
defaults.mcpServers?.share,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
loadedInterface.mcpServers?.public,
defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.SHARE_PUBLIC],
defaults.mcpServers?.public,
),
}
: {}),
},
[PermissionTypes.REMOTE_AGENTS]: {
[Permissions.USE]: getPermissionValue(
@ -350,16 +385,22 @@ export async function updateInterfacePermissions({
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.CREATE],
defaults.remoteAgents?.create,
),
[Permissions.SHARE]: getPermissionValue(
loadedInterface.remoteAgents?.share,
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.SHARE],
defaults.remoteAgents?.share,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
loadedInterface.remoteAgents?.public,
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.SHARE_PUBLIC],
defaults.remoteAgents?.public,
),
...((typeof interfaceConfig?.remoteAgents === 'object' &&
('share' in interfaceConfig.remoteAgents || 'public' in interfaceConfig.remoteAgents)) ||
!existingPermissions?.[PermissionTypes.REMOTE_AGENTS]
? {
[Permissions.SHARE]: getPermissionValue(
loadedInterface.remoteAgents?.share,
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.SHARE],
defaults.remoteAgents?.share,
),
[Permissions.SHARE_PUBLIC]: getPermissionValue(
loadedInterface.remoteAgents?.public,
defaultPerms[PermissionTypes.REMOTE_AGENTS]?.[Permissions.SHARE_PUBLIC],
defaults.remoteAgents?.public,
),
}
: {}),
},
};

View file

@ -628,6 +628,7 @@ export const interfaceSchema = z
z.boolean(),
z.object({
use: z.boolean().optional(),
create: z.boolean().optional(),
share: z.boolean().optional(),
public: z.boolean().optional(),
}),
@ -638,6 +639,7 @@ export const interfaceSchema = z
z.boolean(),
z.object({
use: z.boolean().optional(),
create: z.boolean().optional(),
share: z.boolean().optional(),
public: z.boolean().optional(),
}),
@ -681,11 +683,13 @@ export const interfaceSchema = z
memories: true,
prompts: {
use: true,
create: true,
share: false,
public: false,
},
agents: {
use: true,
create: true,
share: false,
public: false,
},