mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: Restrict skill sync server credentials
This commit is contained in:
parent
5db44b4bb5
commit
e3fe184516
10 changed files with 514 additions and 36 deletions
|
|
@ -1,4 +1,5 @@
|
|||
const express = require('express');
|
||||
const { skillSyncConfigSchema } = require('librechat-data-provider');
|
||||
const { createAdminSkillsSyncHandlers } = require('@librechat/api');
|
||||
const { SystemCapabilities } = require('@librechat/data-schemas');
|
||||
const { hasCapability, requireCapability } = require('~/server/middleware/roles/capabilities');
|
||||
|
|
@ -10,18 +11,51 @@ const configMiddleware = require('~/server/middleware/config/app');
|
|||
const router = express.Router();
|
||||
const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN);
|
||||
|
||||
function getCapabilityUser(req, { platformOnly = false } = {}) {
|
||||
const id = req.user?.id ?? req.user?._id?.toString?.();
|
||||
if (!id) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
id,
|
||||
role: req.user?.role ?? '',
|
||||
...(platformOnly ? {} : { tenantId: req.user?.tenantId }),
|
||||
};
|
||||
}
|
||||
|
||||
function parseSkillSyncConfig(raw) {
|
||||
if (!raw || typeof raw !== 'object') {
|
||||
return undefined;
|
||||
}
|
||||
const parsed = skillSyncConfigSchema.safeParse(raw);
|
||||
return parsed.success ? parsed.data : undefined;
|
||||
}
|
||||
|
||||
function isSameSkillSyncConfig(left, right) {
|
||||
return JSON.stringify(left ?? null) === JSON.stringify(right ?? null);
|
||||
}
|
||||
|
||||
function hasResolvedSkillSyncOverride(req) {
|
||||
const resolved = parseSkillSyncConfig(req.config?.skillSync);
|
||||
const base = parseSkillSyncConfig(req.config?.config?.skillSync);
|
||||
return Boolean(resolved?.github && !isSameSkillSyncConfig(resolved, base));
|
||||
}
|
||||
|
||||
async function hasSkillCapability(req, capability, { platformOnly = false } = {}) {
|
||||
const user = getCapabilityUser(req, { platformOnly });
|
||||
if (!user) {
|
||||
return false;
|
||||
}
|
||||
return hasCapability(user, capability);
|
||||
}
|
||||
|
||||
function requireSkillCapability(capability, { platformOnly = false } = {}) {
|
||||
return async (req, res, next) => {
|
||||
try {
|
||||
const id = req.user?.id ?? req.user?._id?.toString?.();
|
||||
if (!id) {
|
||||
const user = getCapabilityUser(req, { platformOnly });
|
||||
if (!user) {
|
||||
return res.status(401).json({ message: 'Authentication required' });
|
||||
}
|
||||
const user = {
|
||||
id,
|
||||
role: req.user?.role ?? '',
|
||||
...(platformOnly ? {} : { tenantId: req.user?.tenantId }),
|
||||
};
|
||||
if (await hasCapability(user, capability)) {
|
||||
return next();
|
||||
}
|
||||
|
|
@ -32,8 +66,42 @@ function requireSkillCapability(capability, { platformOnly = false } = {}) {
|
|||
};
|
||||
}
|
||||
|
||||
async function attachCredentialReadAccess(req, res, next) {
|
||||
try {
|
||||
const canReadCredentials = await hasSkillCapability(req, SystemCapabilities.READ_SKILLS, {
|
||||
platformOnly: true,
|
||||
});
|
||||
req.skillSyncCanReadCredentials = canReadCredentials;
|
||||
req.skillSyncAllowServerCredentials = canReadCredentials;
|
||||
return next();
|
||||
} catch {
|
||||
return res.status(500).json({ message: 'Internal Server Error' });
|
||||
}
|
||||
}
|
||||
|
||||
async function requireSyncRunCapability(req, res, next) {
|
||||
try {
|
||||
const canManagePlatform = await hasSkillCapability(req, SystemCapabilities.MANAGE_SKILLS, {
|
||||
platformOnly: true,
|
||||
});
|
||||
if (canManagePlatform) {
|
||||
req.skillSyncAllowServerCredentials = true;
|
||||
return next();
|
||||
}
|
||||
if (
|
||||
hasResolvedSkillSyncOverride(req) &&
|
||||
(await hasSkillCapability(req, SystemCapabilities.MANAGE_SKILLS))
|
||||
) {
|
||||
req.skillSyncAllowServerCredentials = false;
|
||||
return next();
|
||||
}
|
||||
return res.status(403).json({ message: 'Forbidden' });
|
||||
} catch {
|
||||
return res.status(500).json({ message: 'Internal Server Error' });
|
||||
}
|
||||
}
|
||||
|
||||
const requireReadSkills = requireSkillCapability(SystemCapabilities.READ_SKILLS);
|
||||
const requireManageSkills = requireSkillCapability(SystemCapabilities.MANAGE_SKILLS);
|
||||
const requirePlatformManageSkills = requireSkillCapability(SystemCapabilities.MANAGE_SKILLS, {
|
||||
platformOnly: true,
|
||||
});
|
||||
|
|
@ -46,8 +114,8 @@ const handlers = createAdminSkillsSyncHandlers({
|
|||
|
||||
router.use(requireJwtAuth, requireAdminAccess, configMiddleware);
|
||||
|
||||
router.get('/sync/status', requireReadSkills, handlers.getSyncStatus);
|
||||
router.post('/sync/run', requireManageSkills, handlers.runSync);
|
||||
router.get('/sync/status', requireReadSkills, attachCredentialReadAccess, handlers.getSyncStatus);
|
||||
router.post('/sync/run', requireSyncRunCapability, handlers.runSync);
|
||||
router.put('/sync/credentials/:credentialKey', requirePlatformManageSkills, handlers.setCredential);
|
||||
router.delete(
|
||||
'/sync/credentials/:credentialKey',
|
||||
|
|
|
|||
|
|
@ -8,11 +8,18 @@ const mockRequireJwtAuth = jest.fn((req, res, next) => {
|
|||
const mockCapabilityMiddleware = jest.fn((req, res, next) => next());
|
||||
const mockRequireCapability = jest.fn(() => mockCapabilityMiddleware);
|
||||
const mockHasCapability = jest.fn().mockResolvedValue(true);
|
||||
let mockResolvedConfig = { skillSync: { github: { enabled: false, sources: [] } } };
|
||||
const mockConfigMiddleware = jest.fn((req, res, next) => {
|
||||
req.config = { skillSync: { github: { enabled: false, sources: [] } } };
|
||||
req.config = mockResolvedConfig;
|
||||
next();
|
||||
});
|
||||
const mockGetGitHubSkillSyncRunnerForRequest = jest.fn();
|
||||
const mockHandlers = {
|
||||
getSyncStatus: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
runSync: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
setCredential: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
deleteCredential: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
};
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
SystemCapabilities: {
|
||||
|
|
@ -23,12 +30,7 @@ jest.mock('@librechat/data-schemas', () => ({
|
|||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
createAdminSkillsSyncHandlers: jest.fn(() => ({
|
||||
getSyncStatus: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
runSync: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
setCredential: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
deleteCredential: jest.fn((req, res) => res.status(200).json({ ok: true })),
|
||||
})),
|
||||
createAdminSkillsSyncHandlers: jest.fn(() => mockHandlers),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/middleware/roles/capabilities', () => ({
|
||||
|
|
@ -52,11 +54,22 @@ jest.mock('~/server/services/Skills/sync', () => ({
|
|||
}));
|
||||
|
||||
describe('admin skills sync routes', () => {
|
||||
it('requires JWT auth and admin capabilities for sync endpoints', async () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockHasCapability.mockResolvedValue(true);
|
||||
mockResolvedConfig = { skillSync: { github: { enabled: false, sources: [] } } };
|
||||
});
|
||||
|
||||
function createApp() {
|
||||
const router = require('./skills');
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use('/api/admin/skills', router);
|
||||
return app;
|
||||
}
|
||||
|
||||
it('requires JWT auth and admin capabilities for sync endpoints', async () => {
|
||||
const app = createApp();
|
||||
|
||||
await request(app).get('/api/admin/skills/sync/status').expect(200);
|
||||
await request(app).post('/api/admin/skills/sync/run').expect(200);
|
||||
|
|
@ -67,15 +80,12 @@ describe('admin skills sync routes', () => {
|
|||
expect(mockRequireJwtAuth).toHaveBeenCalled();
|
||||
expect(mockCapabilityMiddleware).toHaveBeenCalled();
|
||||
expect(mockConfigMiddleware).toHaveBeenCalled();
|
||||
expect(mockHasCapability).toHaveBeenCalledTimes(4);
|
||||
expect(mockHasCapability).toHaveBeenCalledTimes(5);
|
||||
expect(mockHasCapability).toHaveBeenCalledWith(
|
||||
{ id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
|
||||
'read:skills',
|
||||
);
|
||||
expect(mockHasCapability).toHaveBeenCalledWith(
|
||||
{ id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
|
||||
'manage:skills',
|
||||
);
|
||||
expect(mockHasCapability).toHaveBeenCalledWith({ id: 'user-1', role: 'ADMIN' }, 'read:skills');
|
||||
expect(mockHasCapability).toHaveBeenCalledWith(
|
||||
{ id: 'user-1', role: 'ADMIN' },
|
||||
'manage:skills',
|
||||
|
|
@ -85,4 +95,112 @@ describe('admin skills sync routes', () => {
|
|||
expect.objectContaining({ getRunner: mockGetGitHubSkillSyncRunnerForRequest }),
|
||||
);
|
||||
});
|
||||
|
||||
it('marks credential metadata hidden for tenant-scoped status reads', async () => {
|
||||
mockHasCapability.mockImplementation(async (user, capability) => {
|
||||
if (capability === 'read:skills') {
|
||||
return Boolean(user.tenantId);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
await request(app).get('/api/admin/skills/sync/status').expect(200);
|
||||
|
||||
const req = mockHandlers.getSyncStatus.mock.calls[0][0];
|
||||
expect(req.skillSyncCanReadCredentials).toBe(false);
|
||||
expect(req.skillSyncAllowServerCredentials).toBe(false);
|
||||
});
|
||||
|
||||
it('allows tenant admins to run resolved override sync without server credentials', async () => {
|
||||
const skillSync = {
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
id: 'tenant-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
token: '${GITHUB_SKILLS_TOKEN}',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
mockResolvedConfig = { skillSync, config: {} };
|
||||
mockHasCapability.mockImplementation(async (user, capability) => {
|
||||
if (capability === 'manage:skills') {
|
||||
return Boolean(user.tenantId);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
await request(app).post('/api/admin/skills/sync/run').expect(200);
|
||||
|
||||
const req = mockHandlers.runSync.mock.calls[0][0];
|
||||
expect(req.skillSyncAllowServerCredentials).toBe(false);
|
||||
});
|
||||
|
||||
it('prevents tenant admins from manually running base skill sync config', async () => {
|
||||
const skillSync = {
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
id: 'base-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
token: '${GITHUB_SKILLS_TOKEN}',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
mockResolvedConfig = { skillSync, config: { skillSync } };
|
||||
mockHasCapability.mockImplementation(async (user, capability) => {
|
||||
if (capability === 'manage:skills') {
|
||||
return Boolean(user.tenantId);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
await request(app).post('/api/admin/skills/sync/run').expect(403);
|
||||
|
||||
expect(mockHandlers.runSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('allows platform admins to manually run base skill sync config with server credentials', async () => {
|
||||
const skillSync = {
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
id: 'base-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
token: '${GITHUB_SKILLS_TOKEN}',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
mockResolvedConfig = { skillSync, config: { skillSync } };
|
||||
const app = createApp();
|
||||
|
||||
await request(app).post('/api/admin/skills/sync/run').expect(200);
|
||||
|
||||
const req = mockHandlers.runSync.mock.calls[0][0];
|
||||
expect(req.skillSyncAllowServerCredentials).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -57,7 +57,7 @@ async function getSyntheticReq({ userId = SYSTEM_USER_ID, tenantId, loadAppConfi
|
|||
};
|
||||
}
|
||||
|
||||
function createRunner({ getConfig, loadAppConfig } = {}) {
|
||||
function createRunner({ getConfig, loadAppConfig, allowServerCredentials = true } = {}) {
|
||||
const resolveAppConfig = loadAppConfig ?? loadCurrentAppConfig;
|
||||
const resolveConfig = getConfig ?? (() => getSyncConfig(resolveAppConfig));
|
||||
const createdRunner = createGitHubSkillSyncRunner({
|
||||
|
|
@ -141,6 +141,7 @@ function createRunner({ getConfig, loadAppConfig } = {}) {
|
|||
file,
|
||||
);
|
||||
},
|
||||
allowServerCredentials,
|
||||
});
|
||||
return {
|
||||
getStatus: createdRunner.getStatus,
|
||||
|
|
|
|||
|
|
@ -185,6 +185,7 @@ describe('GitHub skill sync service', () => {
|
|||
const requestRunner = mockCreatedRunners[0].runner;
|
||||
const requestConfig = await mockCreatedRunners[0].deps.getConfig();
|
||||
expect(started).toBe(true);
|
||||
expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(false);
|
||||
expect(requestRunner.runOnce).toHaveBeenCalledTimes(1);
|
||||
expect(requestConfig.github.runOnStartup).toBe(false);
|
||||
expect(requestConfig.github.sources[0]).toEqual(
|
||||
|
|
@ -248,17 +249,47 @@ describe('GitHub skill sync service', () => {
|
|||
const runner = service.getGitHubSkillSyncRunnerForRequest({
|
||||
config: { skillSync, config: {} },
|
||||
user: { id: 'user-1', tenantId: 'tenant-a' },
|
||||
skillSyncAllowServerCredentials: true,
|
||||
});
|
||||
const config = await mockCreatedRunners[0].deps.getConfig();
|
||||
|
||||
expect(runner.runOnce).toBe(mockCreatedRunners[0].runner.runOnce);
|
||||
expect(runner.getStatus).toBe(mockCreatedRunners[0].runner.getStatus);
|
||||
expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(true);
|
||||
expect(config.github.runOnStartup).toBe(true);
|
||||
expect(config.github.sources[0]).toEqual(
|
||||
expect.objectContaining({ id: 'tenant-skills', tenantId: 'tenant-a' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not allow request-built admin override runners to use server credentials by default', async () => {
|
||||
const skillSync = {
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: true,
|
||||
sources: [
|
||||
{
|
||||
id: 'tenant-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
token: '${GITHUB_SKILLS_TOKEN}',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
const service = require('./sync');
|
||||
service.getGitHubSkillSyncRunnerForRequest({
|
||||
config: { skillSync, config: {} },
|
||||
user: { id: 'user-1', tenantId: 'tenant-a' },
|
||||
});
|
||||
|
||||
expect(mockCreatedRunners[0].deps.allowServerCredentials).toBe(false);
|
||||
});
|
||||
|
||||
it('does not start a request-scoped sync when the configured source is already running', async () => {
|
||||
const skillSync = {
|
||||
github: {
|
||||
|
|
|
|||
132
packages/api/src/admin/skills.spec.ts
Normal file
132
packages/api/src/admin/skills.spec.ts
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
import type { Response } from 'express';
|
||||
import { createAdminSkillsSyncHandlers } from './skills';
|
||||
|
||||
function createResponse() {
|
||||
const res = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
return res as unknown as Response & {
|
||||
status: jest.Mock;
|
||||
json: jest.Mock;
|
||||
};
|
||||
}
|
||||
|
||||
function createHandlers() {
|
||||
const runner = {
|
||||
getStatus: jest.fn(async () => ({
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
provider: 'github',
|
||||
sourceId: 'tenant-skills',
|
||||
tenantId: 'tenant-a',
|
||||
status: 'idle',
|
||||
credentialKey: 'github-skills-prod',
|
||||
credentialPresent: true,
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
syncedSkillCount: 0,
|
||||
syncedFileCount: 0,
|
||||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
},
|
||||
],
|
||||
credentials: [
|
||||
{
|
||||
provider: 'github',
|
||||
credentialKey: 'github-skills-prod',
|
||||
credentialPresent: true,
|
||||
tokenFingerprint: 'abc123',
|
||||
},
|
||||
],
|
||||
fineGrainedTokenRecommendation: 'Use a fine-grained token.',
|
||||
})),
|
||||
runOnce: jest.fn(async () => ({
|
||||
status: 'completed',
|
||||
sources: [
|
||||
{
|
||||
provider: 'github',
|
||||
sourceId: 'tenant-skills',
|
||||
tenantId: 'tenant-a',
|
||||
status: 'succeeded',
|
||||
credentialKey: 'github-skills-prod',
|
||||
credentialPresent: true,
|
||||
syncedSkillCount: 1,
|
||||
syncedFileCount: 2,
|
||||
deletedSkillCount: 0,
|
||||
deletedFileCount: 0,
|
||||
},
|
||||
],
|
||||
})),
|
||||
};
|
||||
const handlers = createAdminSkillsSyncHandlers({
|
||||
runner,
|
||||
upsertCredential: jest.fn(),
|
||||
deleteCredential: jest.fn(),
|
||||
});
|
||||
return { handlers, runner };
|
||||
}
|
||||
|
||||
describe('createAdminSkillsSyncHandlers', () => {
|
||||
it('omits credential summaries and source credential metadata for tenant-scoped status reads', async () => {
|
||||
const { handlers } = createHandlers();
|
||||
const res = createResponse();
|
||||
|
||||
await handlers.getSyncStatus({ skillSyncCanReadCredentials: false } as never, res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
credentials: [],
|
||||
sources: [
|
||||
expect.objectContaining({
|
||||
credentialKey: undefined,
|
||||
credentialPresent: false,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('includes credential summaries and source credential metadata for platform status reads', async () => {
|
||||
const { handlers } = createHandlers();
|
||||
const res = createResponse();
|
||||
|
||||
await handlers.getSyncStatus({ skillSyncCanReadCredentials: true } as never, res);
|
||||
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
credentials: [expect.objectContaining({ credentialKey: 'github-skills-prod' })],
|
||||
sources: [
|
||||
expect.objectContaining({
|
||||
credentialKey: 'github-skills-prod',
|
||||
credentialPresent: true,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('omits source credential metadata from tenant-scoped manual run responses', async () => {
|
||||
const { handlers } = createHandlers();
|
||||
const res = createResponse();
|
||||
|
||||
await handlers.runSync({ skillSyncAllowServerCredentials: false } as never, res);
|
||||
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sources: [
|
||||
expect.objectContaining({
|
||||
credentialKey: undefined,
|
||||
credentialPresent: false,
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -19,6 +19,8 @@ type AdminSkillsRequest = Request & {
|
|||
_id?: Types.ObjectId;
|
||||
id?: string;
|
||||
};
|
||||
skillSyncAllowServerCredentials?: boolean;
|
||||
skillSyncCanReadCredentials?: boolean;
|
||||
};
|
||||
|
||||
export type AdminSkillSyncDeps = {
|
||||
|
|
@ -52,14 +54,15 @@ function serializeCredential(
|
|||
|
||||
function serializeSourceStatus(
|
||||
status: ISkillSyncStatus & { credentialPresent?: boolean },
|
||||
{ includeCredentialMetadata = true }: { includeCredentialMetadata?: boolean } = {},
|
||||
): TGitHubSkillSyncSourceStatus {
|
||||
return {
|
||||
provider: status.provider,
|
||||
sourceId: status.sourceId,
|
||||
tenantId: status.tenantId,
|
||||
status: status.status,
|
||||
credentialKey: status.credentialKey,
|
||||
credentialPresent: status.credentialPresent ?? false,
|
||||
credentialKey: includeCredentialMetadata ? status.credentialKey : undefined,
|
||||
credentialPresent: includeCredentialMetadata ? (status.credentialPresent ?? false) : false,
|
||||
owner: status.owner,
|
||||
repo: status.repo,
|
||||
ref: status.ref,
|
||||
|
|
@ -96,25 +99,31 @@ export function createAdminSkillsSyncHandlers(deps: AdminSkillSyncDeps) {
|
|||
return runner;
|
||||
}
|
||||
|
||||
async function getSyncStatus(req: Request, res: Response) {
|
||||
async function getSyncStatus(req: AdminSkillsRequest, res: Response) {
|
||||
const includeCredentialMetadata = req.skillSyncCanReadCredentials !== false;
|
||||
const status = await getRunner(req).getStatus();
|
||||
const response: TGitHubSkillSyncStatusResponse = {
|
||||
enabled: status.enabled,
|
||||
intervalMinutes: status.intervalMinutes,
|
||||
runOnStartup: status.runOnStartup,
|
||||
sources: status.sources.map(serializeSourceStatus),
|
||||
credentials: status.credentials.map(serializeCredential),
|
||||
sources: status.sources.map((source) =>
|
||||
serializeSourceStatus(source, { includeCredentialMetadata }),
|
||||
),
|
||||
credentials: includeCredentialMetadata ? status.credentials.map(serializeCredential) : [],
|
||||
fineGrainedTokenRecommendation: status.fineGrainedTokenRecommendation,
|
||||
};
|
||||
return res.status(200).json(response);
|
||||
}
|
||||
|
||||
async function runSync(req: Request, res: Response) {
|
||||
async function runSync(req: AdminSkillsRequest, res: Response) {
|
||||
const includeCredentialMetadata = req.skillSyncAllowServerCredentials === true;
|
||||
const result = await getRunner(req).runOnce();
|
||||
const response: TGitHubSkillSyncManualRunResponse = {
|
||||
status: result.status,
|
||||
message: result.message,
|
||||
sources: result.sources.map(serializeSourceStatus),
|
||||
sources: result.sources.map((source) =>
|
||||
serializeSourceStatus(source, { includeCredentialMetadata }),
|
||||
),
|
||||
};
|
||||
return res.status(result.status === 'skipped' ? 202 : 200).json(response);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -433,6 +433,87 @@ describe('createGitHubSkillSyncRunner', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('does not list or resolve server credentials when server credentials are disabled', async () => {
|
||||
const previousToken = process.env.GITHUB_SKILLS_TOKEN;
|
||||
process.env.GITHUB_SKILLS_TOKEN = 'github_pat_from_env';
|
||||
const getCredentialToken = jest.fn(async () => 'github_pat_from_db');
|
||||
const listCredentials = jest.fn(async () => [
|
||||
{
|
||||
provider: 'github' as const,
|
||||
credentialKey: 'github-skills-prod',
|
||||
credentialPresent: true,
|
||||
tokenFingerprint: 'abc123',
|
||||
},
|
||||
]);
|
||||
const deps = createDeps({
|
||||
allowServerCredentials: false,
|
||||
getCredentialToken,
|
||||
listCredentials,
|
||||
getConfig: () => ({
|
||||
github: {
|
||||
enabled: true,
|
||||
intervalMinutes: 60,
|
||||
runOnStartup: false,
|
||||
sources: [
|
||||
{
|
||||
id: 'librechat-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
token: '${GITHUB_SKILLS_TOKEN}',
|
||||
},
|
||||
{
|
||||
id: 'stored-credential-skills',
|
||||
owner: 'LibreChat',
|
||||
repo: 'skills',
|
||||
ref: 'main',
|
||||
paths: ['skills'],
|
||||
credentialKey: 'github-skills-prod',
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const runner = createGitHubSkillSyncRunner(deps);
|
||||
|
||||
try {
|
||||
const status = await runner.getStatus();
|
||||
const result = await runner.runOnce();
|
||||
|
||||
expect(status.credentials).toEqual([]);
|
||||
expect(status.sources).toEqual([
|
||||
expect.objectContaining({ sourceId: 'librechat-skills', credentialPresent: false }),
|
||||
expect.objectContaining({
|
||||
sourceId: 'stored-credential-skills',
|
||||
credentialPresent: false,
|
||||
}),
|
||||
]);
|
||||
expect(result.status).toBe('failed');
|
||||
expect(result.sources).toEqual([
|
||||
expect.objectContaining({
|
||||
sourceId: 'librechat-skills',
|
||||
status: 'failed',
|
||||
errorCode: 'MISSING_CREDENTIAL',
|
||||
}),
|
||||
expect.objectContaining({
|
||||
sourceId: 'stored-credential-skills',
|
||||
status: 'failed',
|
||||
errorCode: 'MISSING_CREDENTIAL',
|
||||
}),
|
||||
]);
|
||||
expect(listCredentials).not.toHaveBeenCalled();
|
||||
expect(getCredentialToken).not.toHaveBeenCalled();
|
||||
expect(deps.fetchFn).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
if (previousToken == null) {
|
||||
delete process.env.GITHUB_SKILLS_TOKEN;
|
||||
} else {
|
||||
process.env.GITHUB_SKILLS_TOKEN = previousToken;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves slash-delimited refs when fetching the GitHub commit', async () => {
|
||||
const baseFetch = githubFetch();
|
||||
const fetchFn = jest.fn(async (input: RequestInfo | URL) => {
|
||||
|
|
|
|||
|
|
@ -208,6 +208,7 @@ export type GitHubSkillSyncDeps = {
|
|||
}) => Promise<unknown>;
|
||||
fetchFn?: FetchFn;
|
||||
lockOwner?: string;
|
||||
allowServerCredentials?: boolean;
|
||||
};
|
||||
|
||||
export type GitHubSkillSyncRunResult = {
|
||||
|
|
@ -1178,6 +1179,9 @@ async function resolveGitHubToken(
|
|||
deps: GitHubSkillSyncDeps,
|
||||
source: SkillSyncGitHubSourceConfig,
|
||||
): Promise<string | null> {
|
||||
if (deps.allowServerCredentials === false) {
|
||||
return null;
|
||||
}
|
||||
const tokenEnvVar = getTokenEnvVarName(source.token);
|
||||
if (tokenEnvVar) {
|
||||
return process.env[tokenEnvVar]?.trim() || null;
|
||||
|
|
@ -1188,7 +1192,13 @@ async function resolveGitHubToken(
|
|||
return deps.getCredentialToken(PROVIDER, source.credentialKey);
|
||||
}
|
||||
|
||||
function getMissingCredentialMessage(source: SkillSyncGitHubSourceConfig): string {
|
||||
function getMissingCredentialMessage(
|
||||
source: SkillSyncGitHubSourceConfig,
|
||||
allowServerCredentials: boolean,
|
||||
): string {
|
||||
if (!allowServerCredentials) {
|
||||
return 'Server GitHub credentials are not available for this skill sync config';
|
||||
}
|
||||
const tokenEnvVar = getTokenEnvVarName(source.token);
|
||||
if (tokenEnvVar) {
|
||||
return `Missing GitHub token environment variable "${tokenEnvVar}"`;
|
||||
|
|
@ -1207,10 +1217,14 @@ async function syncSource(params: {
|
|||
await deps.upsertStatus(makeStatusInput({ source, status: 'running', startedAt }));
|
||||
try {
|
||||
assertNotCancelled();
|
||||
const allowServerCredentials = deps.allowServerCredentials !== false;
|
||||
const token = await resolveGitHubToken(deps, source);
|
||||
assertNotCancelled();
|
||||
if (!token) {
|
||||
throw new SkillSyncError('MISSING_CREDENTIAL', getMissingCredentialMessage(source));
|
||||
throw new SkillSyncError(
|
||||
'MISSING_CREDENTIAL',
|
||||
getMissingCredentialMessage(source, allowServerCredentials),
|
||||
);
|
||||
}
|
||||
const commit = await fetchCommit({ fetchFn, token, source });
|
||||
assertNotCancelled();
|
||||
|
|
@ -1466,9 +1480,10 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) {
|
|||
|
||||
async function getStatus() {
|
||||
const github = getGithubConfig(await deps.getConfig());
|
||||
const allowServerCredentials = deps.allowServerCredentials !== false;
|
||||
const [storedStatuses, credentials] = await Promise.all([
|
||||
deps.listStatuses(PROVIDER),
|
||||
deps.listCredentials(PROVIDER),
|
||||
allowServerCredentials ? deps.listCredentials(PROVIDER) : Promise.resolve([]),
|
||||
]);
|
||||
const statusBySourceId = new Map(
|
||||
storedStatuses.map((status) => [makeStatusKey(status.sourceId, status.tenantId), status]),
|
||||
|
|
@ -1478,9 +1493,13 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) {
|
|||
);
|
||||
const sources = github.sources.map((source) => {
|
||||
const stored = statusBySourceId.get(makeStatusKey(source.id, source.tenantId));
|
||||
const credential = source.credentialKey ? credentialByKey.get(source.credentialKey) : null;
|
||||
const credential =
|
||||
allowServerCredentials && source.credentialKey
|
||||
? credentialByKey.get(source.credentialKey)
|
||||
: null;
|
||||
const tokenEnvVar = getTokenEnvVarName(source.token);
|
||||
const envTokenPresent = tokenEnvVar ? Boolean(process.env[tokenEnvVar]?.trim()) : false;
|
||||
const envTokenPresent =
|
||||
allowServerCredentials && tokenEnvVar ? Boolean(process.env[tokenEnvVar]?.trim()) : false;
|
||||
return {
|
||||
provider: PROVIDER,
|
||||
sourceId: source.id,
|
||||
|
|
|
|||
|
|
@ -124,6 +124,7 @@ describe('createSkillSyncTriggerOrchestrator', () => {
|
|||
|
||||
const requestConfig = await runners[0].input.getConfig();
|
||||
expect(started).toBe(true);
|
||||
expect(runners[0].input.allowServerCredentials).toBe(false);
|
||||
expect(runners[0].runner.runOnce).toHaveBeenCalledTimes(1);
|
||||
expect(requestConfig?.github?.runOnStartup).toBe(false);
|
||||
expect(requestConfig?.github?.sources[0]).toEqual(
|
||||
|
|
@ -151,16 +152,30 @@ describe('createSkillSyncTriggerOrchestrator', () => {
|
|||
const runner = orchestrator.getRunnerForAdminRequest({
|
||||
config: { skillSync: config, config: {} },
|
||||
user: { tenantId: 'tenant-a' },
|
||||
skillSyncAllowServerCredentials: true,
|
||||
});
|
||||
const runnerConfig = await runners[0].input.getConfig();
|
||||
|
||||
expect(runner).toBe(runners[0].runner);
|
||||
expect(runners[0].input.allowServerCredentials).toBe(true);
|
||||
expect(runnerConfig?.github?.runOnStartup).toBe(true);
|
||||
expect(runnerConfig?.github?.sources[0]).toEqual(
|
||||
expect.objectContaining({ id: 'tenant-skills', tenantId: 'tenant-a' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not allow admin override runners to use server credentials by default', async () => {
|
||||
const config = skillSync();
|
||||
const { orchestrator, runners } = createHarness();
|
||||
|
||||
orchestrator.getRunnerForAdminRequest({
|
||||
config: { skillSync: config, config: {} },
|
||||
user: { tenantId: 'tenant-a' },
|
||||
});
|
||||
|
||||
expect(runners[0].input.allowServerCredentials).toBe(false);
|
||||
});
|
||||
|
||||
it('does not start request sync when the configured source is already running', async () => {
|
||||
const config = skillSync({ runOnStartup: false });
|
||||
const { orchestrator, runners } = createHarness({
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ type SkillSyncRequestUser = {
|
|||
type SkillSyncRequestLike = {
|
||||
config?: SkillSyncAppConfigLike;
|
||||
user?: SkillSyncRequestUser;
|
||||
skillSyncAllowServerCredentials?: boolean;
|
||||
};
|
||||
|
||||
type ResolvedSkillSyncConfig = NonNullable<SkillSyncConfig>;
|
||||
|
|
@ -39,6 +40,7 @@ type SkillSyncTriggerLogger = {
|
|||
export type SkillSyncTriggerRunnerFactoryInput = {
|
||||
getConfig: () => MaybePromise<SkillSyncConfig | undefined>;
|
||||
loadAppConfig: () => MaybePromise<SkillSyncAppConfigLike | undefined>;
|
||||
allowServerCredentials?: boolean;
|
||||
};
|
||||
|
||||
export type SkillSyncTriggerOrchestratorDeps = {
|
||||
|
|
@ -202,6 +204,7 @@ export function createSkillSyncTriggerOrchestrator(deps: SkillSyncTriggerOrchest
|
|||
return deps.createRunner({
|
||||
getConfig: async () => config,
|
||||
loadAppConfig: async () => request.config,
|
||||
allowServerCredentials: Boolean(request.skillSyncAllowServerCredentials),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -219,6 +222,7 @@ export function createSkillSyncTriggerOrchestrator(deps: SkillSyncTriggerOrchest
|
|||
const requestRunner = deps.createRunner({
|
||||
getConfig: async () => config,
|
||||
loadAppConfig: async () => request.config,
|
||||
allowServerCredentials: false,
|
||||
});
|
||||
const status = await requestRunner.getStatus();
|
||||
if (!shouldRunRequestSync(status, { minIntervalMs, staleRunningMs })) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue