mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-16 12:46:34 +01:00
🔏 fix: Scope Agent-Author File Access to Attached Files Only (#12251)
* 🛡️ fix: Scope agent-author file access to attached files only
The hasAccessToFilesViaAgent helper short-circuited for agent authors,
granting access to all requested file IDs without verifying they were
attached to the agent's tool_resources. This enabled an IDOR where any
agent author could delete arbitrary files by supplying their agent_id
alongside unrelated file IDs.
Now both the author and non-author paths check file IDs against the
agent's tool_resources before granting access.
* chore: Use Object.values/for...of and add JSDoc in getAttachedFileIds
* test: Add boundary cases for agent file access authorization
- Agent with no tool_resources denies all access (fail-closed)
- Files across multiple resource types are all reachable
- Author + isDelete: true still scopes to attached files only
This commit is contained in:
Danny Avila
GitHub
parent
f7ab5e645a
commit
ad08df4db6
2 changed files with 141 additions and 25 deletions
|
|
@ -152,12 +152,11 @@ describe('File Access Control', () => {
|
|||
expect(accessMap.get(fileIds[3])).toBe(false);
|
||||
});
|
||||
|
||||
it('should grant access to all files when user is the agent author', async () => {
|
||||
it('should only grant author access to files attached to the agent', async () => {
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
const agentId = uuidv4();
|
||||
const fileIds = [uuidv4(), uuidv4(), uuidv4()];
|
||||
|
||||
// Create author user
|
||||
await User.create({
|
||||
_id: authorId,
|
||||
email: 'author@example.com',
|
||||
|
|
@ -165,7 +164,6 @@ describe('File Access Control', () => {
|
|||
provider: 'local',
|
||||
});
|
||||
|
||||
// Create agent
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Test Agent',
|
||||
|
|
@ -174,12 +172,83 @@ describe('File Access Control', () => {
|
|||
provider: 'openai',
|
||||
tool_resources: {
|
||||
file_search: {
|
||||
file_ids: [fileIds[0]], // Only one file attached
|
||||
file_ids: [fileIds[0]],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
|
||||
const accessMap = await hasAccessToFilesViaAgent({
|
||||
userId: authorId,
|
||||
role: SystemRoles.USER,
|
||||
fileIds,
|
||||
agentId,
|
||||
});
|
||||
|
||||
expect(accessMap.get(fileIds[0])).toBe(true);
|
||||
expect(accessMap.get(fileIds[1])).toBe(false);
|
||||
expect(accessMap.get(fileIds[2])).toBe(false);
|
||||
});
|
||||
|
||||
it('should deny all access when agent has no tool_resources', async () => {
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
const agentId = uuidv4();
|
||||
const fileId = uuidv4();
|
||||
|
||||
await User.create({
|
||||
_id: authorId,
|
||||
email: 'author-no-resources@example.com',
|
||||
emailVerified: true,
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Bare Agent',
|
||||
author: authorId,
|
||||
model: 'gpt-4',
|
||||
provider: 'openai',
|
||||
});
|
||||
|
||||
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
|
||||
const accessMap = await hasAccessToFilesViaAgent({
|
||||
userId: authorId,
|
||||
role: SystemRoles.USER,
|
||||
fileIds: [fileId],
|
||||
agentId,
|
||||
});
|
||||
|
||||
expect(accessMap.get(fileId)).toBe(false);
|
||||
});
|
||||
|
||||
it('should grant access to files across multiple resource types', async () => {
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
const agentId = uuidv4();
|
||||
const fileIds = [uuidv4(), uuidv4(), uuidv4()];
|
||||
|
||||
await User.create({
|
||||
_id: authorId,
|
||||
email: 'author-multi@example.com',
|
||||
emailVerified: true,
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Multi Resource Agent',
|
||||
author: authorId,
|
||||
model: 'gpt-4',
|
||||
provider: 'openai',
|
||||
tool_resources: {
|
||||
file_search: {
|
||||
file_ids: [fileIds[0]],
|
||||
},
|
||||
execute_code: {
|
||||
file_ids: [fileIds[1]],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Check access as the author
|
||||
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
|
||||
const accessMap = await hasAccessToFilesViaAgent({
|
||||
userId: authorId,
|
||||
|
|
@ -188,10 +257,48 @@ describe('File Access Control', () => {
|
|||
agentId,
|
||||
});
|
||||
|
||||
// Author should have access to all files
|
||||
expect(accessMap.get(fileIds[0])).toBe(true);
|
||||
expect(accessMap.get(fileIds[1])).toBe(true);
|
||||
expect(accessMap.get(fileIds[2])).toBe(true);
|
||||
expect(accessMap.get(fileIds[2])).toBe(false);
|
||||
});
|
||||
|
||||
it('should grant author access to attached files when isDelete is true', async () => {
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
const agentId = uuidv4();
|
||||
const attachedFileId = uuidv4();
|
||||
const unattachedFileId = uuidv4();
|
||||
|
||||
await User.create({
|
||||
_id: authorId,
|
||||
email: 'author-delete@example.com',
|
||||
emailVerified: true,
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Delete Test Agent',
|
||||
author: authorId,
|
||||
model: 'gpt-4',
|
||||
provider: 'openai',
|
||||
tool_resources: {
|
||||
file_search: {
|
||||
file_ids: [attachedFileId],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
|
||||
const accessMap = await hasAccessToFilesViaAgent({
|
||||
userId: authorId,
|
||||
role: SystemRoles.USER,
|
||||
fileIds: [attachedFileId, unattachedFileId],
|
||||
agentId,
|
||||
isDelete: true,
|
||||
});
|
||||
|
||||
expect(accessMap.get(attachedFileId)).toBe(true);
|
||||
expect(accessMap.get(unattachedFileId)).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle non-existent agent gracefully', async () => {
|
||||
|
|
|
|||
|
|
@ -4,7 +4,26 @@ const { checkPermission } = require('~/server/services/PermissionService');
|
|||
const { getAgent } = require('~/models/Agent');
|
||||
|
||||
/**
|
||||
* Checks if a user has access to multiple files through a shared agent (batch operation)
|
||||
* @param {Object} agent - The agent document (lean)
|
||||
* @returns {Set<string>} All file IDs attached across all resource types
|
||||
*/
|
||||
function getAttachedFileIds(agent) {
|
||||
const attachedFileIds = new Set();
|
||||
if (agent.tool_resources) {
|
||||
for (const resource of Object.values(agent.tool_resources)) {
|
||||
if (resource?.file_ids && Array.isArray(resource.file_ids)) {
|
||||
for (const fileId of resource.file_ids) {
|
||||
attachedFileIds.add(fileId);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return attachedFileIds;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a user has access to multiple files through a shared agent (batch operation).
|
||||
* Access is always scoped to files actually attached to the agent's tool_resources.
|
||||
* @param {Object} params - Parameters object
|
||||
* @param {string} params.userId - The user ID to check access for
|
||||
* @param {string} [params.role] - Optional user role to avoid DB query
|
||||
|
|
@ -16,7 +35,6 @@ const { getAgent } = require('~/models/Agent');
|
|||
const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDelete }) => {
|
||||
const accessMap = new Map();
|
||||
|
||||
// Initialize all files as no access
|
||||
fileIds.forEach((fileId) => accessMap.set(fileId, false));
|
||||
|
||||
try {
|
||||
|
|
@ -26,13 +44,17 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele
|
|||
return accessMap;
|
||||
}
|
||||
|
||||
// Check if user is the author - if so, grant access to all files
|
||||
const attachedFileIds = getAttachedFileIds(agent);
|
||||
|
||||
if (agent.author.toString() === userId.toString()) {
|
||||
fileIds.forEach((fileId) => accessMap.set(fileId, true));
|
||||
fileIds.forEach((fileId) => {
|
||||
if (attachedFileIds.has(fileId)) {
|
||||
accessMap.set(fileId, true);
|
||||
}
|
||||
});
|
||||
return accessMap;
|
||||
}
|
||||
|
||||
// Check if user has at least VIEW permission on the agent
|
||||
const hasViewPermission = await checkPermission({
|
||||
userId,
|
||||
role,
|
||||
|
|
@ -46,7 +68,6 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele
|
|||
}
|
||||
|
||||
if (isDelete) {
|
||||
// Check if user has EDIT permission (which would indicate collaborative access)
|
||||
const hasEditPermission = await checkPermission({
|
||||
userId,
|
||||
role,
|
||||
|
|
@ -55,23 +76,11 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele
|
|||
requiredPermission: PermissionBits.EDIT,
|
||||
});
|
||||
|
||||
// If user only has VIEW permission, they can't access files
|
||||
// Only users with EDIT permission or higher can access agent files
|
||||
if (!hasEditPermission) {
|
||||
return accessMap;
|
||||
}
|
||||
}
|
||||
|
||||
const attachedFileIds = new Set();
|
||||
if (agent.tool_resources) {
|
||||
for (const [_resourceType, resource] of Object.entries(agent.tool_resources)) {
|
||||
if (resource?.file_ids && Array.isArray(resource.file_ids)) {
|
||||
resource.file_ids.forEach((fileId) => attachedFileIds.add(fileId));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Grant access only to files that are attached to this agent
|
||||
fileIds.forEach((fileId) => {
|
||||
if (attachedFileIds.has(fileId)) {
|
||||
accessMap.set(fileId, true);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue