From 8e8fb01d18b7471607b4e3bd4a894ae135d3cfaa Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 15 Mar 2026 23:02:36 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B1=20fix:=20Enforce=20Agent=20Access?= =?UTF-8?q?=20Control=20on=20Context=20and=20OCR=20File=20Loading=20(#1225?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔏 fix: Apply agent access control filtering to context/OCR resource loading The context/OCR file path in primeResources fetched files by file_id without applying filterFilesByAgentAccess, unlike the file_search and execute_code paths. Add filterFiles dependency injection to primeResources and invoke it after getFiles to enforce consistent access control. * fix: Wire filterFilesByAgentAccess into all agent initialization callers Pass the filterFilesByAgentAccess function from the JS layer into the TS initializeAgent → primeResources chain via dependency injection, covering primary, handoff, added-convo, and memory agent init paths. * test: Add access control filtering tests for primeResources Cover filterFiles invocation with context/OCR files, verify filtering rejects inaccessible files, and confirm graceful fallback when filterFiles, userId, or agentId are absent. * fix: Guard filterFilesByAgentAccess against ephemeral agent IDs Ephemeral agents have no DB document, so getAgent returns null and the access map defaults to all-false, silently blocking all non-owned files. Short-circuit with isEphemeralAgentId to preserve the pass-through behavior for inline-built agents (memory, tool agents). * fix: Clean up resources.ts and JS caller import order Remove redundant optional chain on req.user.role inside user-guarded block, update primeResources JSDoc with filterFiles and agentId params, and reorder JS imports to longest-to-shortest per project conventions. * test: Strengthen OCR assertion and add filterFiles error-path test Use toHaveBeenCalledWith for the OCR filtering test to verify exact arguments after the OCR→context merge step. Add test for filterFiles rejection to verify graceful degradation (logs error, returns original tool_resources). * fix: Correct import order in addedConvo.js and initialize.js Sort by total line length descending: loadAddedAgent (91) before filterFilesByAgentAccess (84), loadAgentTools (91) before filterFilesByAgentAccess (84). * test: Add unit tests for filterFilesByAgentAccess and hasAccessToFilesViaAgent Cover every branch in permissions.js: ephemeral agent guard, missing userId/agentId/files early returns, all-owned short-circuit, mixed owned + non-owned with VIEW/no-VIEW, agent-not-found fail-closed, author path scoped to attached files, EDIT gate on delete, DB error fail-closed, and agent with no tool_resources. * test: Cover file.user undefined/null in permissions spec Files with no user field fall into the non-owned path and get run through hasAccessToFilesViaAgent. Add two cases: attached file with no user field is returned, unattached file with no user field is excluded. --- api/server/controllers/agents/client.js | 2 + .../services/Endpoints/agents/addedConvo.js | 2 + .../services/Endpoints/agents/initialize.js | 3 + api/server/services/Files/permissions.js | 4 +- api/server/services/Files/permissions.spec.js | 409 ++++++++++++++++++ packages/api/src/agents/initialize.ts | 6 +- packages/api/src/agents/resources.test.ts | 275 +++++++++++- packages/api/src/agents/resources.ts | 32 +- 8 files changed, 708 insertions(+), 25 deletions(-) create mode 100644 api/server/services/Files/permissions.spec.js diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 0ecd62b819..c454bd65cf 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -44,6 +44,7 @@ const { isEphemeralAgentId, removeNullishValues, } = require('librechat-data-provider'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { spendTokens, spendStructuredTokens } = require('~/models/spendTokens'); const { encodeAndFormat } = require('~/server/services/Files/images/encode'); const { updateBalance, bulkInsertTransactions } = require('~/models'); @@ -479,6 +480,7 @@ class AgentClient extends BaseClient { getUserKeyValues: db.getUserKeyValues, getToolFilesByIds: db.getToolFilesByIds, getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, }, ); diff --git a/api/server/services/Endpoints/agents/addedConvo.js b/api/server/services/Endpoints/agents/addedConvo.js index 25b1327991..11b87e450e 100644 --- a/api/server/services/Endpoints/agents/addedConvo.js +++ b/api/server/services/Endpoints/agents/addedConvo.js @@ -1,6 +1,7 @@ const { logger } = require('@librechat/data-schemas'); const { initializeAgent, validateAgentModel } = require('@librechat/api'); const { loadAddedAgent, setGetAgent, ADDED_AGENT_ID } = require('~/models/loadAddedAgent'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { getConvoFiles } = require('~/models/Conversation'); const { getAgent } = require('~/models/Agent'); const db = require('~/models'); @@ -108,6 +109,7 @@ const processAddedConvo = async ({ getUserKeyValues: db.getUserKeyValues, getToolFilesByIds: db.getToolFilesByIds, getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, }, ); diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 762236ea19..08f631c3d2 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -22,6 +22,7 @@ const { getDefaultHandlers, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { getModelsConfig } = require('~/server/controllers/ModelController'); const { checkPermission } = require('~/server/services/PermissionService'); const AgentClient = require('~/server/controllers/agents/client'); @@ -204,6 +205,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, }, ); @@ -284,6 +286,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getUserCodeFiles: db.getUserCodeFiles, getToolFilesByIds: db.getToolFilesByIds, getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, }, ); diff --git a/api/server/services/Files/permissions.js b/api/server/services/Files/permissions.js index df484f7c29..b9a5d6656f 100644 --- a/api/server/services/Files/permissions.js +++ b/api/server/services/Files/permissions.js @@ -1,5 +1,5 @@ const { logger } = require('@librechat/data-schemas'); -const { PermissionBits, ResourceType } = require('librechat-data-provider'); +const { PermissionBits, ResourceType, isEphemeralAgentId } = require('librechat-data-provider'); const { checkPermission } = require('~/server/services/PermissionService'); const { getAgent } = require('~/models/Agent'); @@ -104,7 +104,7 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele * @returns {Promise>} Filtered array of accessible files */ const filterFilesByAgentAccess = async ({ files, userId, role, agentId }) => { - if (!userId || !agentId || !files || files.length === 0) { + if (!userId || !agentId || !files || files.length === 0 || isEphemeralAgentId(agentId)) { return files; } diff --git a/api/server/services/Files/permissions.spec.js b/api/server/services/Files/permissions.spec.js new file mode 100644 index 0000000000..85e7b2dc5b --- /dev/null +++ b/api/server/services/Files/permissions.spec.js @@ -0,0 +1,409 @@ +jest.mock('@librechat/data-schemas', () => ({ + logger: { error: jest.fn() }, +})); + +jest.mock('~/server/services/PermissionService', () => ({ + checkPermission: jest.fn(), +})); + +jest.mock('~/models/Agent', () => ({ + getAgent: jest.fn(), +})); + +const { logger } = require('@librechat/data-schemas'); +const { Constants, PermissionBits, ResourceType } = require('librechat-data-provider'); +const { checkPermission } = require('~/server/services/PermissionService'); +const { getAgent } = require('~/models/Agent'); +const { filterFilesByAgentAccess, hasAccessToFilesViaAgent } = require('./permissions'); + +const AUTHOR_ID = 'author-user-id'; +const USER_ID = 'viewer-user-id'; +const AGENT_ID = 'agent_test-abc123'; +const AGENT_MONGO_ID = 'mongo-agent-id'; + +function makeFile(file_id, user) { + return { file_id, user, filename: `${file_id}.txt` }; +} + +function makeAgent(overrides = {}) { + return { + _id: AGENT_MONGO_ID, + id: AGENT_ID, + author: AUTHOR_ID, + tool_resources: { + file_search: { file_ids: ['attached-1', 'attached-2'] }, + execute_code: { file_ids: ['attached-3'] }, + }, + ...overrides, + }; +} + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('filterFilesByAgentAccess', () => { + describe('early returns (no DB calls)', () => { + it('should return files unfiltered for ephemeral agentId', async () => { + const files = [makeFile('f1', 'other-user')]; + const result = await filterFilesByAgentAccess({ + files, + userId: USER_ID, + agentId: Constants.EPHEMERAL_AGENT_ID, + }); + + expect(result).toBe(files); + expect(getAgent).not.toHaveBeenCalled(); + }); + + it('should return files unfiltered for non-agent_ prefixed agentId', async () => { + const files = [makeFile('f1', 'other-user')]; + const result = await filterFilesByAgentAccess({ + files, + userId: USER_ID, + agentId: 'custom-memory-id', + }); + + expect(result).toBe(files); + expect(getAgent).not.toHaveBeenCalled(); + }); + + it('should return files when userId is missing', async () => { + const files = [makeFile('f1', 'someone')]; + const result = await filterFilesByAgentAccess({ + files, + userId: undefined, + agentId: AGENT_ID, + }); + + expect(result).toBe(files); + expect(getAgent).not.toHaveBeenCalled(); + }); + + it('should return files when agentId is missing', async () => { + const files = [makeFile('f1', 'someone')]; + const result = await filterFilesByAgentAccess({ + files, + userId: USER_ID, + agentId: undefined, + }); + + expect(result).toBe(files); + expect(getAgent).not.toHaveBeenCalled(); + }); + + it('should return empty array when files is empty', async () => { + const result = await filterFilesByAgentAccess({ + files: [], + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual([]); + expect(getAgent).not.toHaveBeenCalled(); + }); + + it('should return undefined when files is nullish', async () => { + const result = await filterFilesByAgentAccess({ + files: null, + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toBeNull(); + expect(getAgent).not.toHaveBeenCalled(); + }); + }); + + describe('all files owned by userId', () => { + it('should return all files without calling getAgent', async () => { + const files = [makeFile('f1', USER_ID), makeFile('f2', USER_ID)]; + const result = await filterFilesByAgentAccess({ + files, + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual(files); + expect(getAgent).not.toHaveBeenCalled(); + }); + }); + + describe('mixed owned and non-owned files', () => { + const ownedFile = makeFile('owned-1', USER_ID); + const sharedFile = makeFile('attached-1', AUTHOR_ID); + const unattachedFile = makeFile('not-attached', AUTHOR_ID); + + it('should return owned + accessible non-owned files when user has VIEW', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(true); + + const result = await filterFilesByAgentAccess({ + files: [ownedFile, sharedFile, unattachedFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + }); + + expect(result).toHaveLength(2); + expect(result.map((f) => f.file_id)).toContain('owned-1'); + expect(result.map((f) => f.file_id)).toContain('attached-1'); + expect(result.map((f) => f.file_id)).not.toContain('not-attached'); + }); + + it('should return only owned files when user lacks VIEW permission', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(false); + + const result = await filterFilesByAgentAccess({ + files: [ownedFile, sharedFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + }); + + expect(result).toEqual([ownedFile]); + }); + + it('should return only owned files when agent is not found', async () => { + getAgent.mockResolvedValue(null); + + const result = await filterFilesByAgentAccess({ + files: [ownedFile, sharedFile], + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual([ownedFile]); + }); + + it('should return only owned files on DB error (fail-closed)', async () => { + getAgent.mockRejectedValue(new Error('DB connection lost')); + + const result = await filterFilesByAgentAccess({ + files: [ownedFile, sharedFile], + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual([ownedFile]); + expect(logger.error).toHaveBeenCalled(); + }); + }); + + describe('file with no user field', () => { + it('should treat file as non-owned and run through access check', async () => { + const noUserFile = makeFile('attached-1', undefined); + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(true); + + const result = await filterFilesByAgentAccess({ + files: [noUserFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + }); + + expect(getAgent).toHaveBeenCalled(); + expect(result).toEqual([noUserFile]); + }); + + it('should exclude file with no user field when not attached to agent', async () => { + const noUserFile = makeFile('not-attached', null); + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(true); + + const result = await filterFilesByAgentAccess({ + files: [noUserFile], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + }); + + expect(result).toEqual([]); + }); + }); + + describe('no owned files (all non-owned)', () => { + const file1 = makeFile('attached-1', AUTHOR_ID); + const file2 = makeFile('not-attached', AUTHOR_ID); + + it('should return only attached files when user has VIEW', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(true); + + const result = await filterFilesByAgentAccess({ + files: [file1, file2], + userId: USER_ID, + role: 'USER', + agentId: AGENT_ID, + }); + + expect(result).toEqual([file1]); + }); + + it('should return empty array when no VIEW permission', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(false); + + const result = await filterFilesByAgentAccess({ + files: [file1, file2], + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual([]); + }); + + it('should return empty array when agent not found', async () => { + getAgent.mockResolvedValue(null); + + const result = await filterFilesByAgentAccess({ + files: [file1], + userId: USER_ID, + agentId: AGENT_ID, + }); + + expect(result).toEqual([]); + }); + }); +}); + +describe('hasAccessToFilesViaAgent', () => { + describe('agent not found', () => { + it('should return all-false map', async () => { + getAgent.mockResolvedValue(null); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + fileIds: ['f1', 'f2'], + agentId: AGENT_ID, + }); + + expect(result.get('f1')).toBe(false); + expect(result.get('f2')).toBe(false); + }); + }); + + describe('author path', () => { + it('should grant access to attached files for the agent author', async () => { + getAgent.mockResolvedValue(makeAgent()); + + const result = await hasAccessToFilesViaAgent({ + userId: AUTHOR_ID, + fileIds: ['attached-1', 'not-attached'], + agentId: AGENT_ID, + }); + + expect(result.get('attached-1')).toBe(true); + expect(result.get('not-attached')).toBe(false); + expect(checkPermission).not.toHaveBeenCalled(); + }); + }); + + describe('VIEW permission path', () => { + it('should grant access to attached files for viewer with VIEW permission', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(true); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + role: 'USER', + fileIds: ['attached-1', 'attached-3', 'not-attached'], + agentId: AGENT_ID, + }); + + expect(result.get('attached-1')).toBe(true); + expect(result.get('attached-3')).toBe(true); + expect(result.get('not-attached')).toBe(false); + + expect(checkPermission).toHaveBeenCalledWith({ + userId: USER_ID, + role: 'USER', + resourceType: ResourceType.AGENT, + resourceId: AGENT_MONGO_ID, + requiredPermission: PermissionBits.VIEW, + }); + }); + + it('should deny all when VIEW permission is missing', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValue(false); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + fileIds: ['attached-1'], + agentId: AGENT_ID, + }); + + expect(result.get('attached-1')).toBe(false); + }); + }); + + describe('delete path (EDIT permission required)', () => { + it('should grant access when both VIEW and EDIT pass', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValueOnce(true).mockResolvedValueOnce(true); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + fileIds: ['attached-1'], + agentId: AGENT_ID, + isDelete: true, + }); + + expect(result.get('attached-1')).toBe(true); + expect(checkPermission).toHaveBeenCalledTimes(2); + expect(checkPermission).toHaveBeenLastCalledWith( + expect.objectContaining({ requiredPermission: PermissionBits.EDIT }), + ); + }); + + it('should deny all when VIEW passes but EDIT fails', async () => { + getAgent.mockResolvedValue(makeAgent()); + checkPermission.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + fileIds: ['attached-1'], + agentId: AGENT_ID, + isDelete: true, + }); + + expect(result.get('attached-1')).toBe(false); + }); + }); + + describe('error handling', () => { + it('should return all-false map on DB error (fail-closed)', async () => { + getAgent.mockRejectedValue(new Error('connection refused')); + + const result = await hasAccessToFilesViaAgent({ + userId: USER_ID, + fileIds: ['f1', 'f2'], + agentId: AGENT_ID, + }); + + expect(result.get('f1')).toBe(false); + expect(result.get('f2')).toBe(false); + expect(logger.error).toHaveBeenCalledWith( + '[hasAccessToFilesViaAgent] Error checking file access:', + expect.any(Error), + ); + }); + }); + + describe('agent with no tool_resources', () => { + it('should deny all files even for the author', async () => { + getAgent.mockResolvedValue(makeAgent({ tool_resources: undefined })); + + const result = await hasAccessToFilesViaAgent({ + userId: AUTHOR_ID, + fileIds: ['f1'], + agentId: AGENT_ID, + }); + + expect(result.get('f1')).toBe(false); + }); + }); +}); diff --git a/packages/api/src/agents/initialize.ts b/packages/api/src/agents/initialize.ts index 913835a007..d5bfca5aba 100644 --- a/packages/api/src/agents/initialize.ts +++ b/packages/api/src/agents/initialize.ts @@ -31,6 +31,7 @@ import { filterFilesByEndpointConfig } from '~/files'; import { generateArtifactsPrompt } from '~/prompts'; import { getProviderConfig } from '~/endpoints'; import { primeResources } from './resources'; +import type { TFilterFilesByAgentAccess } from './resources'; /** * Extended agent type with additional fields needed after initialization @@ -111,7 +112,9 @@ export interface InitializeAgentDbMethods extends EndpointDbMethods { /** Update usage tracking for multiple files */ updateFilesUsage: (files: Array<{ file_id: string }>, fileIds?: string[]) => Promise; /** Get files from database */ - getFiles: (filter: unknown, sort: unknown, select: unknown, opts?: unknown) => Promise; + getFiles: (filter: unknown, sort: unknown, select: unknown) => Promise; + /** Filter files by agent access permissions (ownership or agent attachment) */ + filterFilesByAgentAccess?: TFilterFilesByAgentAccess; /** Get tool files by IDs (user-uploaded files only, code files handled separately) */ getToolFilesByIds: (fileIds: string[], toolSet: Set) => Promise; /** Get conversation file IDs */ @@ -271,6 +274,7 @@ export async function initializeAgent( const { attachments: primedAttachments, tool_resources } = await primeResources({ req: req as never, getFiles: db.getFiles as never, + filterFiles: db.filterFilesByAgentAccess, appConfig: req.config, agentId: agent.id, attachments: currentFiles diff --git a/packages/api/src/agents/resources.test.ts b/packages/api/src/agents/resources.test.ts index bfd2327764..641fb9284c 100644 --- a/packages/api/src/agents/resources.test.ts +++ b/packages/api/src/agents/resources.test.ts @@ -4,7 +4,7 @@ import { EModelEndpoint, EToolResources, AgentCapabilities } from 'librechat-dat 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 } from './resources'; +import type { TGetFiles, TFilterFilesByAgentAccess } from './resources'; // Mock logger jest.mock('@librechat/data-schemas', () => ({ @@ -17,16 +17,16 @@ describe('primeResources', () => { let mockReq: ServerRequest & { user?: IUser }; let mockAppConfig: AppConfig; let mockGetFiles: jest.MockedFunction; + let mockFilterFiles: jest.MockedFunction; let requestFileSet: Set; beforeEach(() => { - // Reset mocks jest.clearAllMocks(); - // Setup mock request - mockReq = {} as unknown as ServerRequest & { user?: IUser }; + mockReq = { + user: { id: 'user1', role: 'USER' }, + } as unknown as ServerRequest & { user?: IUser }; - // Setup mock appConfig mockAppConfig = { endpoints: { [EModelEndpoint.agents]: { @@ -35,10 +35,9 @@ describe('primeResources', () => { }, } as AppConfig; - // Setup mock getFiles function mockGetFiles = jest.fn(); + mockFilterFiles = jest.fn().mockImplementation(({ files }) => Promise.resolve(files)); - // Setup request file set requestFileSet = new Set(['file1', 'file2', 'file3']); }); @@ -70,20 +69,21 @@ describe('primeResources', () => { req: mockReq, appConfig: mockAppConfig, getFiles: mockGetFiles, + filterFiles: mockFilterFiles, requestFileSet, attachments: undefined, tool_resources, + agentId: 'agent_test', }); - expect(mockGetFiles).toHaveBeenCalledWith( - { file_id: { $in: ['ocr-file-1'] } }, - {}, - {}, - { userId: undefined, agentId: undefined }, - ); + expect(mockGetFiles).toHaveBeenCalledWith({ file_id: { $in: ['ocr-file-1'] } }, {}, {}); + expect(mockFilterFiles).toHaveBeenCalledWith({ + files: mockOcrFiles, + userId: 'user1', + role: 'USER', + agentId: 'agent_test', + }); expect(result.attachments).toEqual(mockOcrFiles); - // Context field is deleted after files are fetched and re-categorized - // Since the file is not embedded and has no special properties, it won't be categorized expect(result.tool_resources).toEqual({}); }); }); @@ -1108,12 +1108,10 @@ describe('primeResources', () => { 'ocr-file-1', ); - // Verify getFiles was called with merged file_ids expect(mockGetFiles).toHaveBeenCalledWith( { file_id: { $in: ['context-file-1', 'ocr-file-1'] } }, {}, {}, - { userId: undefined, agentId: undefined }, ); }); @@ -1241,6 +1239,249 @@ describe('primeResources', () => { }); }); + describe('access control filtering', () => { + it('should filter context files through filterFiles when provided', async () => { + const ownedFile: TFile = { + user: 'user1', + file_id: 'owned-file', + filename: 'owned.pdf', + filepath: '/uploads/owned.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + const inaccessibleFile: TFile = { + user: 'other-user', + file_id: 'inaccessible-file', + filename: 'secret.pdf', + filepath: '/uploads/secret.pdf', + object: 'file', + type: 'application/pdf', + bytes: 2048, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([ownedFile, inaccessibleFile]); + mockFilterFiles.mockResolvedValue([ownedFile]); + + const tool_resources = { + [EToolResources.context]: { + file_ids: ['owned-file', 'inaccessible-file'], + }, + }; + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources, + agentId: 'agent_shared', + }); + + expect(mockFilterFiles).toHaveBeenCalledWith({ + files: [ownedFile, inaccessibleFile], + userId: 'user1', + role: 'USER', + agentId: 'agent_shared', + }); + expect(result.attachments).toEqual([ownedFile]); + expect(result.attachments).not.toContainEqual(inaccessibleFile); + }); + + it('should filter OCR files merged into context through filterFiles', async () => { + const ocrFile: TFile = { + user: 'other-user', + file_id: 'ocr-restricted', + filename: 'scan.pdf', + filepath: '/uploads/scan.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([ocrFile]); + mockFilterFiles.mockResolvedValue([]); + + const tool_resources = { + [EToolResources.ocr]: { + file_ids: ['ocr-restricted'], + }, + }; + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources, + agentId: 'agent_shared', + }); + + expect(mockFilterFiles).toHaveBeenCalledWith({ + files: [ocrFile], + userId: 'user1', + role: 'USER', + agentId: 'agent_shared', + }); + expect(result.attachments).toBeUndefined(); + }); + + it('should skip filtering when filterFiles is not provided', async () => { + const mockFile: TFile = { + user: 'user1', + file_id: 'file-1', + filename: 'doc.pdf', + filepath: '/uploads/doc.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([mockFile]); + + const tool_resources = { + [EToolResources.context]: { + file_ids: ['file-1'], + }, + }; + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + requestFileSet, + attachments: undefined, + tool_resources, + agentId: 'agent_test', + }); + + expect(mockFilterFiles).not.toHaveBeenCalled(); + expect(result.attachments).toEqual([mockFile]); + }); + + it('should skip filtering when user ID is missing', async () => { + const reqNoUser = {} as unknown as ServerRequest & { user?: IUser }; + const mockFile: TFile = { + user: 'user1', + file_id: 'file-1', + filename: 'doc.pdf', + filepath: '/uploads/doc.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([mockFile]); + + const tool_resources = { + [EToolResources.context]: { + file_ids: ['file-1'], + }, + }; + + const result = await primeResources({ + req: reqNoUser, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources, + agentId: 'agent_test', + }); + + expect(mockFilterFiles).not.toHaveBeenCalled(); + expect(result.attachments).toEqual([mockFile]); + }); + + it('should gracefully handle filterFiles rejection', async () => { + const mockFile: TFile = { + user: 'user1', + file_id: 'file-1', + filename: 'doc.pdf', + filepath: '/uploads/doc.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([mockFile]); + mockFilterFiles.mockRejectedValue(new Error('DB failure')); + + const tool_resources = { + [EToolResources.context]: { + file_ids: ['file-1'], + }, + }; + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources, + agentId: 'agent_test', + }); + + expect(logger.error).toHaveBeenCalledWith('Error priming resources', expect.any(Error)); + expect(result.tool_resources).toEqual(tool_resources); + }); + + it('should skip filtering when agentId is missing', async () => { + const mockFile: TFile = { + user: 'user1', + file_id: 'file-1', + filename: 'doc.pdf', + filepath: '/uploads/doc.pdf', + object: 'file', + type: 'application/pdf', + bytes: 1024, + embedded: false, + usage: 0, + }; + + mockGetFiles.mockResolvedValue([mockFile]); + + const tool_resources = { + [EToolResources.context]: { + file_ids: ['file-1'], + }, + }; + + const result = await primeResources({ + req: mockReq, + appConfig: mockAppConfig, + getFiles: mockGetFiles, + filterFiles: mockFilterFiles, + requestFileSet, + attachments: undefined, + tool_resources, + }); + + expect(mockFilterFiles).not.toHaveBeenCalled(); + expect(result.attachments).toEqual([mockFile]); + }); + }); + describe('edge cases', () => { it('should handle missing appConfig agents endpoint gracefully', async () => { const reqWithoutLocals = {} as ServerRequest & { user?: IUser }; diff --git a/packages/api/src/agents/resources.ts b/packages/api/src/agents/resources.ts index 4655453847..e147c743cf 100644 --- a/packages/api/src/agents/resources.ts +++ b/packages/api/src/agents/resources.ts @@ -10,16 +10,26 @@ import type { Request as ServerRequest } from 'express'; * @param filter - MongoDB filter query for files * @param _sortOptions - Sorting options (currently unused) * @param selectFields - Field selection options - * @param options - Additional options including userId and agentId for access control * @returns Promise resolving to array of files */ export type TGetFiles = ( filter: FilterQuery, _sortOptions: ProjectionType | null | undefined, selectFields: QueryOptions | null | undefined, - options?: { userId?: string; agentId?: string }, ) => Promise>; +/** + * Function type for filtering files by agent access permissions. + * Used to enforce that only files the user has access to (via ownership or agent attachment) + * are returned after a raw DB query. + */ +export type TFilterFilesByAgentAccess = (params: { + files: Array; + userId: string; + role?: string; + agentId: string; +}) => Promise>; + /** * Helper function to add a file to a specific tool resource category * Prevents duplicate files within the same resource category @@ -128,7 +138,7 @@ const categorizeFileForToolResources = ({ /** * Primes resources for agent execution by processing attachments and tool resources * This function: - * 1. Fetches OCR files if OCR is enabled + * 1. Fetches context/OCR files (filtered by agent access control when available) * 2. Processes attachment files * 3. Categorizes files into appropriate tool resources * 4. Prevents duplicate files across all sources @@ -137,15 +147,18 @@ const categorizeFileForToolResources = ({ * @param params.req - Express request object * @param params.appConfig - Application configuration object * @param params.getFiles - Function to retrieve files from database + * @param params.filterFiles - Optional function to enforce agent-based file access control * @param params.requestFileSet - Set of file IDs from the current request * @param params.attachments - Promise resolving to array of attachment files * @param params.tool_resources - Existing tool resources for the agent + * @param params.agentId - Agent ID used for access control filtering * @returns Promise resolving to processed attachments and updated tool resources */ export const primeResources = async ({ req, appConfig, getFiles, + filterFiles, requestFileSet, attachments: _attachments, tool_resources: _tool_resources, @@ -157,6 +170,7 @@ export const primeResources = async ({ attachments: Promise> | undefined; tool_resources: AgentToolResources | undefined; getFiles: TGetFiles; + filterFiles?: TFilterFilesByAgentAccess; agentId?: string; }): Promise<{ attachments: Array | undefined; @@ -228,15 +242,23 @@ export const primeResources = async ({ if (fileIds.length > 0 && isContextEnabled) { delete tool_resources[EToolResources.context]; - const context = await getFiles( + let context = await getFiles( { file_id: { $in: fileIds }, }, {}, {}, - { userId: req.user?.id, agentId }, ); + if (filterFiles && req.user?.id && agentId) { + context = await filterFiles({ + files: context, + userId: req.user.id, + role: req.user.role, + agentId, + }); + } + for (const file of context) { if (!file?.file_id) { continue;