From b11978017dc40ca9d8c84d8417279e239e12f496 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 2 Aug 2026 14:18:28 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B1=20fix:=20Enforce=20Agent=20Runtime?= =?UTF-8?q?=20File=20Trust=20Boundaries=20(#14577)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: secure agent runtime file metadata * chore: sort agent resource test imports * fix: Align Agent Tool Resource Types * fix: Rehydrate Agent Image Resources * fix: preserve remote agent file authorization --- api/app/clients/tools/util/fileSearch.js | 4 +- .../agents/__tests__/openai.spec.js | 42 ++ .../agents/__tests__/responses.unit.spec.js | 42 ++ api/server/controllers/agents/openai.js | 13 +- api/server/controllers/agents/responses.js | 14 +- api/server/controllers/agents/v1.spec.js | 55 ++ api/server/services/Files/Code/process.js | 16 +- .../services/Files/Code/process.spec.js | 131 +++- api/server/services/Files/permissions.js | 59 +- api/server/services/Files/permissions.spec.js | 55 +- api/server/services/ToolService.js | 11 + .../services/__tests__/ToolService.spec.js | 58 ++ .../app/clients/tools/util/fileSearch.test.js | 27 +- packages/api/src/agents/resources.test.ts | 178 ++++- packages/api/src/agents/resources.ts | 67 +- packages/api/src/agents/validation.spec.ts | 49 ++ packages/api/src/agents/validation.ts | 689 +----------------- 17 files changed, 770 insertions(+), 740 deletions(-) diff --git a/api/app/clients/tools/util/fileSearch.js b/api/app/clients/tools/util/fileSearch.js index ceb87eee69..a153a1aec3 100644 --- a/api/app/clients/tools/util/fileSearch.js +++ b/api/app/clients/tools/util/fileSearch.js @@ -24,13 +24,14 @@ const fileSearchJsonSchema = { * @param {ServerRequest} options.req * @param {Agent['tool_resources']} options.tool_resources * @param {string} [options.agentId] - The agent ID for file access control + * @param {string} [options.agentResourceType] - Permission resource type for the authorized agent route * @returns {Promise<{ * files: Array<{ file_id: string; filename: string; fromAgent: boolean }>, * toolContext: string * }>} */ const primeFiles = async (options) => { - const { tool_resources, req, agentId } = options; + const { tool_resources, req, agentId, agentResourceType } = options; const file_ids = tool_resources?.[EToolResources.file_search]?.file_ids ?? []; const agentResourceIds = new Set(file_ids); const resourceFiles = tool_resources?.[EToolResources.file_search]?.files ?? []; @@ -46,6 +47,7 @@ const primeFiles = async (options) => { userId: req.user.id, role: req.user.role, agentId, + resourceType: agentResourceType, }); } else { dbFiles = allFiles; diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index cb413a1dfc..a50038d5dc 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -3,6 +3,8 @@ * Tests that recordCollectedUsage is called correctly for token spending */ +const { ResourceType } = require('librechat-data-provider'); + const mockProcessStream = jest.fn().mockResolvedValue(undefined); const mockSpendTokens = jest.fn().mockResolvedValue({}); const mockSpendStructuredTokens = jest.fn().mockResolvedValue({}); @@ -353,6 +355,46 @@ describe('OpenAIChatCompletionController', () => { }); }); + describe('remote-agent file authorization', () => { + it('threads the remote-agent permission boundary through initialization and tool loading', async () => { + const { initializeAgent, createToolExecuteHandler } = require('@librechat/api'); + const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); + const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); + + await OpenAIChatCompletionController(req, res); + + const [initializeParams, dbMethods] = initializeAgent.mock.calls.at(-1); + const filterParams = { + files: [{ file_id: 'owner-file', user: 'agent-owner' }], + userId: 'user-123', + role: 'USER', + agentId: 'agent-123', + }; + await dbMethods.filterFilesByAgentAccess(filterParams); + expect(filterFilesByAgentAccess).toHaveBeenLastCalledWith({ + ...filterParams, + resourceType: ResourceType.REMOTE_AGENT, + }); + + await initializeParams.loadTools({ + agentId: 'agent-123', + tools: ['file_search'], + provider: 'openAI', + model: 'gpt-4', + tool_resources: { file_search: { file_ids: ['owner-file'] } }, + }); + expect(loadAgentTools).toHaveBeenLastCalledWith( + expect.objectContaining({ agentResourceType: ResourceType.REMOTE_AGENT }), + ); + + const toolExecuteOptions = createToolExecuteHandler.mock.calls.at(-1)[0]; + await toolExecuteOptions.loadTools(['file_search'], 'agent-123'); + expect(loadToolsForExecution).toHaveBeenLastCalledWith( + expect.objectContaining({ agentResourceType: ResourceType.REMOTE_AGENT }), + ); + }); + }); + describe('execution envelope', () => { it('creates the portable run input before agent initialization', async () => { req.user = { diff --git a/api/server/controllers/agents/__tests__/responses.unit.spec.js b/api/server/controllers/agents/__tests__/responses.unit.spec.js index a89d20573b..d54cc0cfb0 100644 --- a/api/server/controllers/agents/__tests__/responses.unit.spec.js +++ b/api/server/controllers/agents/__tests__/responses.unit.spec.js @@ -3,6 +3,8 @@ * Tests that recordCollectedUsage is called correctly for token spending */ +const { ResourceType } = require('librechat-data-provider'); + const mockSpendTokens = jest.fn().mockResolvedValue({}); const mockSpendStructuredTokens = jest.fn().mockResolvedValue({}); const mockRecordCollectedUsage = jest @@ -491,6 +493,46 @@ describe('createResponse controller', () => { }); }); + describe('remote-agent file authorization', () => { + it('threads the remote-agent permission boundary through initialization and tool loading', async () => { + const { initializeAgent, createToolExecuteHandler } = require('@librechat/api'); + const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); + const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); + + await createResponse(req, res); + + const [initializeParams, dbMethods] = initializeAgent.mock.calls.at(-1); + const filterParams = { + files: [{ file_id: 'owner-file', user: 'agent-owner' }], + userId: 'user-123', + role: 'USER', + agentId: 'agent-123', + }; + await dbMethods.filterFilesByAgentAccess(filterParams); + expect(filterFilesByAgentAccess).toHaveBeenLastCalledWith({ + ...filterParams, + resourceType: ResourceType.REMOTE_AGENT, + }); + + await initializeParams.loadTools({ + agentId: 'agent-123', + tools: ['file_search'], + provider: 'anthropic', + model: 'claude-3', + tool_resources: { file_search: { file_ids: ['owner-file'] } }, + }); + expect(loadAgentTools).toHaveBeenLastCalledWith( + expect.objectContaining({ agentResourceType: ResourceType.REMOTE_AGENT }), + ); + + const toolExecuteOptions = createToolExecuteHandler.mock.calls.at(-1)[0]; + await toolExecuteOptions.loadTools(['file_search'], 'agent-123'); + expect(loadToolsForExecution).toHaveBeenLastCalledWith( + expect.objectContaining({ agentResourceType: ResourceType.REMOTE_AGENT }), + ); + }); + }); + describe('token usage recording - non-streaming', () => { it('should call recordCollectedUsage after successful non-streaming completion', async () => { await createResponse(req, res); diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 9a0f3a7051..fa2cfec0cd 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -63,9 +63,13 @@ const { enrichLoadedToolsWithAgentContext, } = require('~/server/services/Endpoints/agents/skillDeps'); const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { logViolation } = require('~/cache'); const db = require('~/models'); +const filterFilesByRemoteAgentAccess = (params) => + filterFilesByAgentAccess({ ...params, resourceType: ResourceType.REMOTE_AGENT }); + /** * Creates a tool loader function for the agent. * @param {AbortSignal} signal - The abort signal @@ -92,6 +96,7 @@ function createToolLoader(signal, definitionsOnly = true) { agent, signal, tool_resources, + agentResourceType: ResourceType.REMOTE_AGENT, definitionsOnly, accessibleMcpServerNames, streamId: null, // No resumable stream for OpenAI compat @@ -246,15 +251,10 @@ const executeOpenAIChatCompletion = async (envelope, { req, res }) => { }; const skillDbMethods = getSkillDbMethods(); - // `filterFilesByAgentAccess` is intentionally omitted: it calls - // `checkPermission` with `resourceType: AGENT`, but this route - // authorizes callers through `REMOTE_AGENT` (via - // `getRemoteAgentPermissions`), so including it would silently drop - // owner-attached context files for any remote user who has - // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. const dbMethods = { getConvoFiles: db.getConvoFiles, getFiles: db.getFiles, + filterFilesByAgentAccess: filterFilesByRemoteAgentAccess, getUserKey: db.getUserKey, getMessages: db.getMessages, getAccessibleMcpServerNames, @@ -501,6 +501,7 @@ const executeOpenAIChatCompletion = async (envelope, { req, res }) => { const result = await loadToolsForExecution({ req, res, + agentResourceType: ResourceType.REMOTE_AGENT, toolNames, agent: ctx.agent ?? agent, signal: abortController.signal, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index d03df17237..cd29798dee 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -71,11 +71,15 @@ const { enrichLoadedToolsWithAgentContext, } = require('~/server/services/Endpoints/agents/skillDeps'); const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { resolveConfigServers, getAccessibleMcpServerNames } = require('~/server/services/MCP'); const { getMCPManager } = require('~/config'); const { logViolation } = require('~/cache'); const db = require('~/models'); +const filterFilesByRemoteAgentAccess = (params) => + filterFilesByAgentAccess({ ...params, resourceType: ResourceType.REMOTE_AGENT }); + /** * Creates a tool loader function for the agent. * @param {AbortSignal} signal - The abort signal @@ -102,6 +106,7 @@ function createToolLoader(signal, definitionsOnly = true) { agent, signal, tool_resources, + agentResourceType: ResourceType.REMOTE_AGENT, definitionsOnly, accessibleMcpServerNames, streamId: null, @@ -368,15 +373,10 @@ const executeResponse = async (envelope, { req, res }) => { model_parameters: agent.model_parameters ?? {}, }; - // `filterFilesByAgentAccess` is intentionally omitted: it calls - // `checkPermission` with `resourceType: AGENT`, but this route - // authorizes callers through `REMOTE_AGENT` (via - // `getRemoteAgentPermissions`), so including it would silently drop - // owner-attached context files for any remote user who has - // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. const dbMethods = { getConvoFiles: db.getConvoFiles, getFiles: db.getFiles, + filterFilesByAgentAccess: filterFilesByRemoteAgentAccess, getUserKey: db.getUserKey, getMessages: db.getMessages, getAccessibleMcpServerNames, @@ -721,6 +721,7 @@ const executeResponse = async (envelope, { req, res }) => { const result = await loadToolsForExecution({ req, res, + agentResourceType: ResourceType.REMOTE_AGENT, toolNames, agent: ctx.agent ?? agent, signal: abortController.signal, @@ -904,6 +905,7 @@ const executeResponse = async (envelope, { req, res }) => { const result = await loadToolsForExecution({ req, res, + agentResourceType: ResourceType.REMOTE_AGENT, toolNames, agent: ctx.agent ?? agent, signal: abortController.signal, diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index bcababc9fe..02f58d9929 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -321,6 +321,35 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(agentInDb.tool_resources.invalid_resource).toBeUndefined(); }); + test('should strip runtime file records before persisting an agent', async () => { + mockReq.body = { + provider: 'openai', + model: 'gpt-4', + name: 'Agent with forged runtime file', + tool_resources: { + execute_code: { + files: [ + { + file_id: 'forged-file', + filepath: '/etc/passwd', + source: FileSources.local, + }, + ], + }, + }, + }; + + await createAgentHandler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(201); + const createdAgent = mockRes.json.mock.calls[0][0]; + expect(createdAgent.tool_resources?.execute_code?.files).toBeUndefined(); + + const agentInDb = await Agent.findOne({ id: createdAgent.id }).lean(); + expect(agentInDb.tool_resources?.execute_code?.files).toBeUndefined(); + expect(agentInDb.versions[0].tool_resources?.execute_code?.files).toBeUndefined(); + }); + test('should strip file_ids not owned by the creator from tool_resources', async () => { const File = mongoose.models.File; @@ -940,6 +969,32 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(updatedAgent.tool_resources.invalid_tool).toBeUndefined(); }); + test('should strip runtime file records before persisting an update', async () => { + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + tool_resources: { + execute_code: { + files: [ + { + file_id: 'forged-file', + filepath: '/etc/passwd', + source: FileSources.local, + }, + ], + }, + }, + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + const agentInDb = await Agent.findOne({ id: existingAgentId }).lean(); + expect(agentInDb.tool_resources.execute_code.files).toBeUndefined(); + const latestVersion = agentInDb.versions[agentInDb.versions.length - 1]; + expect(latestVersion.tool_resources.execute_code.files).toBeUndefined(); + }); + test('should remove empty strings from model_parameters during update (Issue Fix)', async () => { // First create an agent with valid model_parameters await Agent.updateOne( diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index 10f4819b29..67c2bf9fec 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -833,16 +833,24 @@ const appendVisibleCodeFileContext = (toolContext, contextLine) => { * @param {ServerRequest} options.req * @param {Agent['tool_resources']} options.tool_resources * @param {string} [options.agentId] - The agent ID for file access control + * @param {string} [options.agentResourceType] - Permission resource type for the authorized agent route * @returns {Promise<{ * files: Array<{ id: string; session_id: string; name: string }>, * toolContext: string, * }>} */ const primeFiles = async (options) => { - const { tool_resources, req, agentId } = options; + const { tool_resources, req, agentId, agentResourceType } = options; const file_ids = tool_resources?.[EToolResources.execute_code]?.file_ids ?? []; const agentResourceIds = new Set(file_ids); const resourceFiles = tool_resources?.[EToolResources.execute_code]?.files ?? []; + /** Runtime entries identify candidates only; database records remain authoritative for storage metadata. */ + const candidateFileIds = new Set(file_ids); + for (const file of resourceFiles) { + if (typeof file?.file_id === 'string') { + candidateFileIds.add(file.file_id); + } + } /* Step 1 of the priming trace: input volume. Pair with the * per-file `[primeCodeFiles] file=...` lines and the final @@ -854,7 +862,8 @@ const primeFiles = async (options) => { ); // Get all files first - const allFiles = (await getFiles({ file_id: { $in: file_ids } }, null, { text: 0 })) ?? []; + const allFiles = + (await getFiles({ file_id: { $in: Array.from(candidateFileIds) } }, null, { text: 0 })) ?? []; // Filter by access if user and agent are provided let dbFiles; @@ -864,13 +873,12 @@ const primeFiles = async (options) => { userId: req.user.id, role: req.user.role, agentId, + resourceType: agentResourceType, }); } else { dbFiles = allFiles; } - dbFiles = dbFiles.concat(resourceFiles); - const files = []; const sessions = new Map(); let toolContext = ''; diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index d1ff877d35..220b3d2b56 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -29,7 +29,7 @@ jest.mock('librechat-data-provider', () => { }; }); -const { FileContext } = require('librechat-data-provider'); +const { FileContext, ResourceType } = require('librechat-data-provider'); // Mock uuid jest.mock('uuid', () => ({ @@ -1899,6 +1899,135 @@ describe('Code Process', () => { return { handleFileUpload, getDownloadStream }; } + it('uses the permission resource type established by the calling route', async () => { + const files = [ + { + file_id: 'owner-file', + filename: 'owner.txt', + user: 'agent-owner', + metadata: {}, + }, + ]; + getFiles.mockResolvedValue(files); + filterFilesByAgentAccess.mockImplementation(({ files: authorizedFiles }) => + Promise.resolve(authorizedFiles), + ); + + await primeFiles({ + req: { user: { id: 'remote-viewer', role: 'USER' } }, + agentId: 'agent-123', + agentResourceType: ResourceType.REMOTE_AGENT, + tool_resources: { execute_code: { file_ids: ['owner-file'] } }, + }); + + expect(filterFilesByAgentAccess).toHaveBeenCalledWith({ + files, + userId: 'remote-viewer', + role: 'USER', + agentId: 'agent-123', + resourceType: ResourceType.REMOTE_AGENT, + }); + }); + + it('does not read a runtime file record that has no authorized database record', async () => { + const getDownloadStream = jest.fn().mockResolvedValue('forged-stream'); + const handleFileUpload = jest.fn(); + getStrategyFunctions.mockImplementation((source) => { + if (source === 'execute_code') return { handleFileUpload }; + return { getDownloadStream }; + }); + getFiles.mockResolvedValue([]); + mockAxios.mockResolvedValue({ data: null }); + + const result = await primeFiles({ + req: { user: { id: 'user-123', role: 'USER' } }, + tool_resources: { + execute_code: { + files: [ + { + file_id: 'forged-file', + filename: 'secrets.txt', + filepath: '/etc/passwd', + source: 'local', + metadata: { + codeEnvRef: { + kind: 'user', + id: 'user-123', + storage_session_id: 'missing-session', + file_id: 'missing-file', + }, + }, + }, + ], + }, + }, + agentId: 'agent-id', + }); + + expect(result.files).toEqual([]); + expect(getDownloadStream).not.toHaveBeenCalled(); + expect(handleFileUpload).not.toHaveBeenCalled(); + }); + + it('rehydrates a runtime file by ID before using its storage metadata', async () => { + const trustedFile = { + file_id: 'runtime-file', + filename: 'trusted.txt', + filepath: '/uploads/trusted.txt', + source: 'local', + context: 'execute_code', + metadata: { + codeEnvRef: { + kind: 'user', + id: 'user-123', + storage_session_id: 'trusted-session', + file_id: 'trusted-file', + }, + }, + }; + const getDownloadStream = jest.fn().mockResolvedValue('trusted-stream'); + const handleFileUpload = jest + .fn() + .mockResolvedValue({ storage_session_id: 'new-session', file_id: 'new-file' }); + getStrategyFunctions.mockImplementation((source) => { + if (source === 'execute_code') return { handleFileUpload }; + return { getDownloadStream }; + }); + getFiles.mockResolvedValue([trustedFile]); + mockAxios.mockResolvedValue({ data: null }); + + await primeFiles({ + req: { user: { id: 'user-123', role: 'USER' } }, + tool_resources: { + execute_code: { + files: [ + { + ...trustedFile, + filepath: '/etc/passwd', + source: 'forged-source', + metadata: { + codeEnvRef: { + kind: 'user', + id: 'attacker', + storage_session_id: 'forged-session', + file_id: 'forged-file', + }, + }, + }, + ], + }, + }, + agentId: 'agent-id', + }); + + expect(getDownloadStream).toHaveBeenCalledTimes(1); + expect(getDownloadStream).toHaveBeenCalledWith( + { user: { id: 'user-123', role: 'USER' } }, + '/uploads/trusted.txt', + ); + expect(handleFileUpload).toHaveBeenCalledTimes(1); + }); + it('seed receives FRESH (storage_session_id, file_id) from the reupload response', async () => { const dbFile = { file_id: 'librechat-file-id', diff --git a/api/server/services/Files/permissions.js b/api/server/services/Files/permissions.js index fc3efc5e42..bd6936efba 100644 --- a/api/server/services/Files/permissions.js +++ b/api/server/services/Files/permissions.js @@ -1,8 +1,34 @@ const { logger } = require('@librechat/data-schemas'); -const { PermissionBits, ResourceType, isEphemeralAgentId } = require('librechat-data-provider'); -const { checkPermission } = require('~/server/services/PermissionService'); +const { + PermissionBits, + ResourceType, + hasPermissions, + isEphemeralAgentId, +} = require('librechat-data-provider'); +const { getRemoteAgentPermissions } = require('@librechat/api'); +const { checkPermission, getEffectivePermissions } = require('~/server/services/PermissionService'); const { getAgent, getFiles } = require('~/models'); +const checkAgentPermission = async ({ + userId, + role, + resourceType, + resourceId, + requiredPermission, +}) => { + if (resourceType !== ResourceType.REMOTE_AGENT) { + return checkPermission({ userId, role, resourceType, resourceId, requiredPermission }); + } + + const permissions = await getRemoteAgentPermissions( + { getEffectivePermissions }, + userId, + role, + resourceId, + ); + return hasPermissions(permissions, requiredPermission); +}; + /** * @param {Object} agent - The agent document (lean) * @returns {Set} All file IDs attached across all resource types @@ -39,11 +65,20 @@ function getFilesById(files) { * @param {string} [params.role] - Optional user role to avoid DB query * @param {string[]} params.fileIds - Array of file IDs to check * @param {string} params.agentId - The agent ID that might grant access + * @param {string} [params.resourceType=ResourceType.AGENT] - Agent permission resource type * @param {boolean} [params.isDelete] - Whether the operation is a delete operation * @param {Array<{ file_id: string, user: string }>} [params.files] - Pre-fetched file documents * @returns {Promise>} Map of fileId to access status */ -const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDelete, files }) => { +const hasAccessToFilesViaAgent = async ({ + userId, + role, + fileIds, + agentId, + resourceType = ResourceType.AGENT, + isDelete, + files, +}) => { const accessMap = new Map(); fileIds.forEach((fileId) => accessMap.set(fileId, false)); @@ -75,10 +110,10 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele return accessMap; } - const hasViewPermission = await checkPermission({ + const hasViewPermission = await checkAgentPermission({ userId, role, - resourceType: ResourceType.AGENT, + resourceType, resourceId: agent._id, requiredPermission: PermissionBits.VIEW, }); @@ -88,10 +123,10 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele } if (isDelete) { - const hasEditPermission = await checkPermission({ + const hasEditPermission = await checkAgentPermission({ userId, role, - resourceType: ResourceType.AGENT, + resourceType, resourceId: agent._id, requiredPermission: PermissionBits.EDIT, }); @@ -121,9 +156,16 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele * @param {string} params.userId - User ID for access control * @param {string} [params.role] - Optional user role to avoid DB query * @param {string} params.agentId - Agent ID that might grant access to files + * @param {string} [params.resourceType=ResourceType.AGENT] - Agent permission resource type * @returns {Promise>} Filtered array of accessible files */ -const filterFilesByAgentAccess = async ({ files, userId, role, agentId }) => { +const filterFilesByAgentAccess = async ({ + files, + userId, + role, + agentId, + resourceType = ResourceType.AGENT, +}) => { if (!userId || !agentId || !files || files.length === 0 || isEphemeralAgentId(agentId)) { return files; } @@ -151,6 +193,7 @@ const filterFilesByAgentAccess = async ({ files, userId, role, agentId }) => { role, fileIds, agentId, + resourceType, files: filesToCheck, }); diff --git a/api/server/services/Files/permissions.spec.js b/api/server/services/Files/permissions.spec.js index ae799a97a6..94c5e5c571 100644 --- a/api/server/services/Files/permissions.spec.js +++ b/api/server/services/Files/permissions.spec.js @@ -4,6 +4,7 @@ jest.mock('@librechat/data-schemas', () => ({ jest.mock('~/server/services/PermissionService', () => ({ checkPermission: jest.fn(), + getEffectivePermissions: jest.fn(), })); jest.mock('~/models', () => ({ @@ -13,7 +14,7 @@ jest.mock('~/models', () => ({ const { logger } = require('@librechat/data-schemas'); const { Constants, PermissionBits, ResourceType } = require('librechat-data-provider'); -const { checkPermission } = require('~/server/services/PermissionService'); +const { checkPermission, getEffectivePermissions } = require('~/server/services/PermissionService'); const { getAgent, getFiles } = require('~/models'); const { filterFilesByAgentAccess, hasAccessToFilesViaAgent } = require('./permissions'); @@ -41,6 +42,7 @@ function makeAgent(overrides = {}) { beforeEach(() => { jest.clearAllMocks(); + getEffectivePermissions.mockResolvedValue(0); getFiles.mockResolvedValue([ makeFile('attached-1', AUTHOR_ID), makeFile('attached-2', AUTHOR_ID), @@ -186,6 +188,57 @@ describe('filterFilesByAgentAccess', () => { expect(result).toEqual([ownedFile]); }); + it('should grant explicit REMOTE_AGENT VIEW without direct AGENT VIEW', async () => { + getAgent.mockResolvedValue(makeAgent()); + getEffectivePermissions.mockResolvedValueOnce(0).mockResolvedValueOnce(PermissionBits.VIEW); + + const result = await filterFilesByAgentAccess({ + files: [sharedFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + resourceType: ResourceType.REMOTE_AGENT, + }); + + expect(result).toEqual([sharedFile]); + expect(getEffectivePermissions).toHaveBeenNthCalledWith(1, { + userId: USER_ID, + role: 'USER', + resourceType: ResourceType.AGENT, + resourceId: AGENT_MONGO_ID, + }); + expect(getEffectivePermissions).toHaveBeenNthCalledWith(2, { + userId: USER_ID, + role: 'USER', + resourceType: ResourceType.REMOTE_AGENT, + resourceId: AGENT_MONGO_ID, + }); + expect(checkPermission).not.toHaveBeenCalled(); + }); + + it('should grant AGENT SHARE without an explicit REMOTE_AGENT ACL', async () => { + getAgent.mockResolvedValue(makeAgent()); + getEffectivePermissions.mockResolvedValueOnce(PermissionBits.SHARE); + + const result = await filterFilesByAgentAccess({ + files: [sharedFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + resourceType: ResourceType.REMOTE_AGENT, + }); + + expect(result).toEqual([sharedFile]); + expect(getEffectivePermissions).toHaveBeenCalledTimes(1); + expect(getEffectivePermissions).toHaveBeenCalledWith({ + userId: USER_ID, + role: 'USER', + resourceType: ResourceType.AGENT, + resourceId: AGENT_MONGO_ID, + }); + expect(checkPermission).not.toHaveBeenCalled(); + }); + it('should return only owned files when agent is not found', async () => { getAgent.mockResolvedValue(null); diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index b4a81c7627..46b003c631 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -540,6 +540,7 @@ const isBuiltInTool = (toolName) => * @param {ServerRequest} params.req - The request object * @param {ServerResponse} [params.res] - The response object for SSE events * @param {Object} params.agent - The agent configuration + * @param {string} [params.agentResourceType] - Permission resource type for the authorized agent route * @param {string|null} [params.streamId] - Stream ID for resumable mode * @param {number} [params.jobCreatedAt] - The generation epoch that owns emitted tool events * @returns {Promise<{ @@ -554,6 +555,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, + agentResourceType, streamId = null, jobCreatedAt, tool_resources, @@ -1092,6 +1094,7 @@ async function loadToolDefinitionsWrapper({ req, tool_resources, agentId: agent.id, + agentResourceType, }); if (toolContext) { dynamicToolContextMap[Tools.execute_code] = toolContext; @@ -1110,6 +1113,7 @@ async function loadToolDefinitionsWrapper({ req, tool_resources, agentId: agent.id, + agentResourceType, }); if (toolContext) { dynamicToolContextMap[Tools.file_search] = toolContext; @@ -1167,6 +1171,7 @@ async function loadToolDefinitionsWrapper({ * @param {ServerRequest} params.req - The request object * @param {ServerResponse} params.res - The response object * @param {Object} params.agent - The agent configuration + * @param {string} [params.agentResourceType] - Permission resource type for the authorized agent route * @param {AbortSignal} [params.signal] - Abort signal * @param {Object} [params.tool_resources] - Tool resources * @param {string} [params.openAIApiKey] - OpenAI API key @@ -1180,6 +1185,7 @@ async function loadAgentTools({ req, res, agent, + agentResourceType, signal, tool_resources, openAIApiKey, @@ -1193,6 +1199,7 @@ async function loadAgentTools({ req, res, agent, + agentResourceType, streamId, jobCreatedAt, tool_resources, @@ -1298,6 +1305,7 @@ async function loadAgentTools({ options: { req, res, + agentResourceType, mcpServerContext, jobCreatedAt, openAIApiKey, @@ -1556,6 +1564,7 @@ async function loadAgentTools({ * @param {ServerResponse} params.res - The response object * @param {AbortSignal} [params.signal] - Abort signal * @param {Object} params.agent - The agent object + * @param {string} [params.agentResourceType] - Permission resource type for the authorized agent route * @param {string[]} params.toolNames - Names of tools to load * @param {Map} [params.toolRegistry] - Tool registry * @param {Record} [params.mcpAvailableTools] - Run-scoped MCP tool definitions @@ -1573,6 +1582,7 @@ async function loadToolsForExecution({ res, signal, agent, + agentResourceType, toolNames, toolRegistry, backgroundToolNames, @@ -1759,6 +1769,7 @@ async function loadToolsForExecution({ options: { req, res, + agentResourceType, jobCreatedAt, tool_resources, processFileURL, diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 511025802b..d859680587 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -2,6 +2,7 @@ const { Constants: AgentConstants } = require('@librechat/agents'); const { Tools, Constants, + ResourceType, EModelEndpoint, isActionTool, actionDelimiter, @@ -231,6 +232,40 @@ describe('ToolService - Action Capability Gating', () => { const actionToolName = `get_weather${actionDelimiter}api_example_com`; const regularTool = 'calculator'; + it('should preserve the remote-agent permission boundary while priming files', async () => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.file_search, + AgentCapabilities.execute_code, + ]; + const req = createMockReq(capabilities); + const tool_resources = { + file_search: { file_ids: ['search-file'] }, + execute_code: { file_ids: ['code-file'] }, + }; + const { primeFiles: primeSearchFiles } = require('~/app/clients/tools/util/fileSearch'); + const { primeFiles: primeCodeFiles } = require('~/server/services/Files/Code/process'); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_123', tools: [Tools.file_search, Tools.execute_code] }, + tool_resources, + agentResourceType: ResourceType.REMOTE_AGENT, + definitionsOnly: true, + }); + + const expectedParams = { + req, + tool_resources, + agentId: 'agent_123', + agentResourceType: ResourceType.REMOTE_AGENT, + }; + expect(primeSearchFiles).toHaveBeenCalledWith(expectedParams); + expect(primeCodeFiles).toHaveBeenCalledWith(expectedParams); + }); + it('should exclude action tools from definitions when actions capability is disabled', async () => { const capabilities = [AgentCapabilities.tools, AgentCapabilities.web_search]; const req = createMockReq(capabilities); @@ -1096,6 +1131,29 @@ describe('ToolService - Action Capability Gating', () => { }); describe('loadToolsForExecution — action tool gating', () => { + it('should preserve the remote-agent permission boundary for deferred tool loading', async () => { + const capabilities = [AgentCapabilities.tools, AgentCapabilities.file_search]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_123', tools: [Tools.file_search] }, + toolNames: [Tools.file_search], + agentResourceType: ResourceType.REMOTE_AGENT, + actionsEnabled: false, + }); + + expect(mockLoadToolsUtil).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.objectContaining({ + agentResourceType: ResourceType.REMOTE_AGENT, + }), + }), + ); + }); + const actionToolName = `get_weather${actionDelimiter}api_example_com`; const regularTool = Tools.web_search; diff --git a/api/test/app/clients/tools/util/fileSearch.test.js b/api/test/app/clients/tools/util/fileSearch.test.js index fc4000b3ab..0c1ff89515 100644 --- a/api/test/app/clients/tools/util/fileSearch.test.js +++ b/api/test/app/clients/tools/util/fileSearch.test.js @@ -1,4 +1,5 @@ const axios = require('axios'); +const { ResourceType } = require('librechat-data-provider'); jest.mock('axios'); jest.mock('@librechat/api', () => ({ @@ -22,9 +23,33 @@ jest.mock('~/server/services/Files/permissions', () => ({ filterFilesByAgentAccess: jest.fn((options) => Promise.resolve(options.files)), })); -const { createFileSearchTool } = require('~/app/clients/tools/util/fileSearch'); +const { createFileSearchTool, primeFiles } = require('~/app/clients/tools/util/fileSearch'); const { generateShortLivedToken } = require('@librechat/api'); +describe('fileSearch.js - agent file authorization', () => { + it('uses the permission resource type established by the calling route', async () => { + const { getFiles } = require('~/models'); + const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); + const files = [{ file_id: 'owner-file', filename: 'owner.pdf', user: 'agent-owner' }]; + getFiles.mockResolvedValueOnce(files); + + await primeFiles({ + req: { user: { id: 'remote-viewer', role: 'USER' } }, + agentId: 'agent-123', + agentResourceType: ResourceType.REMOTE_AGENT, + tool_resources: { file_search: { file_ids: ['owner-file'] } }, + }); + + expect(filterFilesByAgentAccess).toHaveBeenCalledWith({ + files, + userId: 'remote-viewer', + role: 'USER', + agentId: 'agent-123', + resourceType: ResourceType.REMOTE_AGENT, + }); + }); +}); + describe('fileSearch.js - tuple return validation', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/api/src/agents/resources.test.ts b/packages/api/src/agents/resources.test.ts index 04849a5177..4a200f935f 100644 --- a/packages/api/src/agents/resources.test.ts +++ b/packages/api/src/agents/resources.test.ts @@ -1,10 +1,15 @@ -import { primeResources } from './resources'; import { logger } from '@librechat/data-schemas'; -import { EModelEndpoint, EToolResources, AgentCapabilities } from 'librechat-data-provider'; +import { + EModelEndpoint, + EToolResources, + AgentCapabilities, + FileSources, +} from 'librechat-data-provider'; import type { TAgentsEndpoint, TFile } from 'librechat-data-provider'; import type { IUser, AppConfig } from '@librechat/data-schemas'; import type { Request as ServerRequest } from 'express'; import type { TGetFiles, TFilterFilesByAgentAccess } from './resources'; +import { primeResources } from './resources'; // Mock logger jest.mock('@librechat/data-schemas', () => ({ @@ -115,6 +120,138 @@ describe('primeResources', () => { }); }); + describe('when persisted image-edit file IDs are provided', () => { + it('should rehydrate only accessible image records for tool initialization', async () => { + const accessibleImage: TFile = { + user: 'user1', + file_id: 'accessible-image', + filename: 'accessible.png', + filepath: '/uploads/accessible.png', + object: 'file', + type: 'image/png', + bytes: 2048, + embedded: false, + usage: 0, + height: 800, + width: 600, + }; + const inaccessibleImage: TFile = { + ...accessibleImage, + user: 'other-user', + file_id: 'inaccessible-image', + filename: 'inaccessible.png', + filepath: '/uploads/inaccessible.png', + }; + + mockGetFiles.mockResolvedValue([accessibleImage, inaccessibleImage]); + mockFilterFiles.mockResolvedValue([accessibleImage]); + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources: { + [EToolResources.image_edit]: { + file_ids: ['accessible-image', 'inaccessible-image'], + }, + }, + agentId: 'agent_shared', + }); + + expect(mockGetFiles).toHaveBeenCalledWith( + { file_id: { $in: ['accessible-image', 'inaccessible-image'] } }, + {}, + {}, + ); + expect(mockFilterFiles).toHaveBeenCalledWith({ + files: [accessibleImage, inaccessibleImage], + userId: 'user1', + role: 'USER', + agentId: 'agent_shared', + }); + expect(result.tool_resources?.[EToolResources.image_edit]).toEqual({ + file_ids: ['accessible-image', 'inaccessible-image'], + files: [accessibleImage], + }); + }); + + it('should fetch and filter context and image records in one batch', async () => { + const contextFile: TFile = { + user: 'agent-owner', + file_id: 'context-file', + filename: 'context.pdf', + filepath: '/uploads/context.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: true, + usage: 0, + }; + const sharedImage: TFile = { + user: 'agent-owner', + file_id: 'shared-image', + filename: 'shared.png', + filepath: '/uploads/shared.png', + object: 'file', + type: 'image/png', + bytes: 2048, + embedded: false, + usage: 0, + height: 800, + width: 600, + }; + const imageFile: TFile = { + ...sharedImage, + file_id: 'image-file', + filename: 'image.png', + filepath: '/uploads/image.png', + }; + + mockGetFiles.mockResolvedValue([contextFile, sharedImage, imageFile]); + mockFilterFiles.mockResolvedValue([contextFile, sharedImage, imageFile]); + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources: { + [EToolResources.context]: { + file_ids: ['context-file', 'shared-image'], + }, + [EToolResources.image_edit]: { + file_ids: ['shared-image', 'image-file'], + }, + }, + agentId: 'agent_shared', + }); + + expect(mockGetFiles).toHaveBeenCalledTimes(1); + expect(mockGetFiles).toHaveBeenCalledWith( + { file_id: { $in: ['context-file', 'shared-image', 'image-file'] } }, + {}, + {}, + ); + expect(mockFilterFiles).toHaveBeenCalledTimes(1); + expect(mockFilterFiles).toHaveBeenCalledWith({ + files: [contextFile, sharedImage, imageFile], + userId: 'user1', + role: 'USER', + agentId: 'agent_shared', + }); + expect(result.attachments).toEqual([contextFile, sharedImage]); + expect(result.tool_resources?.[EToolResources.image_edit]?.files).toEqual([ + sharedImage, + imageFile, + ]); + }); + }); + describe('when attachments are provided', () => { it('should process files with fileIdentifier as execute_code resources', async () => { const mockFiles: TFile[] = [ @@ -317,7 +454,7 @@ describe('primeResources', () => { expect(result.attachments?.[1]?.file_id).toBe('file2'); }); - it('should merge existing tool_resources with new files', async () => { + it('should discard persisted files and add trusted attachment records at runtime', async () => { const mockFiles: TFile[] = [ { user: 'user1', @@ -337,17 +474,19 @@ describe('primeResources', () => { const existingToolResources = { [EToolResources.execute_code]: { + file_ids: ['persisted-id'], files: [ { - user: 'user1', - file_id: 'existing-file', - filename: 'existing.py', - filepath: '/uploads/existing.py', + user: 'attacker', + file_id: 'forged-file', + filename: 'forged.py', + filepath: '/etc/passwd', object: 'file' as const, type: 'text/x-python', bytes: 256, embedded: false, usage: 0, + source: FileSources.local, }, ], }, @@ -364,13 +503,10 @@ describe('primeResources', () => { tool_resources: existingToolResources, }); - expect(result.tool_resources?.[EToolResources.execute_code]?.files).toHaveLength(2); - expect(result.tool_resources?.[EToolResources.execute_code]?.files?.[0]?.file_id).toBe( - 'existing-file', - ); - expect(result.tool_resources?.[EToolResources.execute_code]?.files?.[1]?.file_id).toBe( - 'file1', - ); + expect(result.tool_resources?.[EToolResources.execute_code]).toEqual({ + file_ids: ['persisted-id'], + files: mockFiles, + }); }); }); @@ -709,7 +845,7 @@ describe('primeResources', () => { expect(result.attachments?.some((f) => !f?.file_id)).toBe(true); }); - it('should prevent duplicates from existing tool_resources', async () => { + it('should rebuild runtime files from trusted attachments instead of persisted files', async () => { const existingFile: TFile = { user: 'user1', file_id: 'existing-file', @@ -757,11 +893,8 @@ describe('primeResources', () => { tool_resources: existingToolResources, }); - // Should only add the new file to attachments - expect(result.attachments).toHaveLength(1); - expect(result.attachments?.[0]?.file_id).toBe('new-file'); + expect(result.attachments).toEqual([existingFile, newFile]); - // Should not duplicate the existing file in tool_resources expect(result.tool_resources?.[EToolResources.execute_code]?.files).toHaveLength(2); const fileIds = result.tool_resources?.[EToolResources.execute_code]?.files?.map( (f) => f.file_id, @@ -821,7 +954,7 @@ describe('primeResources', () => { expect(fileIds?.filter((id) => id === 'dup-file')).toHaveLength(1); }); - it('should prevent duplicates across different tool_resource categories', async () => { + it('should not let persisted files suppress trusted attachments', async () => { const multiPurposeFile: TFile = { user: 'user1', file_id: 'multi-file', @@ -840,7 +973,6 @@ describe('primeResources', () => { }, }; - // Try to add the same file again const attachments = Promise.resolve([multiPurposeFile]); const result = await primeResources({ @@ -852,10 +984,8 @@ describe('primeResources', () => { tool_resources: existingToolResources, }); - // Should not add to attachments (already exists) - expect(result.attachments).toHaveLength(0); + expect(result.attachments).toEqual([multiPurposeFile]); - // Should not duplicate in file_search expect(result.tool_resources?.[EToolResources.file_search]?.files).toHaveLength(1); expect(result.tool_resources?.[EToolResources.file_search]?.files?.[0]?.file_id).toBe( 'multi-file', diff --git a/packages/api/src/agents/resources.ts b/packages/api/src/agents/resources.ts index 47239e54bb..a7c6e9546b 100644 --- a/packages/api/src/agents/resources.ts +++ b/packages/api/src/agents/resources.ts @@ -5,6 +5,25 @@ import type { IMongoFile, AppConfig, IUser } from '@librechat/data-schemas'; import type { FilterQuery, QueryOptions, ProjectionType } from 'mongoose'; import type { Request as ServerRequest } from 'express'; +import { TOOL_RESOURCE_KEYS } from './orphans'; + +/** Removes runtime-only file records before persisted Agent resources enter tool initialization. */ +const sanitizePersistedToolResources = ( + tool_resources: AgentToolResources | undefined, +): AgentToolResources => { + const sanitized: AgentToolResources = {}; + for (const key of TOOL_RESOURCE_KEYS) { + const resource = tool_resources?.[key]; + if (!resource) { + continue; + } + const persistedResource = { ...resource }; + delete persistedResource.files; + sanitized[key] = persistedResource; + } + return sanitized; +}; + /** * Function type for retrieving files from the database * @param filter - MongoDB filter query for files @@ -180,6 +199,7 @@ export const primeResources = async ({ }> => { const requestAttachments: Array = []; const agentContextAttachments: Array = []; + const persistedToolResources = sanitizePersistedToolResources(_tool_resources); try { /** * Array to collect all unique files that will be returned as attachments @@ -202,7 +222,7 @@ export const primeResources = async ({ * The agent's tool resources object that will be updated with categorized files * Create a shallow copy first to avoid mutating the original */ - const tool_resources: AgentToolResources = { ...(_tool_resources ?? {}) }; + const tool_resources: AgentToolResources = { ...persistedToolResources }; // Deep copy each resource to avoid mutating nested objects/arrays for (const [resourceType, resource] of Object.entries(tool_resources)) { @@ -244,30 +264,45 @@ export const primeResources = async ({ delete tool_resources[EToolResources.ocr]; } - if (fileIds.length > 0 && isContextEnabled) { + const shouldLoadContext = fileIds.length > 0 && isContextEnabled; + const contextFileIds = new Set(shouldLoadContext ? fileIds : []); + const imageEditFileIds = tool_resources[EToolResources.image_edit]?.file_ids ?? []; + const imageEditFileIdSet = new Set(imageEditFileIds); + const persistedResourceFileIds = new Set(contextFileIds); + for (const fileId of imageEditFileIds) { + persistedResourceFileIds.add(fileId); + } + + if (shouldLoadContext) { delete tool_resources[EToolResources.context]; - let context = await getFiles( + } + + let persistedResourceFiles: Array = []; + if (persistedResourceFileIds.size > 0) { + persistedResourceFiles = await getFiles( { - file_id: { $in: fileIds }, + file_id: { $in: Array.from(persistedResourceFileIds) }, }, {}, {}, ); if (filterFiles && req.user?.id && agentId) { - context = await filterFiles({ - files: context, + persistedResourceFiles = await filterFiles({ + files: persistedResourceFiles, userId: req.user.id, role: req.user.role, agentId, }); } + } - for (const file of context) { - if (!file?.file_id) { - continue; - } + for (const file of persistedResourceFiles) { + if (!file?.file_id) { + continue; + } + if (contextFileIds.has(file.file_id)) { // Clear from attachmentFileIds if it was pre-added attachmentFileIds.delete(file.file_id); @@ -284,6 +319,16 @@ export const primeResources = async ({ processedResourceFiles, }); } + + if (imageEditFileIdSet.has(file.file_id)) { + addFileToResource({ + file, + resourceType: EToolResources.image_edit, + tool_resources, + processedResourceFiles, + }); + attachmentFileIds.add(file.file_id); + } } if (!_attachments) { @@ -357,7 +402,7 @@ export const primeResources = async ({ requestAttachments: safeAttachments, agentContextAttachments: agentContextAttachments.length > 0 ? agentContextAttachments : undefined, - tool_resources: _tool_resources, + tool_resources: persistedToolResources, }; } }; diff --git a/packages/api/src/agents/validation.spec.ts b/packages/api/src/agents/validation.spec.ts index 743217780f..fe9f17bfb8 100644 --- a/packages/api/src/agents/validation.spec.ts +++ b/packages/api/src/agents/validation.spec.ts @@ -63,6 +63,40 @@ describe('agentCreateSchema with subagents', () => { }); expect(result.success).toBe(false); }); + + it('strips runtime-populated files from every tool resource', () => { + const forgedFile = { + file_id: 'forged', + filepath: '/etc/passwd', + source: 'local', + metadata: { + codeEnvRef: { + kind: 'user', + id: 'attacker', + storage_session_id: 'missing', + file_id: 'missing', + }, + }, + }; + const result = agentCreateSchema.parse({ + ...base, + tool_resources: { + execute_code: { file_ids: ['execute'], files: [forgedFile] }, + file_search: { file_ids: ['search'], files: [forgedFile] }, + image_edit: { file_ids: ['image'], files: [forgedFile] }, + context: { file_ids: ['context'], files: [forgedFile] }, + ocr: { file_ids: ['ocr'], files: [forgedFile] }, + }, + }); + + expect(result.tool_resources).toEqual({ + execute_code: { file_ids: ['execute'] }, + file_search: { file_ids: ['search'] }, + image_edit: { file_ids: ['image'] }, + context: { file_ids: ['context'] }, + ocr: { file_ids: ['ocr'] }, + }); + }); }); describe('agentUpdateSchema with subagents', () => { @@ -80,4 +114,19 @@ describe('agentUpdateSchema with subagents', () => { }); expect(result.success).toBe(false); }); + + it('strips runtime-populated files from partial updates', () => { + const result = agentUpdateSchema.parse({ + tool_resources: { + execute_code: { + file_ids: ['kept'], + files: [{ file_id: 'forged', filepath: '/etc/passwd', source: 'local' }], + }, + }, + }); + + expect(result.tool_resources).toEqual({ + execute_code: { file_ids: ['kept'] }, + }); + }); }); diff --git a/packages/api/src/agents/validation.ts b/packages/api/src/agents/validation.ts index ee9eaa964d..0b4c6cd2f6 100644 --- a/packages/api/src/agents/validation.ts +++ b/packages/api/src/agents/validation.ts @@ -22,24 +22,20 @@ export const agentAvatarSchema: z.ZodObject< source: z.string(), }); -/** Base resource schema for tool resources */ +/** Persisted resource schema. Full file records are populated only after database authorization. */ export const agentBaseResourceSchema: z.ZodObject< { file_ids: z.ZodOptional>; - files: z.ZodOptional>; }, 'strip' > = z.object({ file_ids: z.array(z.string()).optional(), - files: z.array(z.unknown()).optional(), // Files are populated at runtime, not from user input }); -/** File resource schema extends base with vector_store_ids */ +/** File resource schema extends base with vector_store_ids. */ export const agentFileResourceSchema: z.ZodObject< { file_ids: z.ZodOptional>; - files: z.ZodOptional>; - } & { vector_store_ids: z.ZodOptional>; }, 'strip' @@ -47,174 +43,17 @@ export const agentFileResourceSchema: z.ZodObject< vector_store_ids: z.array(z.string()).optional(), }); -/** Tool resources schema matching AgentToolResources interface */ +/** Persisted tool resources accepted by Agent create and update APIs. */ export const agentToolResourcesSchema: z.ZodOptional< z.ZodObject< { - image_edit: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - execute_code: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - file_search: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - } & { - vector_store_ids: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - > - >; - context: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - /** @deprecated Use context instead */ - ocr: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; + image_edit: z.ZodOptional; + execute_code: z.ZodOptional; + file_search: z.ZodOptional; + context: z.ZodOptional; + ocr: z.ZodOptional; }, - 'strip', - z.ZodTypeAny, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - }, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - } + 'strip' > > = z .object({ @@ -457,175 +296,7 @@ export const agentBaseSchema: z.ZodObject< artifacts: z.ZodOptional; recursion_limit: z.ZodOptional; conversation_starters: z.ZodOptional>; - tool_resources: z.ZodOptional< - z.ZodObject< - { - image_edit: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - execute_code: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - file_search: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - } & { - vector_store_ids: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - > - >; - context: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - /** @deprecated Use context instead */ - ocr: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - }, - 'strip', - z.ZodTypeAny, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - }, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - } - > - >; + tool_resources: typeof agentToolResourcesSchema; tool_options: z.ZodOptional< z.ZodRecord< z.ZodString, @@ -810,175 +481,7 @@ export const agentCreateSchema: z.ZodObject< artifacts: z.ZodOptional; recursion_limit: z.ZodOptional; conversation_starters: z.ZodOptional>; - tool_resources: z.ZodOptional< - z.ZodObject< - { - image_edit: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - execute_code: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - file_search: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - } & { - vector_store_ids: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - > - >; - context: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - /** @deprecated Use context instead */ - ocr: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - }, - 'strip', - z.ZodTypeAny, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - }, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - } - > - >; + tool_resources: typeof agentToolResourcesSchema; tool_options: z.ZodOptional< z.ZodRecord< z.ZodString, @@ -1128,175 +631,7 @@ export const agentUpdateSchema: z.ZodObject< artifacts: z.ZodOptional; recursion_limit: z.ZodOptional; conversation_starters: z.ZodOptional>; - tool_resources: z.ZodOptional< - z.ZodObject< - { - image_edit: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - execute_code: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - file_search: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - } & { - vector_store_ids: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - > - >; - context: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - /** @deprecated Use context instead */ - ocr: z.ZodOptional< - z.ZodObject< - { - file_ids: z.ZodOptional>; - files: z.ZodOptional>; - }, - 'strip', - z.ZodTypeAny, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - }, - { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - > - >; - }, - 'strip', - z.ZodTypeAny, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - }, - { - ocr?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - context?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - execute_code?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - file_search?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - vector_store_ids?: string[] | undefined; - } - | undefined; - image_edit?: - | { - file_ids?: string[] | undefined; - files?: unknown[] | undefined; - } - | undefined; - } - > - >; + tool_resources: typeof agentToolResourcesSchema; tool_options: z.ZodOptional< z.ZodRecord< z.ZodString,