diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index 0929e0759d..a3718addff 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -11,6 +11,7 @@ module.exports = { delete: jest.fn(), })), logAxiosError: jest.fn(), + restoreTenantContextFromReq: jest.fn((req, res, next) => next()), ...overrides, }), diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index ded7d835d7..3d65343648 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -1,7 +1,11 @@ const multer = require('multer'); const express = require('express'); const { sleep } = require('@librechat/agents'); -const { isEnabled, resolveImportMaxFileSize } = require('@librechat/api'); +const { + isEnabled, + resolveImportMaxFileSize, + restoreTenantContextFromReq, +} = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { CacheKeys, EModelEndpoint } = require('librechat-data-provider'); const { @@ -264,6 +268,7 @@ router.post( importUserLimiter, configMiddleware, handleUpload, + restoreTenantContextFromReq, async (req, res) => { try { /* TODO: optimize to return imported conversations and add manually */ diff --git a/api/server/routes/files/index.js b/api/server/routes/files/index.js index 5d32b91036..ec00750a60 100644 --- a/api/server/routes/files/index.js +++ b/api/server/routes/files/index.js @@ -6,6 +6,7 @@ const { uaParser, checkBan, } = require('~/server/middleware'); +const { restoreTenantContextFromReq } = require('@librechat/api'); const { avatar: asstAvatarRouter } = require('~/server/routes/assistants/v1'); const { avatar: agentAvatarRouter } = require('~/server/routes/agents/v1'); const { createMulterInstance } = require('./multer'); @@ -23,7 +24,7 @@ const initialize = async () => { router.use(uaParser); const upload = await createMulterInstance(); - router.post('/speech/stt', upload.single('audio')); + router.post('/speech/stt', upload.single('audio'), restoreTenantContextFromReq); /* Important: speech route must be added before the upload limiters */ router.use('/speech', speech); @@ -43,11 +44,19 @@ const initialize = async () => { next(); }); - router.post('/', upload.single('file')); - router.post('/images', upload.single('file')); - router.post('/images/avatar', upload.single('file')); - router.post('/images/agents/:agent_id/avatar', upload.single('file')); - router.post('/images/assistants/:assistant_id/avatar', upload.single('file')); + router.post('/', upload.single('file'), restoreTenantContextFromReq); + router.post('/images', upload.single('file'), restoreTenantContextFromReq); + router.post('/images/avatar', upload.single('file'), restoreTenantContextFromReq); + router.post( + '/images/agents/:agent_id/avatar', + upload.single('file'), + restoreTenantContextFromReq, + ); + router.post( + '/images/assistants/:assistant_id/avatar', + upload.single('file'), + restoreTenantContextFromReq, + ); router.use('/', files); router.use('/images', images); diff --git a/api/server/routes/files/index.tenant.test.js b/api/server/routes/files/index.tenant.test.js new file mode 100644 index 0000000000..9b703d7524 --- /dev/null +++ b/api/server/routes/files/index.tenant.test.js @@ -0,0 +1,151 @@ +process.env.TENANT_ISOLATION_STRICT = 'true'; + +const express = require('express'); +const request = require('supertest'); +const fs = require('fs/promises'); +const { getTenantId, tenantStorage: mockTenantStorage } = require('@librechat/data-schemas'); + +const TEST_TENANT = 'tenant-files-strict'; + +let mockCurrentUser; + +jest.mock('fs/promises', () => ({ + unlink: jest.fn().mockResolvedValue(undefined), +})); + +const mockTenantResponse = (route) => (req, res) => + res.status(200).json({ route, tenantId: getTenantId() }); + +jest.mock('~/server/middleware', () => ({ + createFileLimiters: jest.fn(() => ({ + fileUploadIpLimiter: (req, res, next) => next(), + fileUploadUserLimiter: (req, res, next) => next(), + })), + configMiddleware: (req, res, next) => { + req.config = { + fileStrategy: 'local', + paths: { uploads: '/tmp/uploads', images: '/tmp/images' }, + }; + next(); + }, + requireJwtAuth: (req, res, next) => { + req.user = mockCurrentUser; + const tenantId = req.user?.tenantId; + if (!tenantId) { + next(); + return; + } + mockTenantStorage.run({ tenantId }, async () => next()); + }, + uaParser: (req, res, next) => next(), + checkBan: (req, res, next) => next(), +})); + +jest.mock('./multer', () => ({ + createMulterInstance: jest.fn(async () => ({ + single: jest.fn(() => (req, res, next) => { + req.file = { + path: '/tmp/uploaded-file', + originalname: 'uploaded.txt', + filename: 'uploaded.txt', + mimetype: 'text/plain', + size: 8, + }; + req.file_id = 'file-upload-id'; + mockTenantStorage.enterWith({}); + next(); + }), + })), +})); + +jest.mock('./files', () => { + const express = require('express'); + const router = express.Router(); + router.post('/', mockTenantResponse('files')); + return router; +}); + +jest.mock('./images', () => { + const express = require('express'); + const router = express.Router(); + router.post('/', mockTenantResponse('images')); + return router; +}); + +jest.mock('./avatar', () => { + const express = require('express'); + const router = express.Router(); + router.post('/', mockTenantResponse('avatar')); + return router; +}); + +jest.mock('./speech', () => { + const express = require('express'); + const router = express.Router(); + router.post('/stt', mockTenantResponse('speech-stt')); + return router; +}); + +jest.mock('~/server/routes/agents/v1', () => { + const express = require('express'); + const avatar = express.Router(); + avatar.post('/:agent_id/avatar/', mockTenantResponse('agent-avatar')); + return { avatar }; +}); + +jest.mock('~/server/routes/assistants/v1', () => { + const express = require('express'); + const avatar = express.Router(); + avatar.post('/:assistant_id/avatar/', mockTenantResponse('assistant-avatar')); + return { avatar }; +}); + +describe('file upload routes restore strict isolation context after multer', () => { + let app; + + beforeAll(async () => { + const { initialize } = require('./index'); + app = express(); + app.use('/api/files', await initialize()); + }); + + beforeEach(() => { + fs.unlink.mockClear(); + mockCurrentUser = { + id: 'user-files-strict', + role: 'USER', + tenantId: TEST_TENANT, + }; + }); + + afterAll(() => { + delete process.env.TENANT_ISOLATION_STRICT; + }); + + it.each([ + ['files', '/api/files'], + ['images', '/api/files/images'], + ['avatar', '/api/files/images/avatar'], + ['agent-avatar', '/api/files/images/agents/agent-1/avatar'], + ['assistant-avatar', '/api/files/images/assistants/asst-1/avatar'], + ['speech-stt', '/api/files/speech/stt'], + ])('restores context for %s upload', async (route, url) => { + const res = await request(app).post(url); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ route, tenantId: TEST_TENANT }); + }); + + it('rejects strict upload continuations when no tenant can be resolved', async () => { + mockCurrentUser = { + id: 'user-without-tenant', + role: 'USER', + }; + + const res = await request(app).post('/api/files'); + + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/Tenant context required/); + expect(fs.unlink).toHaveBeenCalledWith('/tmp/uploaded-file'); + }); +}); diff --git a/api/server/routes/files/speech/tts.js b/api/server/routes/files/speech/tts.js index ab1de205b7..ad60010e8e 100644 --- a/api/server/routes/files/speech/tts.js +++ b/api/server/routes/files/speech/tts.js @@ -1,5 +1,6 @@ const multer = require('multer'); const express = require('express'); +const { restoreTenantContextFromReq } = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { CacheKeys } = require('librechat-data-provider'); const { getVoices, streamAudio, textToSpeech } = require('~/server/services/Files/Audio'); @@ -8,7 +9,7 @@ const { getLogStores } = require('~/cache'); const router = express.Router(); const upload = multer(); -router.post('/manual', upload.none(), async (req, res) => { +router.post('/manual', upload.none(), restoreTenantContextFromReq, async (req, res) => { await textToSpeech(req, res); }); diff --git a/api/server/routes/skills.js b/api/server/routes/skills.js index 05b8f778e6..4052fceb37 100644 --- a/api/server/routes/skills.js +++ b/api/server/routes/skills.js @@ -7,6 +7,8 @@ const { createImportHandler, generateCheckAccess, getStorageMetadata, + resolveRequestTenantId, + restoreTenantContextFromReq, } = require('@librechat/api'); const { isValidObjectIdString, logger } = require('@librechat/data-schemas'); const { @@ -137,10 +139,11 @@ const importHandler = createImportHandler({ getSkillById, deleteSkill, upsertSkillFile, - saveBuffer: (req, { userId, buffer, fileName, basePath, isImage }) => { + saveBuffer: (req, { userId, buffer, fileName, basePath, isImage, tenantId }) => { + const requestTenantId = tenantId ?? resolveRequestTenantId(req); const storage = resolveSkillStorage(req, { isImage }); return storage - .saveBuffer({ userId, buffer, fileName, basePath, tenantId: req.user.tenantId }) + .saveBuffer({ userId, buffer, fileName, basePath, tenantId: requestTenantId }) .then((filepath) => ({ filepath, source: storage.source, @@ -185,6 +188,8 @@ async function uploadFileHandler(req, res) { return res.status(400).json({ error: 'Invalid file path' }); } + const tenantId = resolveRequestTenantId(req); + // Look up existing file before saving — needed to clean up old blob on replace const existingFile = await getSkillFileByPath(skillId, relativePath); @@ -199,7 +204,7 @@ async function uploadFileHandler(req, res) { buffer: file.buffer, fileName: storageFileName, basePath: 'uploads', - tenantId: req.user.tenantId, + tenantId, }); const storageMetadata = getStorageMetadata({ filepath, source: storage.source }); @@ -217,13 +222,14 @@ async function uploadFileHandler(req, res) { bytes: file.size, isExecutable: false, author: req.user._id, + tenantId, }); } catch (dbError) { // Clean up the stored blob so it doesn't leak on DB failure try { const { deleteFile } = getStrategyFunctions(storage.source); if (deleteFile) { - await deleteFile(req, { filepath, user: req.user.id, tenantId: req.user.tenantId }); + await deleteFile(req, { filepath, user: req.user.id, tenantId }); } } catch (cleanupErr) { logger.error('[uploadFile] Failed to clean up orphaned blob:', cleanupErr); @@ -238,7 +244,7 @@ async function uploadFileHandler(req, res) { delOld(req, { filepath: existingFile.filepath, user: existingFile.author ?? req.user.id, - tenantId: existingFile.tenantId ?? req.user.tenantId, + tenantId: existingFile.tenantId ?? tenantId, }).catch((e) => logger.error('[uploadFile] Old blob cleanup failed:', e)); } } @@ -264,6 +270,7 @@ router.post( fileUploadIpLimiter, fileUploadUserLimiter, skillUpload.single('file'), + restoreTenantContextFromReq, importHandler, ); @@ -303,6 +310,7 @@ router.post( fileUploadIpLimiter, fileUploadUserLimiter, singleFileUpload.single('file'), + restoreTenantContextFromReq, uploadFileHandler, ); diff --git a/api/server/routes/skills.tenant.test.js b/api/server/routes/skills.tenant.test.js new file mode 100644 index 0000000000..4586e3cbd1 --- /dev/null +++ b/api/server/routes/skills.tenant.test.js @@ -0,0 +1,377 @@ +process.env.TENANT_ISOLATION_STRICT = 'true'; + +const express = require('express'); +const request = require('supertest'); +const JSZip = require('jszip'); +const mongoose = require('mongoose'); +const { MongoMemoryServer } = require('mongodb-memory-server'); +const { tenantStorage } = require('@librechat/data-schemas'); +const { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, + PermissionBits, +} = require('librechat-data-provider'); + +const TEST_TENANT = 'tenant-skills-strict'; + +jest.mock('~/server/services/Config', () => ({ + getCachedTools: jest.fn().mockResolvedValue({}), + getAppConfig: jest.fn().mockResolvedValue({ + fileStrategy: 'local', + paths: { uploads: '/tmp/uploads', images: '/tmp/images' }, + }), +})); + +jest.mock('~/server/middleware/config/app', () => (req, _res, next) => { + req.config = { + fileStrategy: 'local', + paths: { uploads: '/tmp/uploads', images: '/tmp/images' }, + }; + next(); +}); + +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: jest.fn().mockReturnValue({ + saveBuffer: jest.fn().mockResolvedValue('/uploads/test/file.txt'), + getDownloadStream: jest.fn().mockResolvedValue({ + pipe: jest.fn(), + on: jest.fn(), + [Symbol.asyncIterator]: async function* () { + yield Buffer.from('test content'); + }, + }), + }), +})); + +jest.mock('~/server/utils/getFileStrategy', () => ({ + getFileStrategy: jest.fn().mockReturnValue('local'), +})); + +jest.mock('~/models', () => { + const mongoose = require('mongoose'); + const { createMethods } = require('@librechat/data-schemas'); + const methods = createMethods(mongoose, { + removeAllPermissions: async ({ resourceType, resourceId }) => { + const AclEntry = mongoose.models.AclEntry; + if (AclEntry) { + await AclEntry.deleteMany({ resourceType, resourceId }); + } + }, + }); + return { + ...methods, + getRoleByName: jest.fn(), + }; +}); + +jest.mock('~/server/middleware', () => { + const actual = jest.requireActual('~/server/middleware'); + const { tenantStorage } = require('@librechat/data-schemas'); + + return { + requireJwtAuth: (req, _res, next) => { + const tenantId = req.tenantId ?? req.user?.tenantId; + if (!tenantId) { + next(); + return; + } + tenantStorage.run({ tenantId }, async () => next()); + }, + canAccessSkillResource: actual.canAccessSkillResource, + }; +}); + +let app; +let mongoServer; +let Skill; +let SkillFile; +let AclEntry; +let AccessRole; +let User; +let testUsers; +let testRoles; +let currentTestUser; +let currentRequestTenantId; + +function setTestUser(user) { + currentTestUser = user; +} + +function toRequestUser(user) { + const raw = user.toObject ? user.toObject() : user; + return { + ...raw, + id: user._id.toString(), + _id: user._id, + name: user.name, + role: user.role, + tenantId: user.tenantId, + }; +} + +async function runInTenant(fn) { + return tenantStorage.run({ tenantId: TEST_TENANT }, fn); +} + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + + const dbModels = require('~/db/models'); + Skill = dbModels.Skill; + SkillFile = dbModels.SkillFile; + AclEntry = dbModels.AclEntry; + AccessRole = dbModels.AccessRole; + User = dbModels.User; + + await runInTenant(setupTestData); + + app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + if (currentRequestTenantId) { + req.tenantId = currentRequestTenantId; + } + if (currentTestUser) { + req.user = toRequestUser(currentTestUser); + } + next(); + }); + + currentTestUser = testUsers.owner; + app.use('/api/skills', require('./skills')); +}); + +afterEach(async () => { + await runInTenant(async () => { + await Skill.deleteMany({}); + await SkillFile.deleteMany({}); + await AclEntry.deleteMany({}); + }); + currentTestUser = testUsers.owner; + currentRequestTenantId = undefined; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); + delete process.env.TENANT_ISOLATION_STRICT; + jest.clearAllMocks(); +}); + +async function setupTestData() { + testRoles = { + owner: await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_OWNER, + name: 'Owner', + resourceType: ResourceType.SKILL, + permBits: + PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE, + tenantId: TEST_TENANT, + }), + }; + + testUsers = { + owner: await User.create({ + name: 'Strict Skill Owner', + email: 'strict-skill-owner@test.com', + role: SystemRoles.USER, + tenantId: TEST_TENANT, + }), + }; + + const { getRoleByName } = require('~/models'); + getRoleByName.mockImplementation((roleName) => { + if (roleName === SystemRoles.USER || roleName === SystemRoles.ADMIN) { + return { + permissions: { + SKILLS: { + USE: true, + CREATE: true, + SHARE: true, + SHARE_PUBLIC: true, + }, + }, + }; + } + return null; + }); +} + +async function createSkillAsOwner(overrides = {}) { + return request(app) + .post('/api/skills') + .send({ + name: 'strict-file-skill', + description: 'A strict tenant skill used in multipart route tests.', + body: '# Strict File Skill', + ...overrides, + }); +} + +describe('Skill multipart routes under strict tenant isolation', () => { + it('imports a skill zip and writes skill, ACL, and files in the request tenant', async () => { + const zip = new JSZip(); + zip.file( + 'SKILL.md', + [ + '---', + 'name: strict-import', + 'description: Strict tenant import route test skill.', + '---', + '# Strict Import', + ].join('\n'), + ); + zip.file('scripts/run.sh', 'echo strict'); + const buffer = await zip.generateAsync({ type: 'nodebuffer' }); + + const res = await request(app).post('/api/skills/import').attach('file', buffer, { + filename: 'strict-import.skill', + contentType: 'application/zip', + }); + + expect(res.status).toBe(201); + expect(res.body.tenantId).toBe(TEST_TENANT); + expect(res.body._importSummary.filesSucceeded).toBe(1); + + const { skill, acl, file } = await runInTenant(async () => { + const skill = await Skill.findOne({ name: 'strict-import' }).lean(); + const acl = await AclEntry.findOne({ + resourceType: ResourceType.SKILL, + resourceId: skill._id, + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + }).lean(); + const file = await SkillFile.findOne({ + skillId: skill._id, + relativePath: 'scripts/run.sh', + }).lean(); + return { skill, acl, file }; + }); + + expect(skill.tenantId).toBe(TEST_TENANT); + expect(acl).toEqual( + expect.objectContaining({ + tenantId: TEST_TENANT, + roleId: testRoles.owner._id, + }), + ); + expect(file).toEqual( + expect.objectContaining({ + tenantId: TEST_TENANT, + relativePath: 'scripts/run.sh', + }), + ); + }); + + it('imports using the resolved request tenant when user tenant differs', async () => { + currentRequestTenantId = TEST_TENANT; + setTestUser({ + ...testUsers.owner.toObject(), + tenantId: 'stale-user-tenant', + }); + + const res = await request(app) + .post('/api/skills/import') + .attach('file', Buffer.from('# Request Tenant Markdown'), { + filename: 'request-tenant.md', + contentType: 'text/markdown', + }); + + expect(res.status).toBe(201); + expect(res.body.tenantId).toBe(TEST_TENANT); + + const savedSkill = await runInTenant(async () => + Skill.findOne({ name: 'request-tenant' }).lean(), + ); + expect(savedSkill).toEqual( + expect.objectContaining({ + tenantId: TEST_TENANT, + }), + ); + }); + + it('rejects multipart import in strict mode when the request has no tenant', async () => { + setTestUser({ + _id: new mongoose.Types.ObjectId(), + name: 'No Tenant', + role: SystemRoles.USER, + }); + + const res = await request(app) + .post('/api/skills/import') + .attach('file', Buffer.from('# No Tenant'), { + filename: 'no-tenant.md', + contentType: 'text/markdown', + }); + + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/Tenant context required/); + }); + + it('uploads an individual skill file in the request tenant', async () => { + const created = await createSkillAsOwner(); + expect(created.status).toBe(201); + + const res = await request(app) + .post(`/api/skills/${created.body._id}/files`) + .field('relativePath', 'scripts/manual.sh') + .attach('file', Buffer.from('echo manual'), { + filename: 'manual.sh', + contentType: 'text/x-shellscript', + }); + + expect(res.status).toBe(200); + expect(res.body.tenantId).toBe(TEST_TENANT); + + const savedFile = await runInTenant(async () => + SkillFile.findOne({ + skillId: created.body._id, + relativePath: 'scripts/manual.sh', + }).lean(), + ); + expect(savedFile).toEqual( + expect.objectContaining({ + tenantId: TEST_TENANT, + relativePath: 'scripts/manual.sh', + }), + ); + }); + + it('uploads an individual skill file using the resolved request tenant', async () => { + const created = await createSkillAsOwner({ name: 'request-tenant-file-skill' }); + expect(created.status).toBe(201); + + currentRequestTenantId = TEST_TENANT; + setTestUser({ + ...testUsers.owner.toObject(), + tenantId: 'stale-user-tenant', + }); + + const res = await request(app) + .post(`/api/skills/${created.body._id}/files`) + .field('relativePath', 'scripts/request-tenant.sh') + .attach('file', Buffer.from('echo request tenant'), { + filename: 'request-tenant.sh', + contentType: 'text/x-shellscript', + }); + + expect(res.status).toBe(200); + expect(res.body.tenantId).toBe(TEST_TENANT); + + const savedFile = await runInTenant(async () => + SkillFile.findOne({ + skillId: created.body._id, + relativePath: 'scripts/request-tenant.sh', + }).lean(), + ); + expect(savedFile).toEqual( + expect.objectContaining({ + tenantId: TEST_TENANT, + relativePath: 'scripts/request-tenant.sh', + }), + ); + }); +}); diff --git a/packages/api/src/middleware/__tests__/tenant.spec.ts b/packages/api/src/middleware/__tests__/tenant.spec.ts index 7451817941..d393cdfe0d 100644 --- a/packages/api/src/middleware/__tests__/tenant.spec.ts +++ b/packages/api/src/middleware/__tests__/tenant.spec.ts @@ -1,14 +1,30 @@ -import { getTenantId } from '@librechat/data-schemas'; +import { unlink } from 'fs/promises'; +import { getTenantId, SYSTEM_TENANT_ID } from '@librechat/data-schemas'; import type { Response, NextFunction } from 'express'; import type { ServerRequest } from '~/types/http'; // Import directly from source file — _resetTenantMiddlewareStrictCache is intentionally // excluded from the public barrel export (index.ts). -import { tenantContextMiddleware, _resetTenantMiddlewareStrictCache } from '../tenant'; +import { + tenantContextMiddleware, + restoreTenantContextFromReq, + resolveRequestTenantId, + _resetTenantMiddlewareStrictCache, +} from '../tenant'; + +jest.mock('fs/promises', () => ({ + unlink: jest.fn().mockResolvedValue(undefined), +})); + +const unlinkMock = unlink as jest.MockedFunction; function mockReq(user?: Record): ServerRequest { return { user } as unknown as ServerRequest; } +function mockTenantReq(user?: Record, tenantId?: string): ServerRequest { + return { user, tenantId } as unknown as ServerRequest; +} + function mockRes(): Response { const res = { status: jest.fn().mockReturnThis(), @@ -31,6 +47,7 @@ describe('tenantContextMiddleware', () => { afterEach(() => { _resetTenantMiddlewareStrictCache(); delete process.env.TENANT_ISOLATION_STRICT; + unlinkMock.mockClear(); }); it('sets ALS tenant context for authenticated requests with tenantId', async () => { @@ -57,7 +74,7 @@ describe('tenantContextMiddleware', () => { expect(tenantId).toBeUndefined(); }); - it('returns 403 when user has no tenantId in strict mode', () => { + it('returns 403 when user has no tenantId in strict mode', async () => { process.env.TENANT_ISOLATION_STRICT = 'true'; _resetTenantMiddlewareStrictCache(); @@ -65,7 +82,7 @@ describe('tenantContextMiddleware', () => { const res = mockRes(); const next: NextFunction = jest.fn(); - tenantContextMiddleware(req, res, next); + await tenantContextMiddleware(req, res, next); expect(res.status).toHaveBeenCalledWith(403); expect(res.json).toHaveBeenCalledWith( @@ -99,3 +116,119 @@ describe('tenantContextMiddleware', () => { expect(results).toContain('tenant-2'); }); }); + +describe('restoreTenantContextFromReq', () => { + afterEach(() => { + _resetTenantMiddlewareStrictCache(); + delete process.env.TENANT_ISOLATION_STRICT; + unlinkMock.mockClear(); + }); + + it('restores ALS tenant context from req.user.tenantId', async () => { + const req = mockReq({ tenantId: 'tenant-user', role: 'user' }); + const res = mockRes(); + + const tenantId = await new Promise((resolve) => { + const next: NextFunction = async () => { + await Promise.resolve(); + resolve(getTenantId()); + }; + restoreTenantContextFromReq(req, res, next); + }); + + expect(tenantId).toBe('tenant-user'); + }); + + it('prefers server-resolved req.tenantId over req.user.tenantId', async () => { + const req = mockTenantReq({ tenantId: 'tenant-user', role: 'user' }, 'tenant-request'); + const res = mockRes(); + + const tenantId = await new Promise((resolve) => { + restoreTenantContextFromReq(req, res, () => { + resolve(getTenantId()); + }); + }); + + expect(tenantId).toBe('tenant-request'); + }); + + it('returns 403 in strict mode when no request tenant can be resolved', async () => { + process.env.TENANT_ISOLATION_STRICT = 'true'; + _resetTenantMiddlewareStrictCache(); + + const req = mockReq({ role: 'user' }); + const res = mockRes(); + const next: NextFunction = jest.fn(); + + await restoreTenantContextFromReq(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('Tenant context required') }), + ); + expect(next).not.toHaveBeenCalled(); + }); + + it('deletes uploaded temp files before rejecting strict requests without a tenant', async () => { + process.env.TENANT_ISOLATION_STRICT = 'true'; + _resetTenantMiddlewareStrictCache(); + + const req = { + ...mockReq({ role: 'user' }), + file: { path: '/tmp/no-tenant-upload' }, + } as ServerRequest; + const res = mockRes(); + const next: NextFunction = jest.fn(); + + await restoreTenantContextFromReq(req, res, next); + + expect(unlinkMock).toHaveBeenCalledWith('/tmp/no-tenant-upload'); + expect(res.status).toHaveBeenCalledWith(403); + expect(next).not.toHaveBeenCalled(); + }); + + it('rejects the system tenant sentinel for request-owned work', async () => { + const req = mockReq({ tenantId: SYSTEM_TENANT_ID, role: 'user' }); + const res = mockRes(); + const next: NextFunction = jest.fn(); + + await restoreTenantContextFromReq(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(next).not.toHaveBeenCalled(); + }); + + it('deletes uploaded temp files before rejecting system-tenant requests', async () => { + const req = { + ...mockReq({ tenantId: SYSTEM_TENANT_ID, role: 'user' }), + file: { path: '/tmp/system-tenant-upload' }, + files: [{ path: '/tmp/system-tenant-extra' }], + } as ServerRequest; + const res = mockRes(); + const next: NextFunction = jest.fn(); + + await restoreTenantContextFromReq(req, res, next); + + expect(unlinkMock).toHaveBeenCalledWith('/tmp/system-tenant-upload'); + expect(unlinkMock).toHaveBeenCalledWith('/tmp/system-tenant-extra'); + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'System tenant is not allowed for request-scoped routes', + }); + expect(next).not.toHaveBeenCalled(); + }); +}); + +describe('resolveRequestTenantId', () => { + it('uses req.tenantId before req.user.tenantId', () => { + const req = mockTenantReq({ tenantId: 'tenant-user', role: 'user' }, 'tenant-request'); + + expect(resolveRequestTenantId(req)).toBe('tenant-request'); + }); + + it('falls back to req.user.tenantId', () => { + const req = mockReq({ tenantId: 'tenant-user', role: 'user' }); + + expect(resolveRequestTenantId(req)).toBe('tenant-user'); + }); +}); diff --git a/packages/api/src/middleware/index.ts b/packages/api/src/middleware/index.ts index f14e65fc49..cac5d637b9 100644 --- a/packages/api/src/middleware/index.ts +++ b/packages/api/src/middleware/index.ts @@ -5,7 +5,11 @@ export * from './notFound'; export * from './balance'; export * from './json'; export * from './capabilities'; -export { tenantContextMiddleware } from './tenant'; +export { + tenantContextMiddleware, + restoreTenantContextFromReq, + resolveRequestTenantId, +} from './tenant'; export { preAuthTenantMiddleware } from './preAuthTenant'; export * from './concurrency'; export * from './checkBalance'; diff --git a/packages/api/src/middleware/tenant.ts b/packages/api/src/middleware/tenant.ts index 0b0e003991..d0907e0d65 100644 --- a/packages/api/src/middleware/tenant.ts +++ b/packages/api/src/middleware/tenant.ts @@ -1,5 +1,6 @@ +import { unlink } from 'fs/promises'; import { isMainThread } from 'worker_threads'; -import { tenantStorage, logger } from '@librechat/data-schemas'; +import { getTenantId, tenantStorage, logger, SYSTEM_TENANT_ID } from '@librechat/data-schemas'; import type { Response, NextFunction } from 'express'; import type { ServerRequest } from '~/types/http'; @@ -68,3 +69,117 @@ export function tenantContextMiddleware( next(); }); } + +export type RequestTenantSource = { + tenantId?: string; + user?: { tenantId?: string } | null; +}; + +export function resolveRequestTenantId(req: RequestTenantSource): string | undefined { + return req.tenantId ?? req.user?.tenantId; +} + +type UploadFile = { + path?: string; +}; + +type UploadRequest = ServerRequest & { + file?: UploadFile; + files?: UploadFile[] | Record; +}; + +function collectUploadPaths(req: UploadRequest): string[] { + const paths = new Set(); + if (req.file?.path) { + paths.add(req.file.path); + } + const { files } = req; + if (Array.isArray(files)) { + files.forEach((file) => { + if (file.path) { + paths.add(file.path); + } + }); + } else if (files) { + Object.values(files).forEach((uploads) => { + uploads.forEach((file) => { + if (file.path) { + paths.add(file.path); + } + }); + }); + } + return [...paths]; +} + +async function cleanupUploadedFiles(req: ServerRequest): Promise { + const paths = collectUploadPaths(req as UploadRequest); + if (paths.length === 0) { + return; + } + const results = await Promise.allSettled(paths.map((filepath) => unlink(filepath))); + results.forEach((result, index) => { + if (result.status === 'rejected') { + logger.error('[restoreTenantContextFromReq] Failed to delete rejected upload:', { + path: paths[index], + error: result.reason, + }); + } + }); +} + +async function rejectRequestWithUploadCleanup( + req: ServerRequest, + res: Response, + message: string, +): Promise { + await cleanupUploadedFiles(req); + res.status(403).json({ error: message }); +} + +/** + * Re-enters tenant ALS from the server-resolved request tenant. + * + * Use this after middleware that may cross async stream boundaries (for example + * multipart parsers) and before tenant-isolated model calls. The tenant source + * is restricted to authenticated/resolved request fields, never form data. + */ +export function restoreTenantContextFromReq( + req: ServerRequest, + res: Response, + next: NextFunction, +): void | Promise { + const tenantId = resolveRequestTenantId(req as RequestTenantSource); + + if (!tenantId) { + if (isStrict()) { + return rejectRequestWithUploadCleanup( + req, + res, + 'Tenant context required in strict isolation mode', + ); + } + next(); + return; + } + + if (tenantId === SYSTEM_TENANT_ID) { + logger.warn('[restoreTenantContextFromReq] Rejected system tenant for request route', { + path: req.path, + }); + return rejectRequestWithUploadCleanup( + req, + res, + 'System tenant is not allowed for request-scoped routes', + ); + } + + if (getTenantId() === tenantId) { + next(); + return; + } + + return void tenantStorage.run({ tenantId }, async () => { + next(); + }); +} diff --git a/packages/api/src/skills/import.ts b/packages/api/src/skills/import.ts index b0e5ab3f90..817830cbe5 100644 --- a/packages/api/src/skills/import.ts +++ b/packages/api/src/skills/import.ts @@ -12,6 +12,7 @@ import type { CreateSkillResult, UpsertSkillFileInput, } from '@librechat/data-schemas'; +import { resolveRequestTenantId } from '~/middleware/tenant'; /** Security limits for zip processing. */ const MAX_ZIP_BYTES = 50 * 1024 * 1024; // 50 MB compressed @@ -194,6 +195,7 @@ export interface ImportSkillDeps { } interface ServerRequest extends Request { + tenantId?: string; user: { id: string; _id: Types.ObjectId; @@ -260,7 +262,7 @@ function getAuthorInfo(req: ServerRequest) { const user = req.user; const authorId = (user._id ?? user.id) as unknown as Types.ObjectId; const authorName = user.name ?? user.username ?? 'Unknown'; - const tenantId = user.tenantId; + const tenantId = resolveRequestTenantId(req); return { authorId, authorName, tenantId }; }