🗃️ refactor: File Access via Agent; Deny Deletion if not Editor, Allow Viewer (#9357)

This commit is contained in:
Danny Avila 2025-08-28 21:16:23 -04:00 committed by GitHub
parent 7742b18c9c
commit 8772b04d1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 80 additions and 18 deletions

View file

@ -211,7 +211,67 @@ describe('File Access Control', () => {
expect(accessMap.get(fileIds[1])).toBe(false); expect(accessMap.get(fileIds[1])).toBe(false);
}); });
it('should deny access when user only has VIEW permission', async () => { it('should deny access when user only has VIEW permission and needs access for deletion', async () => {
const userId = new mongoose.Types.ObjectId();
const authorId = new mongoose.Types.ObjectId();
const agentId = uuidv4();
const fileIds = [uuidv4(), uuidv4()];
// Create users
await User.create({
_id: userId,
email: 'user@example.com',
emailVerified: true,
provider: 'local',
});
await User.create({
_id: authorId,
email: 'author@example.com',
emailVerified: true,
provider: 'local',
});
// Create agent with files
const agent = await createAgent({
id: agentId,
name: 'View-Only Agent',
author: authorId,
model: 'gpt-4',
provider: 'openai',
tool_resources: {
file_search: {
file_ids: fileIds,
},
},
});
// Grant only VIEW permission to user on the agent
await grantPermission({
principalType: PrincipalType.USER,
principalId: userId,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
accessRoleId: AccessRoleIds.AGENT_VIEWER,
grantedBy: authorId,
});
// Check access for files
const { hasAccessToFilesViaAgent } = require('~/server/services/Files/permissions');
const accessMap = await hasAccessToFilesViaAgent({
userId: userId,
role: SystemRoles.USER,
fileIds,
agentId,
isDelete: true,
});
// Should have no access to any files when only VIEW permission
expect(accessMap.get(fileIds[0])).toBe(false);
expect(accessMap.get(fileIds[1])).toBe(false);
});
it('should grant access when user has VIEW permission', async () => {
const userId = new mongoose.Types.ObjectId(); const userId = new mongoose.Types.ObjectId();
const authorId = new mongoose.Types.ObjectId(); const authorId = new mongoose.Types.ObjectId();
const agentId = uuidv4(); const agentId = uuidv4();
@ -265,9 +325,8 @@ describe('File Access Control', () => {
agentId, agentId,
}); });
// Should have no access to any files when only VIEW permission expect(accessMap.get(fileIds[0])).toBe(true);
expect(accessMap.get(fileIds[0])).toBe(false); expect(accessMap.get(fileIds[1])).toBe(true);
expect(accessMap.get(fileIds[1])).toBe(false);
}); });
}); });

View file

@ -185,6 +185,7 @@ router.delete('/', async (req, res) => {
role: req.user.role, role: req.user.role,
fileIds: nonOwnedFileIds, fileIds: nonOwnedFileIds,
agentId: req.body.agent_id, agentId: req.body.agent_id,
isDelete: true,
}); });
for (const file of nonOwnedFiles) { for (const file of nonOwnedFiles) {

View file

@ -10,9 +10,10 @@ const { getAgent } = require('~/models/Agent');
* @param {string} [params.role] - Optional user role to avoid DB query * @param {string} [params.role] - Optional user role to avoid DB query
* @param {string[]} params.fileIds - Array of file IDs to check * @param {string[]} params.fileIds - Array of file IDs to check
* @param {string} params.agentId - The agent ID that might grant access * @param {string} params.agentId - The agent ID that might grant access
* @param {boolean} [params.isDelete] - Whether the operation is a delete operation
* @returns {Promise<Map<string, boolean>>} Map of fileId to access status * @returns {Promise<Map<string, boolean>>} Map of fileId to access status
*/ */
const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId }) => { const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDelete }) => {
const accessMap = new Map(); const accessMap = new Map();
// Initialize all files as no access // Initialize all files as no access
@ -44,22 +45,23 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId }) => {
return accessMap; return accessMap;
} }
// Check if user has EDIT permission (which would indicate collaborative access) if (isDelete) {
const hasEditPermission = await checkPermission({ // Check if user has EDIT permission (which would indicate collaborative access)
userId, const hasEditPermission = await checkPermission({
role, userId,
resourceType: ResourceType.AGENT, role,
resourceId: agent._id, resourceType: ResourceType.AGENT,
requiredPermission: PermissionBits.EDIT, resourceId: agent._id,
}); requiredPermission: PermissionBits.EDIT,
});
// If user only has VIEW permission, they can't access files // If user only has VIEW permission, they can't access files
// Only users with EDIT permission or higher can access agent files // Only users with EDIT permission or higher can access agent files
if (!hasEditPermission) { if (!hasEditPermission) {
return accessMap; return accessMap;
}
} }
// User has edit permissions - check which files are actually attached
const attachedFileIds = new Set(); const attachedFileIds = new Set();
if (agent.tool_resources) { if (agent.tool_resources) {
for (const [_resourceType, resource] of Object.entries(agent.tool_resources)) { for (const [_resourceType, resource] of Object.entries(agent.tool_resources)) {