🧬 feat: Add GitHub Skill Sync (#13293)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled

* feat: Add GitHub skill sync

* fix: Address GitHub skill sync CI

* fix: Harden GitHub skill sync review paths

* fix: Prevent overlapping skill sync runs

* fix: Address GitHub skill sync review findings

* fix: Satisfy Git ref lint rule

* fix: Address GitHub sync review follow-ups

* fix: Match skill frontmatter closing fence

* fix: Address GitHub sync review cycle

* fix: Address GitHub sync review follow-ups

* fix: Harden GitHub skill sync worker

* fix: Format GitHub sync rollback log

* fix: Address GitHub sync review feedback

* fix: Format skill import parse handling

* fix: Coerce scalar skill frontmatter and correct scheduler timer clear

- parse: coerce numeric/boolean name and description scalars to strings instead of dropping them to empty (restores pre-refactor behavior; preserves absent-vs-empty distinction for the when-to-use fallback)
- scheduler: clear the setTimeout handle with clearTimeout rather than clearInterval
- test: cover non-string scalar frontmatter coercion

* fix: Tolerate trailing whitespace after SKILL.md opening frontmatter fence

extractFrontmatterBlock required the opening fence to be exactly '---\n', so an opener with trailing spaces/tabs (e.g. '---   \n') silently dropped all frontmatter even though the closing-fence regex already tolerates it. Match the opener with /^---[ \t]*\n/ for symmetry. Addresses Codex P3 (parse.ts:24).

* feat: Run GitHub skill sync under a per-source tenant context

Under TENANT_ISOLATION_STRICT, the sync ran with no async tenant context, so the tenant-isolation mongoose hooks threw on every Skill/SkillFile/AclEntry operation; in non-strict mode synced skills were written tenant-less and never matched tenant-scoped reads. Add an optional per-source tenantId to the skillSync config; when set, each source sync runs inside tenantStorage.run({ tenantId }) so skills, files, and public ACL grants are created and listed within that tenant, and the skill row is stamped with the tenantId for correct dedup. Sources without tenantId keep the prior single-tenant behavior. Avoids runAsSystem. Addresses Codex P2 (sync.js:70).

Lock/status/credential bookkeeping stays outside the tenant context (those collections are intentionally global).

* test: Restore dropped tenant-context coverage for GitHub skill sync

The prior commit shipped the getTenantId import in github.spec.ts without the tenant tests that use it (lost in an interrupted edit), which failed the eslint --max-warnings=0 CI job on an unused import. Restore both github.spec.ts tenant tests (tenant-scoped run stamps tenantId and executes inside the tenant ALS context; no-tenant run stays ambient) and the two config-schemas tenant tests (accepts tenantId, rejects __SYSTEM__).

* test: Restore dropped github.spec tenant-context tests

The previous commit's github.spec.ts edit did not apply (anchor mismatch), so the getTenantId import remained unused and failed eslint --max-warnings=0. Add the two tenant tests that use it: a tenant-scoped run stamps tenantId and executes inside the tenant ALS context, and a no-tenant run stays ambient.

* feat: Scope synced skill author to tenant and harden tenant-context sync

Addresses the latest Codex review on the per-source tenant change:
- makeSourceAuthorId now folds tenantId into the synthetic author hash so the
  same source mirrored into different tenants gets distinct author ids (clearer
  audits, no cross-tenant author collisions). Single-tenant author ids stay
  stable (suffix omitted when tenantId is absent).
- syncSourceInTenantContext uses an async callback per the tenant-context
  contract so the ALS store propagates across awaited Mongoose calls.
- Tests: same-source/different-tenant yields distinct authors; mirror cleanup
  is scoped to the source and deletes only its absent-upstream skills.

* fix: Repair tsc error and guard external edits in github skill sync

- Fix TS2352 in github.spec mirror-cleanup test: build the existing-skill mock via makeSkill with authorName instead of an under-typed 'as CreateSkillInput' cast (this was the failing TypeScript CI check on f00ce3c5a).
- 808: commitExistingRemoteSkillAfterFileSync re-reads to clear our own file-sync version bumps, but now compares refreshed content against the pre-sync snapshot (body/name/description/always-apply) and throws SKILL_CONFLICT on a concurrent external edit instead of overwriting it.

* docs: Note skillSync source tenantId is effectively immutable

Changing/adding/removing a source's tenantId orphans previously mirrored skills in the old tenant (a tenant-scoped sync cannot clean another tenant's data without runAsSystem, which is intentionally avoided).

