fix: Harden GitHub skill sync admin gates

This commit is contained in:
Danny Avila 2026-05-30 16:07:15 -07:00
parent 356012f699
commit d1e3cc44ac
4 changed files with 91 additions and 10 deletions

View file

@ -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;

View file

@ -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',
);
});
});

View file

@ -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({

View file

@ -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<string | undefined> {
try {
const body = (await response.json()) as { message?: unknown };
@ -502,7 +506,7 @@ async function fetchCommit(params: {
}): Promise<GitHubCommitResponse> {
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<GitHubCommitResponse>({
fetchFn: params.fetchFn,
token: params.token,