diff --git a/api/server/services/Skills/sync.js b/api/server/services/Skills/sync.js index 6dbab2bebc..1f806c2109 100644 --- a/api/server/services/Skills/sync.js +++ b/api/server/services/Skills/sync.js @@ -4,9 +4,9 @@ const { createGitHubSkillSyncRunner, startGitHubSkillSyncScheduler, } = require('@librechat/api'); +const { runAsSystem } = require('@librechat/data-schemas'); const db = require('~/models'); const { getAppConfig } = require('~/server/services/Config'); -const { grantPermission } = require('~/server/services/PermissionService'); const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { getFileStrategy } = require('~/server/utils/getFileStrategy'); @@ -77,7 +77,38 @@ function createRunner() { upsertSkillFile: db.upsertSkillFile, deleteSkillFile: db.deleteSkillFile, deleteSkill: db.deleteSkill, - grantPermission, + grantPermission: async ({ + principalType, + principalId, + resourceType, + resourceId, + accessRoleId, + grantedBy, + }) => { + // Default access roles are seeded globally (no tenantId) under runAsSystem, + // but the runner may execute inside a source's tenant context. Resolve the + // role outside tenant isolation so the global role matches, then write the + // ACL entry in the active (tenant) context so tenant users can see it. + const role = await runAsSystem(() => db.findRoleByIdentifier(accessRoleId)); + if (!role) { + throw new Error(`Role ${accessRoleId} not found`); + } + if (role.resourceType !== resourceType) { + throw new Error( + `Role ${accessRoleId} is for ${role.resourceType} resources, not ${resourceType}`, + ); + } + return db.grantPermission( + principalType, + principalId, + resourceType, + resourceId, + role.permBits, + grantedBy, + undefined, + role._id, + ); + }, saveBuffer: async ({ userId, buffer, fileName, basePath, isImage, tenantId }) => { const storage = await resolveSkillStorage({ isImage }); const filepath = await storage.saveBuffer({ diff --git a/api/server/services/Skills/sync.test.js b/api/server/services/Skills/sync.test.js index da4991abfe..46901211d3 100644 --- a/api/server/services/Skills/sync.test.js +++ b/api/server/services/Skills/sync.test.js @@ -1,6 +1,8 @@ const mockGetAppConfig = jest.fn(); const mockGetStrategyFunctions = jest.fn(); const mockGetFileStrategy = jest.fn(); +const mockFindRoleByIdentifier = jest.fn(); +const mockGrantPermission = jest.fn(); let mockRunnerDeps; jest.mock('~/server/services/Config', () => ({ @@ -23,8 +25,10 @@ jest.mock('@librechat/data-schemas', () => ({ runAsSystem: jest.fn((fn) => fn()), })); -jest.mock('~/models', () => ({})); -jest.mock('~/server/services/PermissionService', () => ({ grantPermission: jest.fn() })); +jest.mock('~/models', () => ({ + findRoleByIdentifier: mockFindRoleByIdentifier, + grantPermission: mockGrantPermission, +})); jest.mock('~/server/services/Files/strategies', () => ({ getStrategyFunctions: mockGetStrategyFunctions, })); @@ -36,6 +40,8 @@ describe('GitHub skill sync service', () => { mockGetAppConfig.mockReset(); mockGetStrategyFunctions.mockReset(); mockGetFileStrategy.mockReset(); + mockFindRoleByIdentifier.mockReset(); + mockGrantPermission.mockReset(); mockRunnerDeps = undefined; }); @@ -133,4 +139,59 @@ describe('GitHub skill sync service', () => { expect(runAsSystem).not.toHaveBeenCalled(); }); + + it('resolves the access role outside tenant isolation but writes the ACL in context', async () => { + const { runAsSystem } = require('@librechat/data-schemas'); + mockGetAppConfig.mockResolvedValue({ skillSync: undefined, paths: {} }); + mockFindRoleByIdentifier.mockResolvedValue({ + _id: 'role-object-id', + resourceType: 'skill', + permBits: 1, + }); + mockGrantPermission.mockResolvedValue({ _id: 'acl-entry-id' }); + + const service = require('./sync'); + service.initializeGitHubSkillSync({ skillSync: undefined }); + await mockRunnerDeps.grantPermission({ + principalType: 'public', + principalId: null, + resourceType: 'skill', + resourceId: 'skill-id', + accessRoleId: 'skill_viewer', + grantedBy: 'system', + }); + + expect(runAsSystem).toHaveBeenCalledTimes(1); + expect(mockFindRoleByIdentifier).toHaveBeenCalledWith('skill_viewer'); + expect(mockGrantPermission).toHaveBeenCalledWith( + 'public', + null, + 'skill', + 'skill-id', + 1, + 'system', + undefined, + 'role-object-id', + ); + }); + + it('fails the grant when the access role does not exist', async () => { + mockGetAppConfig.mockResolvedValue({ skillSync: undefined, paths: {} }); + mockFindRoleByIdentifier.mockResolvedValue(null); + + const service = require('./sync'); + service.initializeGitHubSkillSync({ skillSync: undefined }); + + await expect( + mockRunnerDeps.grantPermission({ + principalType: 'public', + principalId: null, + resourceType: 'skill', + resourceId: 'skill-id', + accessRoleId: 'skill_viewer', + grantedBy: 'system', + }), + ).rejects.toThrow('Role skill_viewer not found'); + expect(mockGrantPermission).not.toHaveBeenCalled(); + }); });