* fix: Key GitHub skill upstream identity on source id and path only

Addresses Codex finding (github.ts:217): makeUpstreamId previously included owner/repo, so repointing a source to a renamed or replacement repository (same source id) changed the upstreamId, made findSkillBySourceIdentity miss the existing mirror, and then collided on the (name, author, tenantId) uniqueness constraint — leaving the source stuck failing. Identity now keys on the stable source id + root path only. The feature is unreleased, so there is no stored-id migration. Updated spec upstreamId fixtures to the new format; the existing ref-independent identity test now also covers repo moves.

* fix: Scope GitHub skill mirror deletion to the source tenant

Addresses Codex P1 (github.ts:1047/1057): an ambient source (no tenantId) runs listSkillsBySource without tenant context, which under non-strict isolation returns github-synced skills across all tenants. The mirror-deletion pass then treated other tenants' skills as absent-upstream and could delete them. Filter existingSyncedSkills to rows whose tenantId matches the source's configured tenantId (absent = its own ambient bucket) before deleting, so a sync never removes another tenant's mirrored skills. Covered by a test where an ambient run leaves a tenant-b-owned skill untouched.

* fix: Apply tenant-scoped mirror deletion implementation

The prior commit (75ccfa3fc) added the test but the source change to github.ts was lost in an interrupted edit, leaving a failing test with no implementation. This adds the actual guard: the mirror-deletion pass skips skills whose tenantId does not match the source's configured tenantId (absent = ambient bucket), so an ambient source whose listSkillsBySource returns cross-tenant rows under non-strict isolation cannot delete another tenant's mirrored skills.

* fix: Resolve global access role outside tenant context for synced skill grants

Addresses Codex P2 (github.ts:1166): default access roles (incl. skill_viewer) are seeded globally with no tenantId under runAsSystem, but a tenant-scoped sync wraps ensurePublicViewer in the source's tenant context. The PermissionService grantPermission resolved the role via a tenant-isolated AccessRole query, so the global role did not match and tenant-scoped syncs failed with 'Role skill_viewer not found'. The sync adapter now resolves the role inside runAsSystem (matching the global seed) and writes the ACL entry in the active tenant context, so the AclEntry is tenant-scoped (visible to tenant users) while the role lookup still succeeds. Covered by service tests for the resolve-vs-write split and the missing-role failure.

* fix: Strip placeholder frontmatter booleans and check skill conflict before file sync

- 1083 (github.ts:759): toCleanFrontmatter now drops a non-boolean always-apply (e.g. the 'always-apply:' / 'always-apply: # TODO' placeholder, which js-yaml yields as null). The boolean is already captured in the dedicated alwaysApply field; persisting null left ambiguous frontmatter on the synced skill.
- 1080 (github.ts:1057): for an existing mirrored skill, check for an external content edit (via getSkillById + hasExternalSkillEdit) BEFORE syncSkillFiles mutates the bundled files, so a concurrently edited skill fails fast with SKILL_CONFLICT without partial file rewrites. The post-file-sync check still guards edits that land during the file sync window.
Tests: placeholder always-apply is dropped from synced frontmatter; concurrent-edit conflict leaves files unmutated (no upsert/delete).

* fix: Harden GitHub skill sync review paths

* fix: Reuse moved GitHub skill mirrors

* fix: Scope GitHub sync identity conflicts

* test: Fix GitHub sync conflict mock typing

* fix: Support nested env-backed skill sync

* fix: Keep skill sync config base-only

* fix: Scope GitHub skill identity lookup by tenant

* fix: Harden GitHub skill sync admin gates

* fix: Guard existing skill sync permission grants

* feat: Trigger skill sync from resolved config

* fix: Scope resolved skill sync by tenant

* test: Allow manual skill sync status tenant scoping

* refactor: Extract skill sync trigger orchestrator

* test: Complete orchestrator status fixture

* chore: Bump data provider version

* fix: Restrict skill sync server credentials

* test: Complete admin skill sync status fixtures

* fix: tighten skill sync trigger safeguards

* fix: preserve alwaysApply skill sync alias

* chore: sort skill sync imports

* fix: preserve skill sync request scope

* fix: harden skill sync review edges

* refactor: move skill sync admin access to api package

* fix: add skill sync declaration return types

* fix: satisfy skill sync type checks

* fix: resolve codex skill sync review findings

* fix: harden skill sync review edges

* fix: resolve codex skill sync edge findings

