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).
This commit is contained in:
Danny Avila 2026-05-30 12:03:17 -04:00
parent f8b794725c
commit 7d688b9b7e
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
4 changed files with 61 additions and 18 deletions

View file

@ -79,6 +79,10 @@ cache: true
# paths:
# - skills
# credentialKey: github-skills-prod
# # Optional. Owns the mirrored skills under the given tenant so they are
# # created and shared within that tenant. Required for visibility when
# # tenant isolation is enabled. Omit for single-tenant deployments.
# # tenantId: your-tenant-id
# Custom interface configuration
interface:
@ -166,23 +170,23 @@ interface:
# public: false
# MCP Servers configuration example
# mcpServers:
# Controls user permissions for MCP (Model Context Protocol) server management
# - use: Allow users to use configured MCP servers
# - create: Allow users to create and manage new MCP servers
# - share: Allow users to share MCP servers with other users
# - public: Allow users to share MCP servers publicly (with everyone)
# Controls user permissions for MCP (Model Context Protocol) server management
# - use: Allow users to use configured MCP servers
# - create: Allow users to create and manage new MCP servers
# - share: Allow users to share MCP servers with other users
# - public: Allow users to share MCP servers publicly (with everyone)
# Creation / edit MCP server config Dialog config example
# trustCheckbox:
# label:
# en: 'I understand and I want to continue'
# de: 'Ich verstehe und möchte fortfahren'
# de-DE: 'Ich verstehe und möchte fortfahren' # You can narrow translation to regions like (de-DE or de-CH)
# subLabel:
# en: |
# Librechat hasn't reviewed this MCP server. Attackers may attempt to steal your data or trick the model into taking unintended actions, including destroying data. <a href="https://google.de" target="_blank"><strong>Learn more.</strong></a>
# de: |
# LibreChat hat diesen MCP-Server nicht überprüft. Angreifer könnten versuchen, Ihre Daten zu stehlen oder das Modell zu unbeabsichtigten Aktionen zu verleiten, einschließlich der Zerstörung von Daten. <a href="https://google.de" target="_blank"><strong>Mehr erfahren.</strong></a>
# Creation / edit MCP server config Dialog config example
# trustCheckbox:
# label:
# en: 'I understand and I want to continue'
# de: 'Ich verstehe und möchte fortfahren'
# de-DE: 'Ich verstehe und möchte fortfahren' # You can narrow translation to regions like (de-DE or de-CH)
# subLabel:
# en: |
# Librechat hasn't reviewed this MCP server. Attackers may attempt to steal your data or trick the model into taking unintended actions, including destroying data. <a href="https://google.de" target="_blank"><strong>Learn more.</strong></a>
# de: |
# LibreChat hat diesen MCP-Server nicht überprüft. Angreifer könnten versuchen, Ihre Daten zu stehlen oder das Modell zu unbeabsichtigten Aktionen zu verleiten, einschließlich der Zerstörung von Daten. <a href="https://google.de" target="_blank"><strong>Mehr erfahren.</strong></a>
# Temporary chat retention period in hours (default: 720, min: 1, max: 8760)
# temporaryChatRetention: 1

View file

@ -7,6 +7,7 @@ import type {
ISkillSyncStatus,
SkillSyncStatusInput,
} from '@librechat/data-schemas';
import { getTenantId } from '@librechat/data-schemas';
import { DEFAULT_SKILL_IMPORT_LIMITS } from '../limits';
import { createGitHubSkillSyncRunner } from './github';
import type { GitHubSkillSyncDeps } from './github';

View file

@ -2,7 +2,7 @@ import crypto from 'crypto';
import path from 'path';
import { Types } from 'mongoose';
import { ResourceType, PrincipalType, AccessRoleIds } from 'librechat-data-provider';
import { logger } from '@librechat/data-schemas';
import { logger, tenantStorage } from '@librechat/data-schemas';
import type { SkillSyncConfig, SkillSyncGitHubSourceConfig } from 'librechat-data-provider';
import type {
ISkill,
@ -761,6 +761,7 @@ async function prepareRemoteSkill(params: {
author: makeSourceAuthorId(source),
authorName: SYSTEM_AUTHOR_NAME,
source: PROVIDER,
tenantId: source.tenantId,
};
return { existing, update, createInput };
}
@ -1104,6 +1105,24 @@ async function syncSource(params: {
}
}
/**
* Runs a source sync inside its tenant's async context when `tenantId` is set,
* so the tenant-isolation mongoose hooks scope every skill/file/ACL read and
* write to that tenant (required under strict isolation). Without a configured
* tenant the sync runs in the ambient context, preserving single-tenant behavior.
*/
function syncSourceInTenantContext(params: {
deps: GitHubSkillSyncDeps;
source: SkillSyncGitHubSourceConfig;
fetchFn: FetchFn;
assertNotCancelled: AssertNotCancelled;
}): Promise<ISkillSyncStatus> {
if (!params.source.tenantId) {
return syncSource(params);
}
return tenantStorage.run({ tenantId: params.source.tenantId }, () => syncSource(params));
}
function getGithubConfig(config: SkillSyncConfig | undefined): {
enabled: boolean;
intervalMinutes: number;
@ -1218,7 +1237,9 @@ export function createGitHubSkillSyncRunner(deps: GitHubSkillSyncDeps) {
if (lockLost) {
break;
}
sources.push(await syncSource({ deps, source, fetchFn, assertNotCancelled }));
sources.push(
await syncSourceInTenantContext({ deps, source, fetchFn, assertNotCancelled }),
);
}
const failed = sources.some((source) => source.status === 'failed');
return {

View file

@ -327,6 +327,22 @@ const skillSyncPathSchema = z
},
);
/**
* Tenant that owns the skills mirrored from a source. When set, the sync runner
* executes that source's database writes inside the tenant's async context so
* synced skills are created, listed, and shared within the tenant under strict
* tenant isolation. Mirrors the request tenant-id contract: no reserved system id.
*/
const skillSyncTenantIdSchema = z
.string()
.max(128)
.refine((value) => /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/.test(value), {
message: 'must be a valid tenant id',
})
.refine((value) => value !== '__SYSTEM__', {
message: 'must not be the reserved system tenant id',
});
export const skillSyncGitHubSourceSchema = z.object({
id: skillSyncIdentifierSchema,
owner: skillSyncGitHubOwnerSchema,
@ -334,6 +350,7 @@ export const skillSyncGitHubSourceSchema = z.object({
ref: skillSyncGitHubRefSchema.default('main'),
paths: z.array(skillSyncPathSchema).min(1),
credentialKey: skillSyncIdentifierSchema,
tenantId: skillSyncTenantIdSchema.optional(),
});
export const skillSyncConfigSchema = z