From b7db0dd9bc2b78bf32ecb01c2da0dc706ed826d3 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 5 Jan 2026 13:44:59 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8E=20fix:=20Allow=20Message=20Attachm?= =?UTF-8?q?ents=20for=20Users=20with=20Viewer=20Permission=20on=20Agents?= =?UTF-8?q?=20(#11210)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: allow message attachments for users with viewer permission on agents Fixes regression introduced by the agent file upload access control fix (SBA-ADV-20251204-01). The original fix was too restrictive - it blocked ALL file uploads with agent_id + tool_resource, including temporary message attachments used during chat. ## Problem Users with VIEWER permission on a shared agent could not attach files to their chat messages. The permission check blocked any upload request that included both `agent_id` and `tool_resource`, but message attachments legitimately include both fields since files need to be added to the agent's context for processing within that conversation. * test: Add permission check for file uploads with message_file set to false Introduced a new test case to ensure that file uploads are denied when the `message_file` flag is false, reinforcing permission checks for users with VIEW access on agents. This change enhances security by preventing unauthorized file uploads while maintaining functionality for legitimate message attachments. * fix: Update BadgeRow to handle undefined endpoint in ChatForm Modified the `showEphemeralBadges` prop in the `BadgeRow` component to ensure it correctly handles cases where the `endpoint` is undefined. This change improves the robustness of the chat input functionality by preventing potential errors related to endpoint checks. --- api/server/routes/files/files.agents.test.js | 109 ++++++++++++++++++ api/server/routes/files/files.js | 10 +- client/src/components/Chat/Input/ChatForm.tsx | 4 +- 3 files changed, 120 insertions(+), 3 deletions(-) diff --git a/api/server/routes/files/files.agents.test.js b/api/server/routes/files/files.agents.test.js index bdbdb8837f..7c21e95234 100644 --- a/api/server/routes/files/files.agents.test.js +++ b/api/server/routes/files/files.agents.test.js @@ -608,5 +608,114 @@ describe('File Routes - Agent Files Endpoint', () => { expect(response.status).toBe(200); expect(processAgentFileUpload).toHaveBeenCalled(); }); + + it('should allow message_file attachment to agent even without EDIT permission', async () => { + // Create an agent owned by authorId + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + // Grant only VIEW permission to otherUserId + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: authorId, + }); + + const testApp = createAppWithUser(otherUserId); + + // message_file: true indicates this is a chat message attachment, not a permanent file upload + const response = await request(testApp).post('/files').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + message_file: true, + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + expect(processAgentFileUpload).toHaveBeenCalled(); + }); + + it('should allow message_file attachment (string "true") to agent even without EDIT permission', async () => { + // Create an agent owned by authorId + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + // Grant only VIEW permission to otherUserId + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: authorId, + }); + + const testApp = createAppWithUser(otherUserId); + + // message_file as string "true" (from form data) should also be allowed + const response = await request(testApp).post('/files').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + message_file: 'true', + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + expect(processAgentFileUpload).toHaveBeenCalled(); + }); + + it('should deny file upload when message_file is false (not a message attachment)', async () => { + // Create an agent owned by authorId + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + // Grant only VIEW permission to otherUserId + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + grantedBy: authorId, + }); + + const testApp = createAppWithUser(otherUserId); + + // message_file: false should NOT bypass permission check + const response = await request(testApp).post('/files').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + message_file: false, + file_id: uuidv4(), + }); + + expect(response.status).toBe(403); + expect(response.body.error).toBe('Forbidden'); + expect(processAgentFileUpload).not.toHaveBeenCalled(); + }); }); }); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 6d18096023..5de2ddb379 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -381,8 +381,14 @@ router.post('/', async (req, res) => { return await processFileUpload({ req, res, metadata }); } - /** Check agent permissions for agent file uploads (not message attachments) */ - if (metadata.agent_id && metadata.tool_resource) { + /** + * Check agent permissions for permanent agent file uploads (not message attachments). + * Message attachments (message_file=true) are temporary files for a single conversation + * and should be allowed for users who can chat with the agent. + * Permanent file uploads to tool_resources require EDIT permission. + */ + const isMessageAttachment = metadata.message_file === true || metadata.message_file === 'true'; + if (metadata.agent_id && metadata.tool_resource && !isMessageAttachment) { const userId = req.user.id; /** Admin users bypass permission checks */ diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index b7cd2e6e9a..cb1e30a09d 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -320,7 +320,9 @@ const ChatForm = memo(({ index = 0 }: { index?: number }) => {