* fix: satisfy API declaration build after rebase
This commit is contained in:
Danny Avila 2026-06-10 21:05:54 -04:00 committed by GitHub
parent 470be2395f
commit 197a1dc4e2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
58 changed files with 8805 additions and 162 deletions

View file

@ -194,6 +194,29 @@ describe('createAdminConfigHandlers', () => {
expect(savedOverrides.interface).toEqual({ modelSelect: false });
});
it('preserves skillSync sections in admin overrides', async () => {
const { handlers, deps } = createHandlers({
upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }),
});
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: {
overrides: {
skillSync: { github: { enabled: true } },
interface: { modelSelect: false },
},
},
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(201);
const savedOverrides = deps.upsertConfig.mock.calls[0][3];
expect(savedOverrides.skillSync).toEqual({ github: { enabled: true } });
expect(savedOverrides.interface).toEqual({ modelSelect: false });
});
it('preserves UI sub-keys in composite permission fields like mcpServers', async () => {
const { handlers, deps } = createHandlers({
upsertConfig: jest.fn().mockResolvedValue({ _id: 'c1', configVersion: 1 }),
@ -338,6 +361,24 @@ describe('createAdminConfigHandlers', () => {
expect(deps.unsetConfigField).not.toHaveBeenCalled();
});
it('allows deleting skillSync field paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
query: { fieldPath: 'skillSync.github.enabled' },
});
const res = mockRes();
await handlers.deleteConfigField(req, res);
expect(res.statusCode).toBe(200);
expect(deps.unsetConfigField).toHaveBeenCalledWith(
'role',
'admin',
'skillSync.github.enabled',
);
});
it('allows deleting interface UI field paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
@ -489,6 +530,27 @@ describe('createAdminConfigHandlers', () => {
expect(patchedFields['interface.prompts']).toBeUndefined();
});
it('preserves skillSync field entries in patches', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: {
entries: [
{ fieldPath: 'skillSync.github.enabled', value: true },
{ fieldPath: 'interface.modelSelect', value: false },
],
},
});
const res = mockRes();
await handlers.patchConfigField(req, res);
expect(res.statusCode).toBe(200);
const patchedFields = deps.patchConfigFields.mock.calls[0][3];
expect(patchedFields['skillSync.github.enabled']).toBe(true);
expect(patchedFields['interface.modelSelect']).toBe(false);
});
it('blocks peoplePicker permission sub-key paths', async () => {
const { handlers, deps } = createHandlers();
const req = mockReq({

View file

@ -1,5 +1,6 @@
import { logger } from '@librechat/data-schemas';
import {
BASE_ONLY_CONFIG_SECTIONS,
PrincipalType,
PrincipalModel,
INTERFACE_PERMISSION_FIELDS,
@ -15,6 +16,7 @@ import type { ServerRequest } from '~/types/http';
const UNSAFE_SEGMENTS = /(?:^|\.)(__[\w]*|constructor|prototype)(?:\.|$)/;
const MAX_PATCH_ENTRIES = 100;
const DEFAULT_PRIORITY = 10;
const BASE_ONLY_OVERRIDE_SECTIONS = new Set<string>(BASE_ONLY_CONFIG_SECTIONS);
export function isValidFieldPath(path: string): boolean {
return (
@ -31,6 +33,10 @@ export function getTopLevelSection(fieldPath: string): string {
return fieldPath.split('.')[0];
}
function isBaseOnlyFieldPath(fieldPath: string): boolean {
return BASE_ONLY_OVERRIDE_SECTIONS.has(getTopLevelSection(fieldPath));
}
/**
* Returns true if `fieldPath` targets an interface permission field or permission sub-key.
*
@ -316,7 +322,17 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
return res.status(403).json({ error: 'Insufficient permissions' });
}
let filteredOverrides = overrides;
const filteredOverrides = {
...(overrides as Record<string, unknown>),
} as Partial<TCustomConfig>;
for (const section of BASE_ONLY_OVERRIDE_SECTIONS) {
if (section in filteredOverrides) {
delete (filteredOverrides as Record<string, unknown>)[section];
logger.warn(
`[adminConfig] Stripping base-only config section "${section}" - configure it in librechat.yaml instead`,
);
}
}
const iface = (overrides as Record<string, unknown>).interface;
if (iface != null && typeof iface === 'object' && !Array.isArray(iface)) {
const filteredIface: Record<string, unknown> = {};
@ -345,7 +361,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
);
}
}
filteredOverrides = { ...(overrides as Record<string, unknown>) } as Partial<TCustomConfig>;
if (Object.keys(filteredIface).length > 0) {
(filteredOverrides as Record<string, unknown>).interface = filteredIface;
} else {
@ -436,6 +451,12 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
}
const validEntries = entries.filter((entry) => {
if (isBaseOnlyFieldPath(entry.fieldPath)) {
logger.warn(
`[adminConfig] Stripping base-only config field "${entry.fieldPath}" - configure it in librechat.yaml instead`,
);
return false;
}
if (isInterfacePermissionPath(entry.fieldPath)) {
logger.warn(
`[adminConfig] Stripping interface permission field "${entry.fieldPath}" — use role permissions instead`,
@ -608,6 +629,13 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
});
}
if (isBaseOnlyFieldPath(fieldPath)) {
logger.warn(
`[adminConfig] Ignoring delete for base-only config field "${fieldPath}" - configure it in librechat.yaml instead`,
);
return res.status(200).json({ message: 'No actionable field path provided' });
}
if (isInterfacePermissionPath(fieldPath)) {
logger.warn(
`[adminConfig] Ignoring delete for interface permission field "${fieldPath}" — use role permissions instead`,

View file

@ -2,9 +2,11 @@ export { createAdminConfigHandlers } from './config';
export { createAdminGrantsHandlers } from './grants';
export { createAdminGroupsHandlers } from './groups';
export { createAdminRolesHandlers } from './roles';
export { createAdminSkillsSyncAccess, createAdminSkillsSyncHandlers } from './skills';
export { createAdminUsersHandlers } from './users';
export type { AdminConfigDeps } from './config';
export type { AdminGrantsDeps, GrantPrincipalType } from './grants';
export type { AdminGroupsDeps } from './groups';
export type { AdminRolesDeps } from './roles';
export type { AdminSkillSyncAccessDeps, AdminSkillSyncDeps } from './skills';
export type { AdminUsersDeps } from './users';

View file

@ -0,0 +1,363 @@
import { SystemCapabilities } from '@librechat/data-schemas';
import type { NextFunction, Response } from 'express';
import { createAdminSkillsSyncAccess, 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 createNext(): NextFunction & jest.Mock {
return jest.fn() as NextFunction & jest.Mock;
}
function createHandlers({
statusErrorCode,
statusErrorMessage,
}: { statusErrorCode?: string; statusErrorMessage?: string } = {}) {
const runner = {
getStatus: jest.fn(async () => ({
enabled: true,
intervalMinutes: 60,
runOnStartup: false,
sources: [
{
provider: 'github' as const,
sourceId: 'tenant-skills',
tenantId: 'tenant-a',
status: 'idle' as const,
credentialKey: 'github-skills-prod',
credentialPresent: true,
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
paths: ['skills'],
syncedSkillCount: 0,
syncedFileCount: 0,
deletedSkillCount: 0,
deletedFileCount: 0,
errorCode: statusErrorCode,
errorMessage: statusErrorMessage,
startedAt: undefined,
finishedAt: undefined,
lastSuccessAt: undefined,
lastFailureAt: undefined,
createdAt: undefined,
updatedAt: undefined,
},
],
credentials: [
{
provider: 'github' as const,
credentialKey: 'github-skills-prod',
credentialPresent: true,
tokenFingerprint: 'abc123',
},
],
fineGrainedTokenRecommendation: 'Use a fine-grained token.',
})),
runOnce: jest.fn(async () => ({
status: 'completed' as const,
sources: [
{
provider: 'github' as const,
sourceId: 'tenant-skills',
tenantId: 'tenant-a',
status: 'succeeded' as const,
credentialKey: 'github-skills-prod',
credentialPresent: true,
syncedSkillCount: 1,
syncedFileCount: 2,
deletedSkillCount: 0,
deletedFileCount: 0,
errorCode: statusErrorCode,
errorMessage: statusErrorMessage,
startedAt: undefined,
finishedAt: undefined,
lastSuccessAt: undefined,
lastFailureAt: undefined,
createdAt: undefined,
updatedAt: undefined,
},
],
})),
};
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,
owner: undefined,
repo: undefined,
ref: undefined,
paths: undefined,
}),
],
}),
);
});
it('redacts credential-related errors from tenant-scoped status reads', async () => {
const { handlers } = createHandlers({
statusErrorCode: 'MISSING_CREDENTIAL',
statusErrorMessage: 'Missing GitHub token environment variable "GITHUB_SKILLS_TOKEN"',
});
const res = createResponse();
await handlers.getSyncStatus({ skillSyncCanReadCredentials: false } as never, res);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({
sources: [
expect.objectContaining({
errorCode: 'MISSING_CREDENTIAL',
errorMessage: 'GitHub skill sync credentials are not available',
}),
],
}),
);
});
it('includes credential summaries and source credential metadata for platform status reads', async () => {
const { handlers } = createHandlers({
statusErrorCode: 'MISSING_CREDENTIAL',
statusErrorMessage: 'Missing GitHub credential "github-skills-prod"',
});
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,
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
paths: ['skills'],
errorMessage: 'Missing GitHub credential "github-skills-prod"',
}),
],
}),
);
});
it('omits source credential metadata from tenant-scoped manual run responses', async () => {
const { handlers } = createHandlers({
statusErrorCode: 'MISSING_CREDENTIAL',
statusErrorMessage: 'Missing GitHub credential "github-skills-prod"',
});
const res = createResponse();
await handlers.runSync(
{
skillSyncAllowServerCredentials: true,
skillSyncCanReadCredentials: false,
} as never,
res,
);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({
sources: [
expect.objectContaining({
credentialKey: undefined,
credentialPresent: false,
errorMessage: 'GitHub skill sync credentials are not available',
}),
],
}),
);
});
});
describe('createAdminSkillsSyncAccess', () => {
const baseSkillSync = {
github: {
enabled: true,
intervalMinutes: 60,
runOnStartup: false,
sources: [
{
id: 'base-skills',
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
paths: ['skills'],
token: '${GITHUB_SKILLS_TOKEN}',
},
],
},
};
function createAccess({
hasCapability = jest.fn().mockResolvedValue(true),
getAppConfig = jest.fn().mockResolvedValue({ skillSync: undefined }),
}: {
hasCapability?: jest.Mock;
getAppConfig?: jest.Mock;
} = {}) {
return {
access: createAdminSkillsSyncAccess({ getAppConfig, hasCapability }),
getAppConfig,
hasCapability,
};
}
it('attaches the base skill sync config for override comparison', async () => {
const getAppConfig = jest.fn().mockResolvedValue({ skillSync: baseSkillSync });
const { access } = createAccess({ getAppConfig });
const req = { config: { skillSync: undefined, config: { endpoints: {} } } };
const res = createResponse();
const next = createNext();
await access.attachBaseSkillSyncConfig(req as never, res, next);
expect(getAppConfig).toHaveBeenCalledWith({ baseOnly: true });
expect(req.config.config).toEqual({ endpoints: {}, skillSync: baseSkillSync });
expect(next).toHaveBeenCalledTimes(1);
});
it('marks credential metadata hidden for tenant-scoped status reads', async () => {
const hasCapability = jest.fn(
async (user: { tenantId?: string }, capability: string): Promise<boolean> => {
if (capability === SystemCapabilities.READ_SKILLS) {
return Boolean(user.tenantId);
}
return true;
},
);
const { access } = createAccess({ hasCapability });
const req = { user: { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' } };
const res = createResponse();
const next = createNext();
await access.requireReadSkills(req as never, res, next);
await access.attachCredentialReadAccess(req as never, res, next);
expect(req).toMatchObject({
skillSyncCanReadCredentials: false,
skillSyncAllowServerCredentials: false,
});
expect(hasCapability).toHaveBeenCalledWith(
{ id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
SystemCapabilities.READ_SKILLS,
);
expect(hasCapability).toHaveBeenCalledWith(
{ id: 'user-1', role: 'ADMIN' },
SystemCapabilities.READ_SKILLS,
);
});
it('prevents tenant admins from running overrides that require server credentials', async () => {
const tenantSkillSync = {
github: {
enabled: true,
intervalMinutes: 60,
runOnStartup: false,
sources: [
{
id: 'tenant-skills',
owner: 'LibreChat',
repo: 'skills',
ref: 'main',
paths: ['skills'],
token: '${GITHUB_SKILLS_TOKEN}',
},
],
},
};
const hasCapability = jest.fn(
async (user: { tenantId?: string }, capability: string): Promise<boolean> => {
if (capability === SystemCapabilities.MANAGE_SKILLS) {
return Boolean(user.tenantId);
}
return true;
},
);
const { access } = createAccess({ hasCapability });
const req = {
user: { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
config: { skillSync: tenantSkillSync, config: {} },
};
const res = createResponse();
const next = createNext();
await access.requireSyncRunCapability(req as never, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.json).toHaveBeenCalledWith({
message: 'Tenant-scoped manual skill sync requires platform credential access',
});
expect(next).not.toHaveBeenCalled();
});
it('prevents tenant admins from manually running base skill sync config', async () => {
const hasCapability = jest.fn(
async (user: { tenantId?: string }, capability: string): Promise<boolean> => {
if (capability === SystemCapabilities.MANAGE_SKILLS) {
return Boolean(user.tenantId);
}
return true;
},
);
const { access } = createAccess({ hasCapability });
const req = {
user: { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
config: { skillSync: baseSkillSync, config: { skillSync: baseSkillSync } },
};
const res = createResponse();
const next = createNext();
await access.requireSyncRunCapability(req as never, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.json).toHaveBeenCalledWith({ message: 'Forbidden' });
expect(next).not.toHaveBeenCalled();
});
it('allows platform admins to manually run base skill sync config with server credentials', async () => {
const { access } = createAccess();
const req = {
user: { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' },
config: { skillSync: baseSkillSync, config: { skillSync: baseSkillSync } },
};
const res = createResponse();
const next = createNext();
await access.requireSyncRunCapability(req as never, res, next);
expect(req).toMatchObject({
skillSyncAllowServerCredentials: true,
skillSyncCanReadCredentials: true,
});
expect(next).toHaveBeenCalledTimes(1);
});
});

View file

@ -0,0 +1,390 @@
import { SystemCapabilities } from '@librechat/data-schemas';
import { skillSyncConfigSchema } from 'librechat-data-provider';
import type {
TGitHubSkillSyncStatusResponse,
TGitHubSkillSyncSourceStatus,
TGitHubSkillSyncCredentialSummary,
TGitHubSkillSyncManualRunResponse,
SkillSyncConfig,
} from 'librechat-data-provider';
import type {
ISkillSyncStatus,
SkillSyncProvider,
SkillSyncCredentialSummary,
UpsertSkillSyncCredentialInput,
SystemCapability,
} from '@librechat/data-schemas';
import type { NextFunction, Request, RequestHandler, Response } from 'express';
import type { Types } from 'mongoose';
import type { GitHubSkillSyncRunner } from '~/skills/sync';
export type AdminSkillsRequest = Request & {
user?: {
_id?: Types.ObjectId;
id?: string;
};
skillSyncAllowServerCredentials?: boolean;
skillSyncCanReadCredentials?: boolean;
};
type SkillSyncConfigContainer = {
skillSync?: unknown;
config?: {
skillSync?: unknown;
} & Record<string, unknown>;
} & Record<string, unknown>;
type AdminSkillSyncAccessRequest = Request & {
user?: {
_id?: Types.ObjectId | { toString(): string };
id?: string;
role?: string;
tenantId?: string;
};
config?: SkillSyncConfigContainer;
skillSyncAllowServerCredentials?: boolean;
skillSyncCanReadCredentials?: boolean;
};
type SkillSyncCapabilityUser = {
id: string;
role: string;
tenantId?: string;
};
export type AdminSkillSyncDeps = {
runner?: GitHubSkillSyncRunner;
getRunner?: (req: Request) => GitHubSkillSyncRunner;
upsertCredential: (input: UpsertSkillSyncCredentialInput) => Promise<SkillSyncCredentialSummary>;
deleteCredential: (
provider: SkillSyncProvider,
credentialKey: string,
) => Promise<{ deleted: boolean }>;
};
export type AdminSkillSyncAccessDeps = {
getAppConfig: (options: { baseOnly: true }) => Promise<{ skillSync?: unknown } | undefined>;
hasCapability: (user: SkillSyncCapabilityUser, capability: SystemCapability) => Promise<boolean>;
};
type AdminSkillsSyncHandler = (req: AdminSkillsRequest, res: Response) => Promise<Response>;
export type AdminSkillsSyncHandlers = {
getSyncStatus: AdminSkillsSyncHandler;
runSync: AdminSkillsSyncHandler;
setCredential: AdminSkillsSyncHandler;
deleteCredential: (req: Request, res: Response) => Promise<Response>;
};
export type AdminSkillsSyncAccess = {
attachBaseSkillSyncConfig: RequestHandler;
attachCredentialReadAccess: RequestHandler;
requireReadSkills: RequestHandler;
requirePlatformManageSkills: RequestHandler;
requireSyncRunCapability: RequestHandler;
};
const CREDENTIAL_KEY_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}$/;
function toIso(date: Date | undefined): string | undefined {
return date ? date.toISOString() : undefined;
}
function serializeCredential(
credential: SkillSyncCredentialSummary,
): TGitHubSkillSyncCredentialSummary {
return {
provider: credential.provider,
credentialKey: credential.credentialKey,
credentialPresent: credential.credentialPresent,
tokenFingerprint: credential.tokenFingerprint,
createdAt: toIso(credential.createdAt),
updatedAt: toIso(credential.updatedAt),
};
}
function isCredentialError(status: ISkillSyncStatus): boolean {
if (status.errorCode === 'MISSING_CREDENTIAL') {
return true;
}
return /credential|token environment variable|server github credentials/i.test(
status.errorMessage ?? '',
);
}
function serializeErrorMessage(
status: ISkillSyncStatus,
{ includeCredentialMetadata }: { includeCredentialMetadata: boolean },
): string | undefined {
if (includeCredentialMetadata || !isCredentialError(status)) {
return status.errorMessage;
}
return 'GitHub skill sync credentials are not available';
}
function serializeSourceStatus(
status: ISkillSyncStatus & { credentialPresent?: boolean },
{ includeCredentialMetadata = true }: { includeCredentialMetadata?: boolean } = {},
): TGitHubSkillSyncSourceStatus {
const includePrivateSourceMetadata = includeCredentialMetadata;
return {
provider: status.provider,
sourceId: status.sourceId,
tenantId: status.tenantId,
status: status.status,
credentialKey: includeCredentialMetadata ? status.credentialKey : undefined,
credentialPresent: includeCredentialMetadata ? (status.credentialPresent ?? false) : false,
owner: includePrivateSourceMetadata ? status.owner : undefined,
repo: includePrivateSourceMetadata ? status.repo : undefined,
ref: includePrivateSourceMetadata ? status.ref : undefined,
paths: includePrivateSourceMetadata ? status.paths : undefined,
startedAt: toIso(status.startedAt),
finishedAt: toIso(status.finishedAt),
lastSuccessAt: toIso(status.lastSuccessAt),
lastFailureAt: toIso(status.lastFailureAt),
errorCode: status.errorCode,
errorMessage: serializeErrorMessage(status, { includeCredentialMetadata }),
syncedSkillCount: status.syncedSkillCount,
syncedFileCount: status.syncedFileCount,
deletedSkillCount: status.deletedSkillCount,
deletedFileCount: status.deletedFileCount,
createdAt: toIso(status.createdAt),
updatedAt: toIso(status.updatedAt),
};
}
function isCredentialKey(value: unknown): value is string {
return typeof value === 'string' && CREDENTIAL_KEY_PATTERN.test(value);
}
function getUserObjectId(req: AdminSkillsRequest): Types.ObjectId | undefined {
return req.user?._id;
}
function getCapabilityUser(
req: AdminSkillSyncAccessRequest,
{ platformOnly = false }: { platformOnly?: boolean } = {},
): SkillSyncCapabilityUser | null {
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: unknown): SkillSyncConfig | undefined {
if (!raw || typeof raw !== 'object') {
return undefined;
}
const parsed = skillSyncConfigSchema.safeParse(raw);
return parsed.success ? parsed.data : undefined;
}
function isSameSkillSyncConfig(left: SkillSyncConfig, right: SkillSyncConfig): boolean {
return JSON.stringify(left ?? null) === JSON.stringify(right ?? null);
}
function hasResolvedSkillSyncOverride(req: AdminSkillSyncAccessRequest): boolean {
const resolved = parseSkillSyncConfig(req.config?.skillSync);
const base = parseSkillSyncConfig(req.config?.config?.skillSync);
return Boolean(resolved?.github && !isSameSkillSyncConfig(resolved, base));
}
function sendInternalServerError(res: Response): void {
res.status(500).json({ message: 'Internal Server Error' });
}
export function createAdminSkillsSyncAccess(deps: AdminSkillSyncAccessDeps): AdminSkillsSyncAccess {
async function hasSkillCapability(
req: AdminSkillSyncAccessRequest,
capability: SystemCapability,
{ platformOnly = false }: { platformOnly?: boolean } = {},
): Promise<boolean> {
const user = getCapabilityUser(req, { platformOnly });
if (!user) {
return false;
}
return deps.hasCapability(user, capability);
}
function requireSkillCapability(
capability: SystemCapability,
{ platformOnly = false }: { platformOnly?: boolean } = {},
): RequestHandler {
return async (
req: AdminSkillSyncAccessRequest,
res: Response,
next: NextFunction,
): Promise<void> => {
try {
const user = getCapabilityUser(req, { platformOnly });
if (!user) {
res.status(401).json({ message: 'Authentication required' });
return;
}
if (await deps.hasCapability(user, capability)) {
next();
return;
}
res.status(403).json({ message: 'Forbidden' });
} catch {
sendInternalServerError(res);
}
};
}
const attachBaseSkillSyncConfig: RequestHandler = async (
req: AdminSkillSyncAccessRequest,
res: Response,
next: NextFunction,
): Promise<void> => {
try {
const baseConfig = await deps.getAppConfig({ baseOnly: true });
const existingConfig = req.config ?? {};
req.config = {
...existingConfig,
config: {
...(existingConfig.config ?? {}),
skillSync: baseConfig?.skillSync,
},
};
next();
} catch {
sendInternalServerError(res);
}
};
const attachCredentialReadAccess: RequestHandler = async (
req: AdminSkillSyncAccessRequest,
res: Response,
next: NextFunction,
): Promise<void> => {
try {
const canReadCredentials = await hasSkillCapability(req, SystemCapabilities.READ_SKILLS, {
platformOnly: true,
});
req.skillSyncCanReadCredentials = canReadCredentials;
req.skillSyncAllowServerCredentials = canReadCredentials;
next();
} catch {
sendInternalServerError(res);
}
};
const requireSyncRunCapability: RequestHandler = async (
req: AdminSkillSyncAccessRequest,
res: Response,
next: NextFunction,
): Promise<void> => {
try {
const canManagePlatform = await hasSkillCapability(req, SystemCapabilities.MANAGE_SKILLS, {
platformOnly: true,
});
if (canManagePlatform) {
req.skillSyncAllowServerCredentials = true;
req.skillSyncCanReadCredentials = true;
next();
return;
}
if (
hasResolvedSkillSyncOverride(req) &&
(await hasSkillCapability(req, SystemCapabilities.MANAGE_SKILLS))
) {
res.status(403).json({
message: 'Tenant-scoped manual skill sync requires platform credential access',
});
return;
}
res.status(403).json({ message: 'Forbidden' });
} catch {
sendInternalServerError(res);
}
};
return {
attachBaseSkillSyncConfig,
attachCredentialReadAccess,
requireReadSkills: requireSkillCapability(SystemCapabilities.READ_SKILLS),
requirePlatformManageSkills: requireSkillCapability(SystemCapabilities.MANAGE_SKILLS, {
platformOnly: true,
}),
requireSyncRunCapability,
};
}
export function createAdminSkillsSyncHandlers(deps: AdminSkillSyncDeps): AdminSkillsSyncHandlers {
function getRunner(req: Request): GitHubSkillSyncRunner {
const runner = deps.getRunner?.(req) ?? deps.runner;
if (!runner) {
throw new Error('GitHub skill sync runner is not configured');
}
return runner;
}
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((source) =>
serializeSourceStatus(source, { includeCredentialMetadata }),
),
credentials: includeCredentialMetadata ? status.credentials.map(serializeCredential) : [],
fineGrainedTokenRecommendation: status.fineGrainedTokenRecommendation,
};
return res.status(200).json(response);
}
async function runSync(req: AdminSkillsRequest, res: Response) {
const includeCredentialMetadata = req.skillSyncCanReadCredentials === true;
const result = await getRunner(req).runOnce();
const response: TGitHubSkillSyncManualRunResponse = {
status: result.status,
message: result.message,
sources: result.sources.map((source) =>
serializeSourceStatus(source, { includeCredentialMetadata }),
),
};
return res.status(result.status === 'skipped' ? 202 : 200).json(response);
}
async function setCredential(req: AdminSkillsRequest, res: Response) {
const { credentialKey } = req.params;
if (!isCredentialKey(credentialKey)) {
return res.status(400).json({ error: 'Invalid credential key' });
}
const token = (req.body as { token?: unknown } | undefined)?.token;
if (typeof token !== 'string' || token.trim().length === 0) {
return res.status(400).json({ error: 'GitHub token is required' });
}
const credential = await deps.upsertCredential({
provider: 'github',
credentialKey,
token: token.trim(),
userId: getUserObjectId(req),
});
return res.status(200).json(serializeCredential(credential));
}
async function deleteCredential(req: Request, res: Response) {
const { credentialKey } = req.params;
if (!isCredentialKey(credentialKey)) {
return res.status(400).json({ error: 'Invalid credential key' });
}
const result = await deps.deleteCredential('github', credentialKey);
return res.status(200).json({ credentialKey, deleted: result.deleted });
}
return {
getSyncStatus,
runSync,
setCredential,
deleteCredential,
};
}