From f4904cbac6d578d26e5acfb01b37c5c370eda159 Mon Sep 17 00:00:00 2001 From: Atef Bellaaj Date: Mon, 12 Oct 2026 12:24:30 +0000 Subject: [PATCH] feat(prompts): make external prompt groups read-only Reject edits, deletes, new revisions, promotion and revision deletes on prompt groups whose source is not native with a read_only service error, mapped to 403 "Externally managed prompts are read-only". The guard reads the stored group, so the MANAGE_PROMPTS bypass is covered. Sharing, usage counts and resolution are unchanged. The client hides Edit and Delete for external groups, shows a read-only detail view with a Langfuse notice and keeps Share. --- api/server/routes/prompts.test.js | 248 +++++++++++++++++- .../Prompts/dialogs/DeletePrompt.tsx | 13 +- .../dialogs/__tests__/DeletePrompt.spec.tsx | 97 +++++++ .../Prompts/fields/CategorySelector.tsx | 14 +- .../components/Prompts/fields/PromptName.tsx | 20 +- .../__tests__/CategorySelector.spec.tsx | 66 +++++ .../fields/__tests__/PromptName.spec.tsx | 39 +++ .../components/Prompts/forms/PromptForm.tsx | 122 +++++++-- .../forms/__tests__/PromptForm.spec.tsx | 205 +++++++++++++++ .../Prompts/lists/ChatGroupItem.tsx | 40 ++- .../lists/__tests__/ChatGroupItem.spec.tsx | 95 +++++++ .../usePromptGroupPermissions.test.ts | 86 ++++++ client/src/hooks/Prompts/index.ts | 2 + .../Prompts/usePromptGroupPermissions.ts | 39 +++ client/src/locales/en/translation.json | 3 + .../src/utils/__tests__/promptGroups.test.ts | 75 +++++- client/src/utils/__tests__/prompts.test.ts | 32 +++ client/src/utils/promptGroups.ts | 45 +++- client/src/utils/prompts.ts | 16 ++ eslint-suppressions.json | 2 +- packages/api/src/prompts/handlers.spec.ts | 205 +++++++++++---- packages/api/src/prompts/handlers.ts | 39 +-- packages/api/src/prompts/service.spec.ts | 203 +++++++++++++- packages/api/src/prompts/service.ts | 89 ++++++- packages/api/src/prompts/types.ts | 17 +- 25 files changed, 1681 insertions(+), 131 deletions(-) create mode 100644 client/src/components/Prompts/dialogs/__tests__/DeletePrompt.spec.tsx create mode 100644 client/src/components/Prompts/fields/__tests__/CategorySelector.spec.tsx create mode 100644 client/src/components/Prompts/fields/__tests__/PromptName.spec.tsx create mode 100644 client/src/components/Prompts/forms/__tests__/PromptForm.spec.tsx create mode 100644 client/src/components/Prompts/lists/__tests__/ChatGroupItem.spec.tsx create mode 100644 client/src/hooks/Prompts/__tests__/usePromptGroupPermissions.test.ts create mode 100644 client/src/hooks/Prompts/usePromptGroupPermissions.ts create mode 100644 client/src/utils/__tests__/prompts.test.ts diff --git a/api/server/routes/prompts.test.js b/api/server/routes/prompts.test.js index 047c4fc0a90..ad768c6d631 100644 --- a/api/server/routes/prompts.test.js +++ b/api/server/routes/prompts.test.js @@ -49,6 +49,9 @@ jest.mock('~/server/middleware', () => ({ canAccessPromptViaGroup: jest.requireActual('~/server/middleware').canAccessPromptViaGroup, canAccessPromptGroupResource: jest.requireActual('~/server/middleware').canAccessPromptGroupResource, + checkBan: (_req, _res, next) => next(), + uaParser: (_req, _res, next) => next(), + canAccessResource: jest.requireActual('~/server/middleware').canAccessResource, })); const { configMiddleware } = require('~/server/middleware'); @@ -56,6 +59,7 @@ const { configMiddleware } = require('~/server/middleware'); let app; let mongoServer; let promptRoutes; +let accessPermissionsRoutes; let Prompt, PromptGroup, AclEntry, AccessRole, User, SystemGrant, Config; let testUsers, testRoles; let grantPermission; @@ -114,6 +118,11 @@ beforeAll(async () => { // Import routes after middleware is set up promptRoutes = require('./prompts'); app.use('/api/prompts', promptRoutes); + + // Mounted so the sharing test below can call the real access-permissions router + // (and its real `updateResourcePermissions` controller) instead of the service layer. + accessPermissionsRoutes = require('./accessPermissions'); + app.use('/api/permissions', accessPermissionsRoutes); }); afterEach(() => { @@ -204,7 +213,7 @@ async function setupTestData() { getRoleByName.mockImplementation((roleName) => { switch (roleName) { case SystemRoles.USER: - return { permissions: { PROMPTS: { USE: true, CREATE: true } } }; + return { permissions: { PROMPTS: { USE: true, CREATE: true, SHARE: true } } }; case SystemRoles.ADMIN: return { permissions: { PROMPTS: { USE: true, CREATE: true, SHARE: true } } }; default: @@ -1628,6 +1637,243 @@ describe('GET /api/prompts/groups/:groupId/resolve', () => { }); }); +describe('Externally managed (Langfuse) prompt groups are read-only', () => { + const readOnlyBody = { code: 'read_only', error: 'Externally managed prompts are read-only' }; + let group; + + async function seedLangfuseGroup(owner = testUsers.owner) { + const langfuseGroup = await PromptGroup.create({ + name: 'Read-only Langfuse group', + category: 'langfuse-read-only', + author: owner._id, + authorName: owner.name, + source: 'langfuse', + sourcePromptName: 'greeting', + sourceProjectId: 'ro-project', + sourceDestination: 'env', + }); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: langfuseGroup._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: owner._id, + }); + return langfuseGroup; + } + + beforeEach(async () => { + group = await seedLangfuseGroup(); + }); + + afterEach(async () => { + await Prompt.deleteMany({}); + await PromptGroup.deleteMany({}); + await AclEntry.deleteMany({}); + }); + + it('rejects PATCH /groups/:groupId with 403 and leaves the group unchanged', async () => { + const before = await PromptGroup.findById(group._id).lean(); + + const response = await request(app) + .patch(`/api/prompts/groups/${group._id}`) + .send({ name: 'Hacked name' }) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + const after = await PromptGroup.findById(group._id).lean(); + expect(after).toEqual(before); + }); + + it('rejects DELETE /groups/:groupId with 403 and leaves the group in place', async () => { + const response = await request(app).delete(`/api/prompts/groups/${group._id}`).expect(403); + + expect(response.body).toEqual(readOnlyBody); + await expect(PromptGroup.findById(group._id)).resolves.not.toBeNull(); + const ownerEntry = await AclEntry.findOne({ + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + }); + expect(ownerEntry).toBeTruthy(); + }); + + it('rejects POST /groups/:groupId/prompts (new revision) with 403 and adds no revision', async () => { + const response = await request(app) + .post(`/api/prompts/groups/${group._id}/prompts`) + .send({ prompt: { prompt: 'New text', type: 'text' } }) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + await expect(Prompt.countDocuments({ groupId: group._id })).resolves.toBe(0); + }); + + it('rejects PATCH /groups/:groupId for an admin too (MANAGE_PROMPTS ACL bypass still hits the service guard)', async () => { + setTestUser(app, testUsers.admin); + const before = await PromptGroup.findById(group._id).lean(); + + const response = await request(app) + .patch(`/api/prompts/groups/${group._id}`) + .send({ name: 'Admin hack' }) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + const after = await PromptGroup.findById(group._id).lean(); + expect(after).toEqual(before); + }); + + it('rejects DELETE /groups/:groupId for an admin too (MANAGE_PROMPTS ACL bypass still hits the service guard)', async () => { + setTestUser(app, testUsers.admin); + + const response = await request(app).delete(`/api/prompts/groups/${group._id}`).expect(403); + + expect(response.body).toEqual(readOnlyBody); + await expect(PromptGroup.findById(group._id)).resolves.not.toBeNull(); + }); + + describe('with a Prompt row inserted directly for the group', () => { + let revision; + + beforeEach(async () => { + revision = await Prompt.create({ + prompt: 'Synced from Langfuse', + author: testUsers.owner._id, + type: 'text', + groupId: group._id, + }); + }); + + it('rejects PATCH /:promptId/tags/production with 403 and does not promote', async () => { + const response = await request(app) + .patch(`/api/prompts/${revision._id}/tags/production`) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + const after = await PromptGroup.findById(group._id).lean(); + expect(after.productionId ?? null).toBeNull(); + await expect(Prompt.findById(revision._id)).resolves.not.toBeNull(); + }); + + it('rejects DELETE /:promptId with 403 and keeps the revision', async () => { + const response = await request(app) + .delete(`/api/prompts/${revision._id}`) + .query({ groupId: group._id.toString() }) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + await expect(Prompt.findById(revision._id)).resolves.not.toBeNull(); + await expect(PromptGroup.findById(group._id)).resolves.not.toBeNull(); + }); + + it('rejects DELETE /:promptId for an admin too (MANAGE_PROMPTS ACL bypass still hits the service guard)', async () => { + setTestUser(app, testUsers.admin); + + const response = await request(app) + .delete(`/api/prompts/${revision._id}`) + .query({ groupId: group._id.toString() }) + .expect(403); + + expect(response.body).toEqual(readOnlyBody); + await expect(Prompt.findById(revision._id)).resolves.not.toBeNull(); + }); + }); + + it('keeps POST /groups/:groupId/use working: it increments numberOfGenerations', async () => { + const response = await request(app).post(`/api/prompts/groups/${group._id}/use`).expect(200); + + expect(response.body).toEqual({ numberOfGenerations: 1 }); + const after = await PromptGroup.findById(group._id).lean(); + expect(after.numberOfGenerations).toBe(1); + }); + + it('keeps sharing working through the access-permissions routes', async () => { + setTestUser(app, testUsers.owner); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.PROMPTGROUP}/${group._id}`) + .send({ + updated: [ + { + type: PrincipalType.USER, + id: testUsers.viewer._id.toString(), + accessRoleId: AccessRoleIds.PROMPTGROUP_VIEWER, + }, + ], + }) + .expect(200); + + expect(response.body.message).toBe('Permissions updated successfully'); + + const sharedEntry = await AclEntry.findOne({ + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + principalType: PrincipalType.USER, + principalId: testUsers.viewer._id, + }); + expect(sharedEntry).toBeTruthy(); + + // Sharing an externally managed group only grants an ACL entry; it never touches + // the read-only group document itself. + const after = await PromptGroup.findById(group._id).lean(); + expect(after.name).toBe(group.name); + }); +}); + +describe('A stored prompt group with no source (predates the field) stays editable', () => { + let group; + + beforeEach(async () => { + // Bypasses the schema's `source: 'native'` default to reproduce a group + // written before the field existed. + const inserted = await PromptGroup.collection.insertOne({ + name: 'Pre-source Group', + category: 'legacy-no-source', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + }); + group = { _id: inserted.insertedId }; + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + }); + + afterEach(async () => { + await Prompt.deleteMany({}); + await PromptGroup.deleteMany({}); + await AclEntry.deleteMany({}); + }); + + it('allows PATCH /groups/:groupId to rename it, same as a native group', async () => { + const response = await request(app) + .patch(`/api/prompts/groups/${group._id}`) + .send({ name: 'Renamed legacy group' }) + .expect(200); + + expect(response.body.name).toBe('Renamed legacy group'); + const stored = await PromptGroup.findById(group._id).lean(); + expect(stored.name).toBe('Renamed legacy group'); + }); + + it('allows POST /groups/:groupId/prompts to add a text prompt, same as a native group', async () => { + const response = await request(app) + .post(`/api/prompts/groups/${group._id}/prompts`) + .send({ prompt: { prompt: 'New legacy prompt text', type: 'text' } }) + .expect(200); + + expect(response.body.prompt.prompt).toBe('New legacy prompt text'); + await expect(Prompt.countDocuments({ groupId: group._id })).resolves.toBe(1); + }); +}); + describe('Prompt Routes - response and failure compatibility', () => { let consoleErrorSpy; let group; diff --git a/client/src/components/Prompts/dialogs/DeletePrompt.tsx b/client/src/components/Prompts/dialogs/DeletePrompt.tsx index 90333fab11d..2bef2102c52 100644 --- a/client/src/components/Prompts/dialogs/DeletePrompt.tsx +++ b/client/src/components/Prompts/dialogs/DeletePrompt.tsx @@ -6,7 +6,9 @@ import { OGDialogTrigger, TooltipAnchor, OGDialogTemplate, + useToastContext, } from '@librechat/client'; +import { getPromptMutationErrorKey } from '~/utils'; import { useDeletePrompt } from '~/data-provider'; import { useLocalize } from '~/hooks'; @@ -75,7 +77,16 @@ interface DeletePromptProps { const DeletePrompt = React.memo( ({ promptId, groupId, promptName, disabled }: DeletePromptProps) => { - const deletePromptMutation = useDeletePrompt(); + const localize = useLocalize(); + const { showToast } = useToastContext(); + const deletePromptMutation = useDeletePrompt({ + onError: (error) => { + showToast({ + status: 'error', + message: localize(getPromptMutationErrorKey(error, 'com_ui_prompt_delete_error')), + }); + }, + }); const handleDelete = useCallback(() => { if (!promptId) { diff --git a/client/src/components/Prompts/dialogs/__tests__/DeletePrompt.spec.tsx b/client/src/components/Prompts/dialogs/__tests__/DeletePrompt.spec.tsx new file mode 100644 index 00000000000..175ec5f31c4 --- /dev/null +++ b/client/src/components/Prompts/dialogs/__tests__/DeletePrompt.spec.tsx @@ -0,0 +1,97 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import DeletePrompt from '../DeletePrompt'; + +const mockShowToast = jest.fn(); +const mockMutate = jest.fn(); +let capturedOnError: ((error: unknown) => void) | undefined; + +jest.mock('~/data-provider', () => ({ + useDeletePrompt: (options?: { onError?: (error: unknown) => void }) => { + capturedOnError = options?.onError; + return { mutate: mockMutate, isLoading: false }; + }, +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('@librechat/client', () => { + const React = jest.requireActual('react'); + return { + Button: ({ children, ...rest }: any) => React.createElement('button', rest, children), + TooltipAnchor: ({ render }: { render: React.ReactNode }) => render, + OGDialog: ({ children }: any) => React.createElement(React.Fragment, null, children), + OGDialogTrigger: ({ children }: any) => children, + OGDialogTemplate: ({ selection }: any) => + React.createElement( + 'button', + { 'data-testid': 'delete-confirm-button', onClick: selection?.selectHandler }, + selection?.selectText ?? 'Confirm', + ), + useToastContext: () => ({ showToast: mockShowToast }), + }; +}); + +describe('DeletePrompt', () => { + beforeEach(() => { + mockShowToast.mockClear(); + mockMutate.mockClear(); + capturedOnError = undefined; + }); + + it('deletes the revision on confirm', () => { + render( + , + ); + + fireEvent.click(screen.getByTestId('delete-confirm-button')); + + expect(mockMutate).toHaveBeenCalledWith({ _id: 'prompt-1', groupId: 'group-1' }); + }); + + it('shows the externally-managed message when the server rejects a read-only revision delete', () => { + render( + , + ); + + expect(capturedOnError).toBeDefined(); + capturedOnError?.({ + isAxiosError: true, + response: { status: 403, data: { code: 'read_only' } }, + }); + + expect(mockShowToast).toHaveBeenCalledWith({ + status: 'error', + message: 'com_ui_prompt_read_only_error', + }); + }); + + it('falls back to the generic delete-error message for a non-read-only failure', () => { + render( + , + ); + + capturedOnError?.({ isAxiosError: true, response: { status: 500, data: {} } }); + + expect(mockShowToast).toHaveBeenCalledWith({ + status: 'error', + message: 'com_ui_prompt_delete_error', + }); + }); +}); diff --git a/client/src/components/Prompts/fields/CategorySelector.tsx b/client/src/components/Prompts/fields/CategorySelector.tsx index b0d163d21bc..9db87de454b 100644 --- a/client/src/components/Prompts/fields/CategorySelector.tsx +++ b/client/src/components/Prompts/fields/CategorySelector.tsx @@ -1,9 +1,9 @@ import React, { useId, useMemo, useState } from 'react'; import * as Ariakit from '@ariakit/react'; import { useTranslation } from 'react-i18next'; -import { DropdownPopup } from '@librechat/client'; import { LocalStorageKeys } from 'librechat-data-provider'; import { useFormContext, Controller } from 'react-hook-form'; +import { DropdownPopup, disabledFillClasses } from '@librechat/client'; import type { MenuItemProps } from '@librechat/client'; import type { ReactNode } from 'react'; import { usePromptGroupsContext } from '~/Providers'; @@ -16,6 +16,8 @@ interface CategorySelectorProps { className?: string; /** Portaled menus are unclickable inside a modal dialog, which locks pointer events on the body */ portal?: boolean; + /** Disables the trigger for a read-only group */ + disabled?: boolean; } const CategorySelector: React.FC = ({ @@ -23,6 +25,7 @@ const CategorySelector: React.FC = ({ onValueChange, className = '', portal = true, + disabled = false, }) => { const instanceId = useId(); const menuId = `${instanceId}-category-menu`; @@ -79,11 +82,14 @@ const CategorySelector: React.FC = ({ const trigger = ( setIsOpen(!isOpen)} + disabled={disabled} + onClick={() => !disabled && setIsOpen(!isOpen)} aria-label={t('com_ui_prompt_category_selector_aria')} >
@@ -92,7 +98,7 @@ const CategorySelector: React.FC = ({ )} {displayCategory.value ? displayCategory.label : t('com_ui_category')}
- + {!disabled && } ); diff --git a/client/src/components/Prompts/fields/PromptName.tsx b/client/src/components/Prompts/fields/PromptName.tsx index 8a4d809f6c8..beabaa95d21 100644 --- a/client/src/components/Prompts/fields/PromptName.tsx +++ b/client/src/components/Prompts/fields/PromptName.tsx @@ -7,12 +7,20 @@ type Props = { name?: string; isLoading?: boolean; isError?: boolean; + /** Hides the edit affordance and keeps the name read-only */ + disabled?: boolean; onSave: (newName: string) => void; }; type SaveStatus = 'idle' | 'saving' | 'saved' | 'error'; -const PromptName: React.FC = ({ name, isLoading = false, isError = false, onSave }) => { +const PromptName: React.FC = ({ + name, + isLoading = false, + isError = false, + disabled = false, + onSave, +}) => { const localize = useLocalize(); const inputRef = useRef(null); const wasLoadingRef = useRef(false); @@ -99,7 +107,11 @@ const PromptName: React.FC = ({ name, isLoading = false, isError = false, }; }, []); - const displayButton = ( + const displayButton = disabled ? ( + + {newName} + + ) : (