🧱 fix: Enforce Agent Access Control on Context and OCR File Loading (#12253)

* 🔏 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.
This commit is contained in:
Danny Avila 2026-03-15 23:02:36 -04:00 committed by GitHub
parent 6f87b49df8
commit 8e8fb01d18
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 708 additions and 25 deletions

View file

@ -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);
});
});
});