diff --git a/api/server/routes/admin/skills.js b/api/server/routes/admin/skills.js index 820b2e44c0..74a81059d9 100644 --- a/api/server/routes/admin/skills.js +++ b/api/server/routes/admin/skills.js @@ -1,14 +1,37 @@ const express = require('express'); const { createAdminSkillsSyncHandlers } = require('@librechat/api'); const { SystemCapabilities } = require('@librechat/data-schemas'); -const { requireCapability } = require('~/server/middleware/roles/capabilities'); +const { hasCapability, requireCapability } = require('~/server/middleware/roles/capabilities'); const { requireJwtAuth } = require('~/server/middleware'); const { upsertSkillSyncCredential, deleteSkillSyncCredential } = require('~/models'); const { getGitHubSkillSyncRunner } = require('~/server/services/Skills/sync'); const router = express.Router(); const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); -const requireManageSkills = requireCapability(SystemCapabilities.MANAGE_SKILLS); + +function requirePlatformCapability(capability) { + return async (req, res, next) => { + try { + const id = req.user?.id ?? req.user?._id?.toString?.(); + if (!id) { + return res.status(401).json({ message: 'Authentication required' }); + } + const user = { + id, + role: req.user?.role ?? '', + }; + if (await hasCapability(user, capability)) { + return next(); + } + return res.status(403).json({ message: 'Forbidden' }); + } catch { + return res.status(500).json({ message: 'Internal Server Error' }); + } + }; +} + +const requirePlatformReadSkills = requirePlatformCapability(SystemCapabilities.READ_SKILLS); +const requirePlatformManageSkills = requirePlatformCapability(SystemCapabilities.MANAGE_SKILLS); const handlers = createAdminSkillsSyncHandlers({ runner: getGitHubSkillSyncRunner(), @@ -18,9 +41,13 @@ const handlers = createAdminSkillsSyncHandlers({ router.use(requireJwtAuth, requireAdminAccess); -router.get('/sync/status', handlers.getSyncStatus); -router.post('/sync/run', requireManageSkills, handlers.runSync); -router.put('/sync/credentials/:credentialKey', requireManageSkills, handlers.setCredential); -router.delete('/sync/credentials/:credentialKey', requireManageSkills, handlers.deleteCredential); +router.get('/sync/status', requirePlatformReadSkills, handlers.getSyncStatus); +router.post('/sync/run', requirePlatformManageSkills, handlers.runSync); +router.put('/sync/credentials/:credentialKey', requirePlatformManageSkills, handlers.setCredential); +router.delete( + '/sync/credentials/:credentialKey', + requirePlatformManageSkills, + handlers.deleteCredential, +); module.exports = router; diff --git a/api/server/routes/admin/skills.test.js b/api/server/routes/admin/skills.test.js index 07471238b6..59fe33f529 100644 --- a/api/server/routes/admin/skills.test.js +++ b/api/server/routes/admin/skills.test.js @@ -1,12 +1,20 @@ const express = require('express'); const request = require('supertest'); -const mockRequireJwtAuth = jest.fn((req, res, next) => next()); +const mockRequireJwtAuth = jest.fn((req, res, next) => { + req.user = { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' }; + next(); +}); const mockCapabilityMiddleware = jest.fn((req, res, next) => next()); const mockRequireCapability = jest.fn(() => mockCapabilityMiddleware); +const mockHasCapability = jest.fn().mockResolvedValue(true); jest.mock('@librechat/data-schemas', () => ({ - SystemCapabilities: { ACCESS_ADMIN: 'access:admin', MANAGE_SKILLS: 'manage:skills' }, + SystemCapabilities: { + ACCESS_ADMIN: 'access:admin', + READ_SKILLS: 'read:skills', + MANAGE_SKILLS: 'manage:skills', + }, })); jest.mock('@librechat/api', () => ({ @@ -19,6 +27,7 @@ jest.mock('@librechat/api', () => ({ })); jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: mockHasCapability, requireCapability: mockRequireCapability, })); @@ -51,8 +60,13 @@ describe('admin skills sync routes', () => { await request(app).delete('/api/admin/skills/sync/credentials/default').expect(200); expect(mockRequireCapability).toHaveBeenCalledWith('access:admin'); - expect(mockRequireCapability).toHaveBeenCalledWith('manage:skills'); expect(mockRequireJwtAuth).toHaveBeenCalled(); expect(mockCapabilityMiddleware).toHaveBeenCalled(); + expect(mockHasCapability).toHaveBeenCalledTimes(4); + expect(mockHasCapability).toHaveBeenCalledWith({ id: 'user-1', role: 'ADMIN' }, 'read:skills'); + expect(mockHasCapability).toHaveBeenCalledWith( + { id: 'user-1', role: 'ADMIN' }, + 'manage:skills', + ); }); }); diff --git a/packages/api/src/skills/sync/github.spec.ts b/packages/api/src/skills/sync/github.spec.ts index ff5d47f190..46f7bccb11 100644 --- a/packages/api/src/skills/sync/github.spec.ts +++ b/packages/api/src/skills/sync/github.spec.ts @@ -432,6 +432,42 @@ describe('createGitHubSkillSyncRunner', () => { } }); + it('preserves slash-delimited refs when fetching the GitHub commit', async () => { + const baseFetch = githubFetch(); + const fetchFn = jest.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes('/commits/')) { + expect(url).toContain('/commits/heads/release/2026-05'); + expect(url).not.toContain('heads%2Frelease%2F2026-05'); + } + return baseFetch(input); + }) as unknown as typeof fetch; + const deps = createDeps({ + fetchFn, + getConfig: () => ({ + github: { + enabled: true, + intervalMinutes: 60, + runOnStartup: false, + sources: [ + { + id: 'librechat-skills', + owner: 'LibreChat', + repo: 'skills', + ref: 'heads/release/2026-05', + paths: ['skills'], + credentialKey: 'github-skills-prod', + }, + ], + }, + }), + }); + const result = await createGitHubSkillSyncRunner(deps).runOnce(); + + expect(result.status).toBe('completed'); + expect(fetchFn).toHaveBeenCalled(); + }); + it('runs a tenant-scoped source inside its tenant context and stamps the skill tenantId', async () => { let observedTenantId: string | undefined = 'unset'; const deps = createDeps({ diff --git a/packages/api/src/skills/sync/github.ts b/packages/api/src/skills/sync/github.ts index 1e326ad123..63f6967b3d 100644 --- a/packages/api/src/skills/sync/github.ts +++ b/packages/api/src/skills/sync/github.ts @@ -439,6 +439,10 @@ function buildGitHubUrl(pathname: string): string { return `${GITHUB_API_BASE}${pathname}`; } +function encodeGitHubPath(value: string): string { + return value.split('/').map(encodeURIComponent).join('/'); +} + async function readGitHubErrorMessage(response: Response): Promise { try { const body = (await response.json()) as { message?: unknown }; @@ -502,7 +506,7 @@ async function fetchCommit(params: { }): Promise { const owner = encodeURIComponent(params.source.owner); const repo = encodeURIComponent(params.source.repo); - const ref = encodeURIComponent(params.source.ref); + const ref = encodeGitHubPath(params.source.ref); return githubJson({ fetchFn: params.fetchFn, token: params.token,