diff --git a/.github/workflows/backend-review.yml b/.github/workflows/backend-review.yml index 9dd3905c0e..03b7c135d2 100644 --- a/.github/workflows/backend-review.yml +++ b/.github/workflows/backend-review.yml @@ -1,11 +1,6 @@ name: Backend Unit Tests on: pull_request: - branches: - - main - - dev - - dev-staging - - release/* paths: - 'api/**' - 'packages/**' diff --git a/.github/workflows/frontend-review.yml b/.github/workflows/frontend-review.yml index 9c2d4a37b1..05b3f4154f 100644 --- a/.github/workflows/frontend-review.yml +++ b/.github/workflows/frontend-review.yml @@ -2,11 +2,6 @@ name: Frontend Unit Tests on: pull_request: - branches: - - main - - dev - - dev-staging - - release/* paths: - 'client/**' - 'packages/data-provider/**' diff --git a/api/server/routes/roles.js b/api/server/routes/roles.js index 456d144af1..feed23f8dd 100644 --- a/api/server/routes/roles.js +++ b/api/server/routes/roles.js @@ -11,6 +11,7 @@ const { marketplacePermissionsSchema, peoplePickerPermissionsSchema, remoteAgentsPermissionsSchema, + skillPermissionsSchema, } = require('librechat-data-provider'); const { hasCapability, requireCapability } = require('~/server/middleware/roles/capabilities'); const { updateRoleByName, getRoleByName } = require('~/models'); @@ -60,6 +61,11 @@ const permissionConfigs = { permissionType: PermissionTypes.REMOTE_AGENTS, errorMessage: 'Invalid remote agents permissions.', }, + skills: { + schema: skillPermissionsSchema, + permissionType: PermissionTypes.SKILLS, + errorMessage: 'Invalid skill permissions.', + }, }; /** @@ -177,4 +183,10 @@ router.put('/:roleName/marketplace', manageRoles, createPermissionUpdateHandler( */ router.put('/:roleName/remote-agents', manageRoles, createPermissionUpdateHandler('remote-agents')); +/** + * PUT /api/roles/:roleName/skills + * Update skill permissions for a specific role + */ +router.put('/:roleName/skills', manageRoles, createPermissionUpdateHandler('skills')); + module.exports = router; diff --git a/api/server/routes/skills.js b/api/server/routes/skills.js index 38d4b38ca5..9e9676fb45 100644 --- a/api/server/routes/skills.js +++ b/api/server/routes/skills.js @@ -1,7 +1,19 @@ +const path = require('path'); +const crypto = require('crypto'); +const multer = require('multer'); const express = require('express'); -const { createSkillsHandlers, generateCheckAccess } = require('@librechat/api'); -const { isValidObjectIdString } = require('@librechat/data-schemas'); -const { PermissionBits, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { + createSkillsHandlers, + createImportHandler, + generateCheckAccess, +} = require('@librechat/api'); +const { isValidObjectIdString, logger } = require('@librechat/data-schemas'); +const { + PermissionBits, + PermissionTypes, + Permissions, + FileContext, +} = require('librechat-data-provider'); const { createSkill, getSkillById, @@ -9,21 +21,60 @@ const { updateSkill, deleteSkill, listSkillFiles, + upsertSkillFile, deleteSkillFile, + getSkillFileByPath, + updateSkillFileContent, getRoleByName, } = require('~/models'); const { requireJwtAuth, canAccessSkillResource } = require('~/server/middleware'); const { findAccessibleResources, findPubliclyAccessibleResources, + hasPublicPermission, grantPermission, } = require('~/server/services/PermissionService'); +const { getStrategyFunctions } = require('~/server/services/Files/strategies'); +const { createFileLimiters } = require('~/server/middleware/limiters/uploadLimiters'); +const configMiddleware = require('~/server/middleware/config/app'); +const { getFileStrategy } = require('~/server/utils/getFileStrategy'); const router = express.Router(); -// Role-based capability gates. Mirrors prompts.js — the ACL middleware on each -// route handles per-resource permissions; these check that the caller's role -// is even allowed to use / create skills at all. +// --------------------------------------------------------------------------- +// Multer: memory storage for skill imports (zip processed in-memory) +// --------------------------------------------------------------------------- +const ALLOWED_EXTENSIONS = new Set(['.md', '.zip', '.skill']); +const MAX_IMPORT_SIZE = 50 * 1024 * 1024; // 50 MB + +const memoryStorage = multer.memoryStorage(); + +const skillImportFilter = (_req, file, cb) => { + const ext = path.extname(file.originalname).toLowerCase(); + if (ALLOWED_EXTENSIONS.has(ext)) { + cb(null, true); + } else { + // N.B. The error handler at the bottom of this file matches this "Only " prefix. + cb(new Error('Only .md, .zip, and .skill files are allowed'), false); + } +}; + +const skillUpload = multer({ + storage: memoryStorage, + fileFilter: skillImportFilter, + limits: { fileSize: MAX_IMPORT_SIZE }, +}); + +// Per-file upload (for adding individual files to an existing skill) +const MAX_SINGLE_FILE_SIZE = 10 * 1024 * 1024; // 10 MB +const singleFileUpload = multer({ + storage: memoryStorage, + limits: { fileSize: MAX_SINGLE_FILE_SIZE }, +}); + +// --------------------------------------------------------------------------- +// Role-based capability gates +// --------------------------------------------------------------------------- const checkSkillAccess = generateCheckAccess({ permissionType: PermissionTypes.SKILLS, permissions: [Permissions.USE], @@ -35,9 +86,18 @@ const checkSkillCreate = generateCheckAccess({ getRoleByName, }); +// --------------------------------------------------------------------------- +// Rate limiters (reuse existing file upload limiters) +// --------------------------------------------------------------------------- +const { fileUploadIpLimiter, fileUploadUserLimiter } = createFileLimiters(); + router.use(requireJwtAuth); +router.use(configMiddleware); router.use(checkSkillAccess); +// --------------------------------------------------------------------------- +// CRUD handlers +// --------------------------------------------------------------------------- const handlers = createSkillsHandlers({ createSkill, getSkillById, @@ -46,12 +106,158 @@ const handlers = createSkillsHandlers({ deleteSkill, listSkillFiles, deleteSkillFile, + getSkillFileByPath, + updateSkillFileContent, + getStrategyFunctions, findAccessibleResources, findPubliclyAccessibleResources, + hasPublicPermission, grantPermission, isValidObjectIdString, }); +// --------------------------------------------------------------------------- +// File storage helper: resolve the active strategy's saveBuffer +// --------------------------------------------------------------------------- +function resolveSkillStorage(req, { isImage = false } = {}) { + const source = getFileStrategy(req.config, { context: FileContext.skill_file, isImage }); + const strategy = getStrategyFunctions(source); + if (!strategy.saveBuffer) { + throw new Error(`Storage backend "${source}" does not support file writes`); + } + return { saveBuffer: strategy.saveBuffer, source }; +} + +// --------------------------------------------------------------------------- +// Import handler (zip/md/skill → create skill + files) +// --------------------------------------------------------------------------- +const importHandler = createImportHandler({ + createSkill, + getSkillById, + deleteSkill, + upsertSkillFile, + saveBuffer: (req, { userId, buffer, fileName, basePath, isImage }) => { + const storage = resolveSkillStorage(req, { isImage }); + return storage.saveBuffer({ userId, buffer, fileName, basePath }).then((filepath) => ({ + filepath, + source: storage.source, + })); + }, + deleteFile: (req, file) => { + const { deleteFile } = getStrategyFunctions(file.source); + if (deleteFile) { + return deleteFile(req, file); + } + return Promise.resolve(); + }, + grantPermission, +}); + +// --------------------------------------------------------------------------- +// Per-file upload handler (add a single file to an existing skill) +// --------------------------------------------------------------------------- +async function uploadFileHandler(req, res) { + try { + const { file } = req; + if (!file) { + return res.status(400).json({ error: 'No file provided' }); + } + + const skillId = req.params.id; + const relativePath = req.body.relativePath; + if (!relativePath) { + return res.status(400).json({ error: 'relativePath is required in form body' }); + } + if (relativePath.toUpperCase() === 'SKILL.MD') { + return res.status(400).json({ error: 'SKILL.md is reserved; update the skill body instead' }); + } + // Reject traversal, absolute paths, empty/dot segments — matches model-layer validator + // so storage writes don't happen before DB rejects the path. + if ( + !/^[a-zA-Z0-9._\-/]+$/.test(relativePath) || + /^\//.test(relativePath) || + relativePath.split('/').some((s) => s === '' || s === '.' || s === '..') + ) { + return res.status(400).json({ error: 'Invalid file path' }); + } + + // Look up existing file before saving — needed to clean up old blob on replace + const existingFile = await getSkillFileByPath(skillId, relativePath); + + const fileId = crypto.randomUUID(); + const filename = file.originalname; + const storageFileName = `${fileId}__${filename}`; + + const isImage = (file.mimetype || '').startsWith('image/'); + const storage = resolveSkillStorage(req, { isImage }); + const filepath = await storage.saveBuffer({ + userId: req.user.id, + buffer: file.buffer, + fileName: storageFileName, + basePath: 'uploads', + }); + + let result; + try { + result = await upsertSkillFile({ + skillId, + relativePath, + file_id: fileId, + filename, + filepath, + source: storage.source, + mimeType: file.mimetype || 'application/octet-stream', + bytes: file.size, + isExecutable: false, + author: req.user._id, + }); + } catch (dbError) { + // Clean up the stored blob so it doesn't leak on DB failure + try { + const { deleteFile } = getStrategyFunctions(storage.source); + if (deleteFile) { + await deleteFile(req, { filepath }); + } + } catch (cleanupErr) { + logger.error('[uploadFile] Failed to clean up orphaned blob:', cleanupErr); + } + throw dbError; + } + + // Clean up old blob if this was a replace (different filepath means new storage object) + if (existingFile && existingFile.filepath !== filepath) { + const { deleteFile: delOld } = getStrategyFunctions(existingFile.source); + if (delOld) { + delOld(req, { filepath: existingFile.filepath }).catch((e) => + logger.error('[uploadFile] Old blob cleanup failed:', e), + ); + } + } + + return res.status(200).json(result); + } catch (error) { + if (error.code === 'SKILL_FILE_VALIDATION_FAILED') { + return res.status(400).json({ error: error.message }); + } + logger.error('[uploadFile] Error:', error); + return res.status(500).json({ error: 'Failed to upload file' }); + } +} + +// --------------------------------------------------------------------------- +// Routes +// --------------------------------------------------------------------------- + +// Import: accepts .md / .zip / .skill via multipart +router.post( + '/import', + checkSkillCreate, + fileUploadIpLimiter, + fileUploadUserLimiter, + skillUpload.single('file'), + importHandler, +); + router.get('/', handlers.list); router.post('/', checkSkillCreate, handlers.create); @@ -81,16 +287,20 @@ router.get( handlers.listFiles, ); +// Per-file upload (live — replaces 501 stub) router.post( '/:id/files', canAccessSkillResource({ requiredPermission: PermissionBits.EDIT }), - handlers.uploadFileStub, + fileUploadIpLimiter, + fileUploadUserLimiter, + singleFileUpload.single('file'), + uploadFileHandler, ); router.get( '/:id/files/:relativePath', canAccessSkillResource({ requiredPermission: PermissionBits.VIEW }), - handlers.downloadFileStub, + handlers.downloadFile, ); router.delete( @@ -99,4 +309,13 @@ router.delete( handlers.deleteFile, ); +// Multer + file-filter error handler — surface as 400, forward everything else + +router.use((err, _req, res, next) => { + if (err && (err.name === 'MulterError' || err.message?.startsWith('Only '))) { + return res.status(400).json({ error: err.message }); + } + return next(err); +}); + module.exports = router; diff --git a/api/server/routes/skills.test.js b/api/server/routes/skills.test.js index c4b0c6b8c9..1a0a23c896 100644 --- a/api/server/routes/skills.test.js +++ b/api/server/routes/skills.test.js @@ -12,6 +12,35 @@ const { jest.mock('~/server/services/Config', () => ({ getCachedTools: jest.fn().mockResolvedValue({}), + getAppConfig: jest.fn().mockResolvedValue({ + fileStrategy: 'local', + paths: { uploads: '/tmp/uploads', images: '/tmp/images' }, + }), +})); + +jest.mock('~/server/middleware/config/app', () => (req, _res, next) => { + req.config = { + fileStrategy: 'local', + paths: { uploads: '/tmp/uploads', images: '/tmp/images' }, + }; + next(); +}); + +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: jest.fn().mockReturnValue({ + saveBuffer: jest.fn().mockResolvedValue('/uploads/test/file.txt'), + getDownloadStream: jest.fn().mockResolvedValue({ + pipe: jest.fn(), + on: jest.fn(), + [Symbol.asyncIterator]: async function* () { + yield Buffer.from('test content'); + }, + }), + }), +})); + +jest.mock('~/server/utils/getFileStrategy', () => ({ + getFileStrategy: jest.fn().mockReturnValue('local'), })); jest.mock('~/models', () => { @@ -381,22 +410,32 @@ describe('Skill routes', () => { }); }); - describe('POST /api/skills/:id/files (stub)', () => { - it('returns 501 with phase marker', async () => { + describe('POST /api/skills/:id/files (live)', () => { + it('returns 400 when no file is provided', async () => { const created = await createSkillAsOwner(); const res = await request(app).post(`/api/skills/${created.body._id}/files`); - expect(res.status).toBe(501); - expect(res.body.phase).toBe(2); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/no file/i); }); }); - describe('GET /api/skills/:id/files/:relativePath (stub)', () => { - it('returns 501 stub', async () => { + describe('GET /api/skills/:id/files/:relativePath', () => { + it('returns SKILL.md content from skill body', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).get(`/api/skills/${created.body._id}/files/SKILL.md`); + expect(res.status).toBe(200); + expect(res.body.mimeType).toBe('text/markdown'); + expect(res.body.isBinary).toBe(false); + expect(res.body.filename).toBe('SKILL.md'); + expect(res.body.content).toBeDefined(); + }); + + it('returns 404 for a nonexistent file', async () => { const created = await createSkillAsOwner(); const res = await request(app).get( - `/api/skills/${created.body._id}/files/scripts%2Fparse.sh`, + `/api/skills/${created.body._id}/files/scripts%2Fmissing.sh`, ); - expect(res.status).toBe(501); + expect(res.status).toBe(404); }); }); diff --git a/api/server/utils/getFileStrategy.js b/api/server/utils/getFileStrategy.js index 2e3dfdd79e..1bab7f2468 100644 --- a/api/server/utils/getFileStrategy.js +++ b/api/server/utils/getFileStrategy.js @@ -43,6 +43,10 @@ function getFileStrategy(appConfig, { isAvatar = false, isImage = false, context if (isAvatar || context === FileContext.avatar) { selectedStrategy = strategies.avatar || defaultStrategy; + } else if (context === FileContext.skill_file) { + // Skill files: explicit skills strategy → fall back by type → default + selectedStrategy = + strategies.skills || (isImage ? strategies.image : strategies.document) || defaultStrategy; } else if (isImage || context === FileContext.image_generation) { selectedStrategy = strategies.image || defaultStrategy; } else { diff --git a/client/package.json b/client/package.json index 9765917a87..feb07b1937 100644 --- a/client/package.json +++ b/client/package.json @@ -7,7 +7,7 @@ "typecheck": "tsc --noEmit", "data-provider": "cd .. && npm run build:data-provider", "build:file": "cross-env NODE_ENV=production vite build --debug > vite-output.log 2>&1", - "build": "cross-env NODE_ENV=production vite build && node ./scripts/post-build.cjs", + "build": "cross-env NODE_ENV=production NODE_OPTIONS=--max-old-space-size=8192 vite build && node ./scripts/post-build.cjs", "build:ci": "cross-env NODE_ENV=development vite build --mode ci", "dev": "cross-env NODE_ENV=development vite", "preview-prod": "cross-env NODE_ENV=development vite preview", @@ -101,6 +101,7 @@ "react-textarea-autosize": "^8.4.0", "react-transition-group": "^4.4.5", "react-virtualized": "^9.22.6", + "react-vtree": "^3.0.0", "recoil": "^0.7.7", "regenerator-runtime": "^0.14.1", "rehype-highlight": "^6.0.0", diff --git a/client/src/common/agents-types.ts b/client/src/common/agents-types.ts index c3ea06f890..cd3e5e0f49 100644 --- a/client/src/common/agents-types.ts +++ b/client/src/common/agents-types.ts @@ -38,6 +38,7 @@ export type AgentForm = { tools?: string[]; /** Per-tool configuration options (deferred loading, allowed callers, etc.) */ tool_options?: AgentToolOptions; + skills?: string[]; provider?: AgentProvider | OptionWithIcon; /** @deprecated Use edges instead */ agent_ids?: string[]; diff --git a/client/src/components/Prompts/display/PromptTextCard.tsx b/client/src/components/Prompts/display/PromptTextCard.tsx index 379dcab21f..7204c8a5ab 100644 --- a/client/src/components/Prompts/display/PromptTextCard.tsx +++ b/client/src/components/Prompts/display/PromptTextCard.tsx @@ -5,7 +5,7 @@ import rehypeKatex from 'rehype-katex'; import supersub from 'remark-supersub'; import ReactMarkdown from 'react-markdown'; import rehypeHighlight from 'rehype-highlight'; -import { Copy, Check, FileText } from 'lucide-react'; +import { Copy, Check } from 'lucide-react'; import { Button, TooltipAnchor, useToastContext } from '@librechat/client'; import { codeNoExecution } from '~/components/Chat/Messages/Content/MarkdownComponents'; import { PromptVariableGfm } from '../editor/Markdown'; @@ -52,14 +52,8 @@ const PromptTextCard = ({ mainText }: PromptTextCardProps) => { }, [mainText, showToast, localize, isCopied]); return ( -
-
-
-
+
+
{ } /> -
+
= ({ name, isEditing, setIsEditing }) => { return (

{localize('com_ui_control_bar')}

-
-
-
-
+
+
= ({ name, isEditing, setIsEditing }) => { } />
-
-
{!isEditing && ( +
+ ); + })} +
+
+ +
+
+ + )} + {/* Agent Tools & Actions */}