diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 536044e5cc..8dbe881fbb 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -59,6 +59,39 @@ const getSafeModelParameters = (modelParameters) => { const { useResponsesApi } = modelParameters ?? {}; return typeof useResponsesApi === 'boolean' ? { useResponsesApi } : {}; }; +const hasEditBit = (permission) => (permission & PermissionBits.EDIT) === PermissionBits.EDIT; + +const sanitizeViewerSkillScope = (agent, accessibleSkillSet) => { + const skillScopeEnabled = agent.skills_enabled === true; + delete agent.skills_enabled; + + if (!skillScopeEnabled) { + delete agent.skills; + return agent; + } + + const configuredSkills = Array.isArray(agent.skills) ? agent.skills : []; + if (configuredSkills.length === 0) { + delete agent.skills; + if (accessibleSkillSet.size > 0) { + agent.skills_enabled = true; + } + return agent; + } + + const visibleSkills = configuredSkills + .map((skillId) => String(skillId)) + .filter((skillId) => accessibleSkillSet.has(skillId)); + + if (visibleSkills.length === 0) { + delete agent.skills; + return agent; + } + + agent.skills = visibleSkills; + agent.skills_enabled = true; + return agent; +}; /** * Looks up each referenced agent id in Mongo, splits them into three @@ -848,6 +881,7 @@ const getListAgentsHandler = async (req, res) => { } else if (typeof requiredPermission !== 'number') { requiredPermission = PermissionBits.VIEW; } + const canReturnSkillConfig = hasEditBit(requiredPermission); // Base filter const filter = {}; @@ -921,6 +955,7 @@ const getListAgentsHandler = async (req, res) => { otherParams: filter, limit, after: cursor, + includeSkillConfig: true, }); const agents = data?.data ?? []; @@ -928,10 +963,24 @@ const getListAgentsHandler = async (req, res) => { return res.json(data); } + let accessibleSkillSet = null; + if (!canReturnSkillConfig) { + const accessibleSkillIds = await findAccessibleResources({ + userId, + role: req.user.role, + resourceType: ResourceType.SKILL, + requiredPermissions: PermissionBits.VIEW, + }); + accessibleSkillSet = new Set(accessibleSkillIds.map((oid) => oid.toString())); + } + const publicSet = new Set(publiclyAccessibleIds.map((oid) => oid.toString())); const urlCache = cachedRefresh?.urlCache; data.data = agents.map((agent) => { + if (accessibleSkillSet) { + sanitizeViewerSkillScope(agent, accessibleSkillSet); + } try { if (agent?._id && publicSet.has(agent._id.toString())) { agent.isPublic = true; diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index 17904ad3fd..f36152abec 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -2,7 +2,7 @@ const mongoose = require('mongoose'); const { nanoid } = require('nanoid'); const { v4: uuidv4 } = require('uuid'); const { agentSchema, fileSchema } = require('@librechat/data-schemas'); -const { FileSources, PermissionBits } = require('librechat-data-provider'); +const { FileSources, PermissionBits, ResourceType } = require('librechat-data-provider'); const { MongoMemoryServer } = require('mongodb-memory-server'); // Only mock the dependencies that are not database-related @@ -1303,6 +1303,68 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(response.data[0].name).toBe('Agent A1'); }); + test('should return only expected safe list fields for VIEW callers', async () => { + const hiddenSkillId = new mongoose.Types.ObjectId(); + await Agent.findByIdAndUpdate(agentA1._id, { + avatar: { filepath: '/avatars/a1.png', source: FileSources.local }, + category: 'general', + support_contact: { name: 'Support', email: 'support@example.com' }, + is_promoted: true, + instructions: 'private system instructions', + tools: ['execute_code'], + actions: ['example.com::action'], + model_parameters: { temperature: 0.7 }, + tool_resources: { file_search: { file_ids: ['file-1'] } }, + tool_options: { execute_code: { defer_loading: true } }, + subagents: { enabled: true, agent_ids: [agentA2.id] }, + edges: [{ from: agentA1.id, to: agentA2.id }], + skills_enabled: true, + skills: [hiddenSkillId.toString()], + }); + + mockReq.user.id = userB.toString(); + mockReq.query.requiredPermission = String(PermissionBits.VIEW); + findAccessibleResources.mockImplementation(({ resourceType }) => { + if (resourceType === ResourceType.AGENT) { + return Promise.resolve([agentA1._id]); + } + if (resourceType === ResourceType.SKILL) { + return Promise.resolve([]); + } + return Promise.resolve([]); + }); + findPubliclyAccessibleResources.mockResolvedValue([]); + + await getListAgentsHandler(mockReq, mockRes); + + const response = mockRes.json.mock.calls[0][0]; + const agent = response.data[0]; + expect(Object.keys(agent).sort()).toEqual( + [ + '_id', + 'author', + 'avatar', + 'category', + 'description', + 'id', + 'is_promoted', + 'name', + 'support_contact', + 'updatedAt', + ].sort(), + ); + expect(agent).toEqual( + expect.objectContaining({ + id: agentA1.id, + name: 'Agent A1', + description: 'User A agent 1', + author: userA.toString(), + category: 'general', + is_promoted: true, + }), + ); + }); + test('should return multiple accessible agents', async () => { // User B has access to multiple agents mockReq.user.id = userB.toString(); @@ -1428,6 +1490,91 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(response.data).toHaveLength(1); }); + test('should return only viewer-accessible skill scope for VIEW list callers', async () => { + const visibleSkillId = new mongoose.Types.ObjectId(); + const hiddenSkillId = new mongoose.Types.ObjectId(); + await Agent.findByIdAndUpdate(agentA1._id, { + skills_enabled: true, + skills: [visibleSkillId.toString(), hiddenSkillId.toString()], + }); + + mockReq.user.id = userB.toString(); + mockReq.query.requiredPermission = String(PermissionBits.VIEW); + findAccessibleResources.mockImplementation(({ resourceType }) => { + if (resourceType === ResourceType.AGENT) { + return Promise.resolve([agentA1._id]); + } + if (resourceType === ResourceType.SKILL) { + return Promise.resolve([visibleSkillId]); + } + return Promise.resolve([]); + }); + findPubliclyAccessibleResources.mockResolvedValue([]); + + await getListAgentsHandler(mockReq, mockRes); + + const response = mockRes.json.mock.calls[0][0]; + expect(response.data).toHaveLength(1); + expect(response.data[0].skills_enabled).toBe(true); + expect(response.data[0].skills).toEqual([visibleSkillId.toString()]); + expect(response.data[0].skills).not.toContain(hiddenSkillId.toString()); + }); + + test('should omit skill scope for VIEW list callers with no accessible configured skills', async () => { + const hiddenSkillId = new mongoose.Types.ObjectId(); + await Agent.findByIdAndUpdate(agentA1._id, { + skills_enabled: true, + skills: [hiddenSkillId.toString()], + }); + + mockReq.user.id = userB.toString(); + mockReq.query.requiredPermission = String(PermissionBits.VIEW); + findAccessibleResources.mockImplementation(({ resourceType }) => { + if (resourceType === ResourceType.AGENT) { + return Promise.resolve([agentA1._id]); + } + if (resourceType === ResourceType.SKILL) { + return Promise.resolve([]); + } + return Promise.resolve([]); + }); + findPubliclyAccessibleResources.mockResolvedValue([]); + + await getListAgentsHandler(mockReq, mockRes); + + const response = mockRes.json.mock.calls[0][0]; + expect(response.data).toHaveLength(1); + expect(response.data[0].skills).toBeUndefined(); + expect(response.data[0].skills_enabled).toBeUndefined(); + }); + + test('should return raw skill configuration for EDIT list callers', async () => { + const visibleSkillId = new mongoose.Types.ObjectId(); + const hiddenSkillId = new mongoose.Types.ObjectId(); + await Agent.findByIdAndUpdate(agentA1._id, { + skills_enabled: true, + skills: [visibleSkillId.toString(), hiddenSkillId.toString()], + }); + + mockReq.user.id = userB.toString(); + mockReq.query.requiredPermission = String(PermissionBits.EDIT); + findAccessibleResources.mockResolvedValue([agentA1._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + + await getListAgentsHandler(mockReq, mockRes); + + const response = mockRes.json.mock.calls[0][0]; + expect(response.data).toHaveLength(1); + expect(response.data[0].skills_enabled).toBe(true); + expect(response.data[0].skills).toEqual([ + visibleSkillId.toString(), + hiddenSkillId.toString(), + ]); + expect(findAccessibleResources).not.toHaveBeenCalledWith( + expect.objectContaining({ resourceType: ResourceType.SKILL }), + ); + }); + test('should handle promoted filter with ACL', async () => { // Create a promoted agent const promotedAgent = await Agent.create({ diff --git a/packages/data-schemas/src/methods/agent.spec.ts b/packages/data-schemas/src/methods/agent.spec.ts index 2d5a6666bb..4efb76d3ac 100644 --- a/packages/data-schemas/src/methods/agent.spec.ts +++ b/packages/data-schemas/src/methods/agent.spec.ts @@ -3334,10 +3334,7 @@ describe('Support Contact Field', () => { expect(result.data[0].name).toBe('Agent A1'); }); - test('should include the skills field in the list projection', async () => { - // Frontend popover scoping relies on `agent.skills` being present in - // list results so it can narrow the `$` catalog without refetching the - // full agent document. Locks in that projection contract. + test('should omit skill configuration from the default list projection', async () => { const targetSkillIds = [ new mongoose.Types.ObjectId().toString(), new mongoose.Types.ObjectId().toString(), @@ -3350,6 +3347,7 @@ describe('Support Contact Field', () => { model: 'gpt-4', author: userA, skills: targetSkillIds, + skills_enabled: true, }); const result = await getListAgentsByAccess({ @@ -3357,8 +3355,36 @@ describe('Support Contact Field', () => { otherParams: {}, }); + expect(result.data).toHaveLength(1); + expect(result.data[0].skills).toBeUndefined(); + expect(result.data[0].skills_enabled).toBeUndefined(); + }); + + test('should include skill configuration only when explicitly requested', async () => { + const targetSkillIds = [ + new mongoose.Types.ObjectId().toString(), + new mongoose.Types.ObjectId().toString(), + ]; + const scopedAgent = await createAgent({ + id: `agent_${uuidv4().slice(0, 12)}`, + name: 'Scoped Agent', + description: 'Agent with configured skill scope', + provider: 'openai', + model: 'gpt-4', + author: userA, + skills: targetSkillIds, + skills_enabled: true, + }); + + const result = await getListAgentsByAccess({ + accessibleIds: [scopedAgent._id] as mongoose.Types.ObjectId[], + otherParams: {}, + includeSkillConfig: true, + }); + expect(result.data).toHaveLength(1); expect(result.data[0].skills).toEqual(targetSkillIds); + expect(result.data[0].skills_enabled).toBe(true); }); test('should return multiple accessible agents when provided', async () => { diff --git a/packages/data-schemas/src/methods/agent.ts b/packages/data-schemas/src/methods/agent.ts index 833f0b9f63..9078616c93 100644 --- a/packages/data-schemas/src/methods/agent.ts +++ b/packages/data-schemas/src/methods/agent.ts @@ -648,11 +648,13 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag otherParams = {}, limit = null, after = null, + includeSkillConfig = false, }: { accessibleIds?: Types.ObjectId[]; otherParams?: Record; limit?: number | null; after?: string | null; + includeSkillConfig?: boolean; }): Promise<{ object: string; data: Array>; @@ -700,7 +702,7 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag } } - let query = Agent.find(baseQuery, { + const projection: Record = { id: 1, _id: 1, name: 1, @@ -711,13 +713,14 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag category: 1, support_contact: 1, is_promoted: 1, - /* Needed so the client can scope the `$` skill popover to each agent's - configured catalog without refetching the full agent document. The - master toggle is required alongside the allowlist so the popover can - distinguish "enabled with full catalog" from "disabled". */ - skills: 1, - skills_enabled: 1, - }).sort({ updatedAt: -1, _id: 1 }); + }; + + if (includeSkillConfig) { + projection.skills = 1; + projection.skills_enabled = 1; + } + + let query = Agent.find(baseQuery, projection).sort({ updatedAt: -1, _id: 1 }); if (isPaginated && normalizedLimit) { query = query.limit(normalizedLimit + 1);