diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 5de2ddb379..9290d1a7ed 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -2,12 +2,12 @@ const fs = require('fs').promises; const express = require('express'); const { EnvVar } = require('@librechat/agents'); const { logger } = require('@librechat/data-schemas'); +const { verifyAgentUploadPermission } = require('@librechat/api'); const { Time, isUUID, CacheKeys, FileSources, - SystemRoles, ResourceType, EModelEndpoint, PermissionBits, @@ -381,48 +381,15 @@ router.post('/', async (req, res) => { return await processFileUpload({ req, res, metadata }); } - /** - * 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 */ - if (req.user.role !== SystemRoles.ADMIN) { - const agent = await getAgent({ id: metadata.agent_id }); - - if (!agent) { - return res.status(404).json({ - error: 'Not Found', - message: 'Agent not found', - }); - } - - /** Check if user is the author or has edit permission */ - if (agent.author.toString() !== userId) { - const hasEditPermission = await checkPermission({ - userId, - role: req.user.role, - resourceType: ResourceType.AGENT, - resourceId: agent._id, - requiredPermission: PermissionBits.EDIT, - }); - - if (!hasEditPermission) { - logger.warn( - `[/files] User ${userId} denied upload to agent ${metadata.agent_id} (insufficient permissions)`, - ); - return res.status(403).json({ - error: 'Forbidden', - message: 'Insufficient permissions to upload files to this agent', - }); - } - } - } + const denied = await verifyAgentUploadPermission({ + req, + res, + metadata, + getAgent, + checkPermission, + }); + if (denied) { + return; } return await processAgentFileUpload({ req, res, metadata }); diff --git a/api/server/routes/files/images.agents.test.js b/api/server/routes/files/images.agents.test.js new file mode 100644 index 0000000000..862ab87d63 --- /dev/null +++ b/api/server/routes/files/images.agents.test.js @@ -0,0 +1,376 @@ +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { v4: uuidv4 } = require('uuid'); +const { createMethods } = require('@librechat/data-schemas'); +const { MongoMemoryServer } = require('mongodb-memory-server'); +const { + SystemRoles, + AccessRoleIds, + ResourceType, + PrincipalType, +} = require('librechat-data-provider'); +const { createAgent } = require('~/models/Agent'); + +jest.mock('~/server/services/Files/process', () => ({ + processAgentFileUpload: jest.fn().mockImplementation(async ({ res }) => { + return res.status(200).json({ message: 'Agent file uploaded', file_id: 'test-file-id' }); + }), + processImageFile: jest.fn().mockImplementation(async ({ res }) => { + return res.status(200).json({ message: 'Image processed' }); + }), + filterFile: jest.fn(), +})); + +jest.mock('fs', () => { + const actualFs = jest.requireActual('fs'); + return { + ...actualFs, + promises: { + ...actualFs.promises, + unlink: jest.fn().mockResolvedValue(undefined), + }, + }; +}); + +const fs = require('fs'); +const { processAgentFileUpload } = require('~/server/services/Files/process'); + +const router = require('~/server/routes/files/images'); + +describe('POST /images - Agent Upload Permission Check (Integration)', () => { + let mongoServer; + let authorId; + let otherUserId; + let agentCustomId; + let User; + let Agent; + let AclEntry; + let methods; + let modelsToCleanup = []; + + beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + const mongoUri = mongoServer.getUri(); + await mongoose.connect(mongoUri); + + const { createModels } = require('@librechat/data-schemas'); + const models = createModels(mongoose); + modelsToCleanup = Object.keys(models); + Object.assign(mongoose.models, models); + methods = createMethods(mongoose); + + User = models.User; + Agent = models.Agent; + AclEntry = models.AclEntry; + + await methods.seedDefaultRoles(); + }); + + afterAll(async () => { + const collections = mongoose.connection.collections; + for (const key in collections) { + await collections[key].deleteMany({}); + } + for (const modelName of modelsToCleanup) { + if (mongoose.models[modelName]) { + delete mongoose.models[modelName]; + } + } + await mongoose.disconnect(); + await mongoServer.stop(); + }); + + beforeEach(async () => { + await Agent.deleteMany({}); + await User.deleteMany({}); + await AclEntry.deleteMany({}); + + authorId = new mongoose.Types.ObjectId(); + otherUserId = new mongoose.Types.ObjectId(); + agentCustomId = `agent_${uuidv4().replace(/-/g, '').substring(0, 21)}`; + + await User.create({ _id: authorId, username: 'author', email: 'author@test.com' }); + await User.create({ _id: otherUserId, username: 'other', email: 'other@test.com' }); + + jest.clearAllMocks(); + }); + + const createAppWithUser = (userId, userRole = SystemRoles.USER) => { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + if (req.method === 'POST') { + req.file = { + originalname: 'test.png', + mimetype: 'image/png', + size: 100, + path: '/tmp/t.png', + filename: 'test.png', + }; + req.file_id = uuidv4(); + } + next(); + }); + app.use((req, _res, next) => { + req.user = { id: userId.toString(), role: userRole }; + req.app = { locals: {} }; + req.config = { fileStrategy: 'local', paths: { imageOutput: '/tmp/images' } }; + next(); + }); + app.use('/images', router); + return app; + }; + + it('should return 403 when user has no permission on agent', async () => { + await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + const app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(403); + expect(response.body.error).toBe('Forbidden'); + expect(processAgentFileUpload).not.toHaveBeenCalled(); + expect(fs.promises.unlink).toHaveBeenCalledWith('/tmp/t.png'); + }); + + it('should allow upload for agent owner', async () => { + await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + const app = createAppWithUser(authorId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + expect(processAgentFileUpload).toHaveBeenCalled(); + }); + + it('should allow upload for admin regardless of ownership', async () => { + await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + const app = createAppWithUser(otherUserId, SystemRoles.ADMIN); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + expect(processAgentFileUpload).toHaveBeenCalled(); + }); + + it('should allow upload for user with EDIT permission', async () => { + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + const { grantPermission } = require('~/server/services/PermissionService'); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: otherUserId, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + accessRoleId: AccessRoleIds.AGENT_EDITOR, + grantedBy: authorId, + }); + + const app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + expect(processAgentFileUpload).toHaveBeenCalled(); + }); + + it('should deny upload for user with only VIEW permission', async () => { + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + 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 app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: agentCustomId, + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(403); + expect(response.body.error).toBe('Forbidden'); + expect(processAgentFileUpload).not.toHaveBeenCalled(); + expect(fs.promises.unlink).toHaveBeenCalledWith('/tmp/t.png'); + }); + + it('should skip permission check for regular image uploads without agent_id/tool_resource', async () => { + const app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + file_id: uuidv4(), + }); + + expect(response.status).toBe(200); + }); + + it('should return 404 for non-existent agent', async () => { + const app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').send({ + endpoint: 'agents', + agent_id: 'agent_nonexistent123456789', + tool_resource: 'context', + file_id: uuidv4(), + }); + + expect(response.status).toBe(404); + expect(response.body.error).toBe('Not Found'); + expect(processAgentFileUpload).not.toHaveBeenCalled(); + expect(fs.promises.unlink).toHaveBeenCalledWith('/tmp/t.png'); + }); + + it('should allow message_file attachment (boolean true) without EDIT permission', async () => { + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + 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 app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').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") without EDIT permission', async () => { + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + 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 app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').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 upload when message_file is false (not a message attachment)', async () => { + const agent = await createAgent({ + id: agentCustomId, + name: 'Test Agent', + provider: 'openai', + model: 'gpt-4', + author: authorId, + }); + + 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 app = createAppWithUser(otherUserId); + const response = await request(app).post('/images').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(); + expect(fs.promises.unlink).toHaveBeenCalledWith('/tmp/t.png'); + }); +}); diff --git a/api/server/routes/files/images.js b/api/server/routes/files/images.js index 8072612a69..185ec7a671 100644 --- a/api/server/routes/files/images.js +++ b/api/server/routes/files/images.js @@ -2,12 +2,15 @@ const path = require('path'); const fs = require('fs').promises; const express = require('express'); const { logger } = require('@librechat/data-schemas'); +const { verifyAgentUploadPermission } = require('@librechat/api'); const { isAssistantsEndpoint } = require('librechat-data-provider'); const { processAgentFileUpload, processImageFile, filterFile, } = require('~/server/services/Files/process'); +const { checkPermission } = require('~/server/services/PermissionService'); +const { getAgent } = require('~/models/Agent'); const router = express.Router(); @@ -22,6 +25,16 @@ router.post('/', async (req, res) => { metadata.file_id = req.file_id; if (!isAssistantsEndpoint(metadata.endpoint) && metadata.tool_resource != null) { + const denied = await verifyAgentUploadPermission({ + req, + res, + metadata, + getAgent, + checkPermission, + }); + if (denied) { + return; + } return await processAgentFileUpload({ req, res, metadata }); } diff --git a/packages/api/src/files/agents/auth.ts b/packages/api/src/files/agents/auth.ts new file mode 100644 index 0000000000..d9fb2b7423 --- /dev/null +++ b/packages/api/src/files/agents/auth.ts @@ -0,0 +1,113 @@ +import type { IUser } from '@librechat/data-schemas'; +import type { Response } from 'express'; +import type { Types } from 'mongoose'; +import { logger } from '@librechat/data-schemas'; +import { SystemRoles, ResourceType, PermissionBits } from 'librechat-data-provider'; +import type { ServerRequest } from '~/types'; + +export type AgentUploadAuthResult = + | { allowed: true } + | { allowed: false; status: number; error: string; message: string }; + +export interface AgentUploadAuthParams { + userId: string; + userRole: string; + agentId?: string; + toolResource?: string | null; + messageFile?: boolean | string; +} + +export interface AgentUploadAuthDeps { + getAgent: (params: { id: string }) => Promise<{ + _id: string | Types.ObjectId; + author?: string | Types.ObjectId | null; + } | null>; + checkPermission: (params: { + userId: string; + role: string; + resourceType: ResourceType; + resourceId: string | Types.ObjectId; + requiredPermission: number; + }) => Promise; +} + +export async function checkAgentUploadAuth( + params: AgentUploadAuthParams, + deps: AgentUploadAuthDeps, +): Promise { + const { userId, userRole, agentId, toolResource, messageFile } = params; + const { getAgent, checkPermission } = deps; + + const isMessageAttachment = messageFile === true || messageFile === 'true'; + if (!agentId || toolResource == null || isMessageAttachment) { + return { allowed: true }; + } + + if (userRole === SystemRoles.ADMIN) { + return { allowed: true }; + } + + const agent = await getAgent({ id: agentId }); + if (!agent) { + return { allowed: false, status: 404, error: 'Not Found', message: 'Agent not found' }; + } + + if (agent.author?.toString() === userId) { + return { allowed: true }; + } + + const hasEditPermission = await checkPermission({ + userId, + role: userRole, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + requiredPermission: PermissionBits.EDIT, + }); + + if (hasEditPermission) { + return { allowed: true }; + } + + logger.warn( + `[agentUploadAuth] User ${userId} denied upload to agent ${agentId} (insufficient permissions)`, + ); + return { + allowed: false, + status: 403, + error: 'Forbidden', + message: 'Insufficient permissions to upload files to this agent', + }; +} + +/** @returns true if denied (response already sent), false if allowed */ +export async function verifyAgentUploadPermission({ + req, + res, + metadata, + getAgent, + checkPermission, +}: { + req: ServerRequest; + res: Response; + metadata: { agent_id?: string; tool_resource?: string | null; message_file?: boolean | string }; + getAgent: AgentUploadAuthDeps['getAgent']; + checkPermission: AgentUploadAuthDeps['checkPermission']; +}): Promise { + const user = req.user as IUser; + const result = await checkAgentUploadAuth( + { + userId: user.id, + userRole: user.role ?? '', + agentId: metadata.agent_id, + toolResource: metadata.tool_resource, + messageFile: metadata.message_file, + }, + { getAgent, checkPermission }, + ); + + if (!result.allowed) { + res.status(result.status).json({ error: result.error, message: result.message }); + return true; + } + return false; +} diff --git a/packages/api/src/files/agents/index.ts b/packages/api/src/files/agents/index.ts new file mode 100644 index 0000000000..269586ee8b --- /dev/null +++ b/packages/api/src/files/agents/index.ts @@ -0,0 +1 @@ +export * from './auth'; diff --git a/packages/api/src/files/index.ts b/packages/api/src/files/index.ts index 707f2ef7fb..c3bdb49478 100644 --- a/packages/api/src/files/index.ts +++ b/packages/api/src/files/index.ts @@ -1,3 +1,4 @@ +export * from './agents'; export * from './audio'; export * from './context'; export * from './documents/crud'; diff --git a/packages/api/src/mcp/oauth/tokens.ts b/packages/api/src/mcp/oauth/tokens.ts index 7b1d189347..6094a05386 100644 --- a/packages/api/src/mcp/oauth/tokens.ts +++ b/packages/api/src/mcp/oauth/tokens.ts @@ -83,46 +83,40 @@ export class MCPTokenStorage { `${logPrefix} Token expires_in: ${'expires_in' in tokens ? tokens.expires_in : 'N/A'}, expires_at: ${'expires_at' in tokens ? tokens.expires_at : 'N/A'}`, ); - // Handle both expires_in and expires_at formats + const defaultTTL = 365 * 24 * 60 * 60; + let accessTokenExpiry: Date; + let expiresInSeconds: number; if ('expires_at' in tokens && tokens.expires_at) { /** MCPOAuthTokens format - already has calculated expiry */ logger.debug(`${logPrefix} Using expires_at: ${tokens.expires_at}`); accessTokenExpiry = new Date(tokens.expires_at); + expiresInSeconds = Math.floor((accessTokenExpiry.getTime() - Date.now()) / 1000); } else if (tokens.expires_in) { - /** Standard OAuthTokens format - calculate expiry */ + /** Standard OAuthTokens format - use expires_in directly to avoid lossy Date round-trip */ logger.debug(`${logPrefix} Using expires_in: ${tokens.expires_in}`); + expiresInSeconds = tokens.expires_in; accessTokenExpiry = new Date(Date.now() + tokens.expires_in * 1000); } else { - /** No expiry provided - default to 1 year */ logger.debug(`${logPrefix} No expiry provided, using default`); - accessTokenExpiry = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000); + expiresInSeconds = defaultTTL; + accessTokenExpiry = new Date(Date.now() + defaultTTL * 1000); } logger.debug(`${logPrefix} Calculated expiry date: ${accessTokenExpiry.toISOString()}`); - logger.debug( - `${logPrefix} Date object: ${JSON.stringify({ - time: accessTokenExpiry.getTime(), - valid: !isNaN(accessTokenExpiry.getTime()), - iso: accessTokenExpiry.toISOString(), - })}`, - ); - // Ensure the date is valid before passing to createToken if (isNaN(accessTokenExpiry.getTime())) { logger.error(`${logPrefix} Invalid expiry date calculated, using default`); - accessTokenExpiry = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000); + accessTokenExpiry = new Date(Date.now() + defaultTTL * 1000); + expiresInSeconds = defaultTTL; } - // Calculate expiresIn (seconds from now) - const expiresIn = Math.floor((accessTokenExpiry.getTime() - Date.now()) / 1000); - const accessTokenData = { userId, type: 'mcp_oauth', identifier, token: encryptedAccessToken, - expiresIn: expiresIn > 0 ? expiresIn : 365 * 24 * 60 * 60, // Default to 1 year if negative + expiresIn: expiresInSeconds > 0 ? expiresInSeconds : defaultTTL, }; // Check if token already exists and update if it does