🛡️ fix: Agent Permission Check on Image Upload Route (#12219)

* fix: add agent permission check to image upload route

* refactor: remove unused SystemRoles import and format test file for clarity

* fix: address review findings for image upload agent permission check

* refactor: move agent upload auth logic to TypeScript in packages/api

Extract pure authorization logic from agentPermCheck.js into
checkAgentUploadAuth() in packages/api/src/files/agentUploadAuth.ts.
The function returns a structured result ({ allowed, status, error })
instead of writing HTTP responses directly, eliminating the dual
responsibility and confusing sentinel return value. The JS wrapper
in /api is now a thin adapter that translates the result to HTTP.

* test: rewrite image upload permission tests as integration tests

Replace mock-heavy images-agent-perm.spec.js with integration tests
using MongoMemoryServer, real models, and real PermissionService.
Follows the established pattern in files.agents.test.js. Moves test
to sibling location (images.agents.test.js) matching backend convention.
Adds temp file cleanup assertions on 403/404 responses and covers
message_file exemption paths (boolean true, string "true", false).

* fix: widen AgentUploadAuthDeps types to accept ObjectId from Mongoose

The injected getAgent returns Mongoose documents where _id and author
are Types.ObjectId at runtime, not string. Widen the DI interface to
accept string | Types.ObjectId for _id, author, and resourceId so the
contract accurately reflects real callers.

* chore: move agent upload auth into files/agents/ subdirectory

* refactor: delete agentPermCheck.js wrapper, move verifyAgentUploadPermission to packages/api

The /api-only dependencies (getAgent, checkPermission) are now passed
as object-field params from the route call sites. Both images.js and
files.js import verifyAgentUploadPermission from @librechat/api and
inject the deps directly, eliminating the intermediate JS wrapper.

* style: fix import type ordering in agent upload auth

* fix: prevent token TTL race in MCPTokenStorage.storeTokens

When expires_in is provided, use it directly instead of round-tripping
through Date arithmetic. The previous code computed accessTokenExpiry
as a Date, then after an async encryptV2 call, recomputed expiresIn by
subtracting Date.now(). On loaded CI runners the elapsed time caused
Math.floor to truncate to 0, triggering the 1-year fallback and making
the token appear permanently valid — so refresh never fired.

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

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');
+  });
+});

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

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<boolean>;
+}
+
+export async function checkAgentUploadAuth(
+  params: AgentUploadAuthParams,
+  deps: AgentUploadAuthDeps,
+): Promise<AgentUploadAuthResult> {
+  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<boolean> {
+  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;
+}

packages/api/src/files/agents/index.ts

@@ -0,0 +1 @@
+export * from './auth';

packages/api/src/files/index.ts

@@ -1,3 +1,4 @@
+export * from './agents';
 export * from './audio';
 export * from './context';
 export * from './documents/crud';

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