mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-28 03:37:20 +00:00
🛂 fix: Judge upload content against the destination the file will reach
A deployment that fail-closes on an uninspectable derived field rejected unified uploads it should have accepted. The content preflight was told the request's tool resource, which unified mode leaves empty, so it could not see that routing would promote the file to a text context and extract inspectable text from it. The same PDF, image or audio file was accepted when uploaded through the legacy chooser as an explicit context resource. Both routes now resolve the effective destination before the preflight and pass it in, so the check judges what processing will actually do. An explicit ocr resource resolves to context here as well, matching what the processing path already does with it. The resolution helpers move out of the file processing service into their own routing module. The routes need them while several suites mock that service wholesale, and a mocked resolver silently changes upload dispatch rather than failing loudly.
This commit is contained in:
parent
a67665d0a4
commit
25eea4832d
6 changed files with 216 additions and 119 deletions
|
|
@ -39,7 +39,11 @@ const {
|
|||
processDeleteRequest,
|
||||
processAgentFileUpload,
|
||||
} = require('~/server/services/Files/process');
|
||||
const { resolveUploadEndpoint, resolveUploadAgent } = require('~/server/services/Files/agent');
|
||||
const {
|
||||
resolveEffectiveToolResource,
|
||||
resolveUploadEndpoint,
|
||||
resolveUploadAgent,
|
||||
} = require('~/server/services/Files/routing');
|
||||
const { fileAccess } = require('~/server/middleware/accessResources/fileAccess');
|
||||
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
||||
const { getOpenAIClient } = require('~/server/controllers/assistants/helpers');
|
||||
|
|
@ -741,11 +745,16 @@ router.post('/', async (req, res) => {
|
|||
});
|
||||
filterFile({ req, endpoint: effectiveEndpoint });
|
||||
|
||||
/* Same destination the processing path will use: a unified upload routed to text
|
||||
* becomes a context resource, and the preflight must account for that extraction
|
||||
* before fail-closing on an uninspectable derived field. */
|
||||
const effectiveToolResource = await resolveEffectiveToolResource({ req, metadata });
|
||||
|
||||
await assertUploadContentAllowed({
|
||||
filters: req.config?.filters,
|
||||
file: req.file,
|
||||
endpoint: metadata.endpoint,
|
||||
toolResource: metadata.tool_resource,
|
||||
toolResource: effectiveToolResource,
|
||||
fileConfig: mergeFileConfig(req.config?.fileConfig),
|
||||
ocrConfigured: req.config?.ocr != null,
|
||||
ragConfigured: !!process.env.RAG_API_URL,
|
||||
|
|
|
|||
|
|
@ -20,9 +20,20 @@ jest.mock('~/server/services/Files/process', () => ({
|
|||
return res.status(200).json({ message: 'Image processed' });
|
||||
}),
|
||||
filterFile: jest.fn(),
|
||||
resolvesToTextDelivery: jest.fn().mockResolvedValue(false),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/routing', () => {
|
||||
const actual = jest.requireActual('~/server/services/Files/routing');
|
||||
return {
|
||||
...actual,
|
||||
/* Real by default so the dispatch is exercised end to end; individual tests override
|
||||
* it for a single call to stand in for a routing configuration. */
|
||||
resolveEffectiveToolResource: jest.fn((...args) =>
|
||||
actual.resolveEffectiveToolResource(...args),
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('fs', () => {
|
||||
const actualFs = jest.requireActual('fs');
|
||||
return {
|
||||
|
|
@ -35,11 +46,8 @@ jest.mock('fs', () => {
|
|||
});
|
||||
|
||||
const fs = require('fs');
|
||||
const {
|
||||
processAgentFileUpload,
|
||||
processImageFile,
|
||||
resolvesToTextDelivery,
|
||||
} = require('~/server/services/Files/process');
|
||||
const { processAgentFileUpload, processImageFile } = require('~/server/services/Files/process');
|
||||
const { resolveEffectiveToolResource } = require('~/server/services/Files/routing');
|
||||
const { filterFile } = require('~/server/services/Files/process');
|
||||
const { UninspectableFileError } = require('@librechat/api');
|
||||
|
||||
|
|
@ -290,6 +298,45 @@ describe('POST /images - Agent Upload Permission Check (Integration)', () => {
|
|||
expect(processAgentFileUpload).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('defers extracted-text fail-close for a unified upload the config routes to text', async () => {
|
||||
/* Same policy and file as the explicit-context case above, but with no tool_resource.
|
||||
* Routing promotes it to a context resource, so the preflight has to see the same
|
||||
* downstream extraction rather than fail-closing on an uninspectable derived field. */
|
||||
await createAgent({
|
||||
id: agentCustomId,
|
||||
name: 'Test Agent',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: authorId,
|
||||
});
|
||||
resolveEffectiveToolResource.mockResolvedValueOnce('context');
|
||||
|
||||
const app = createAppWithUser(authorId, SystemRoles.USER, {
|
||||
filters: {
|
||||
files: {
|
||||
pii: {
|
||||
fields: ['extracted_text'],
|
||||
starterPatterns: [],
|
||||
customPatterns: [],
|
||||
uninspectable: 'block',
|
||||
},
|
||||
},
|
||||
},
|
||||
fileConfig: {
|
||||
ocr: { supportedMimeTypes: ['image/png'] },
|
||||
},
|
||||
ocr: {},
|
||||
});
|
||||
const response = await request(app).post('/images').send({
|
||||
endpoint: 'agents',
|
||||
agent_id: agentCustomId,
|
||||
file_id: uuidv4(),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(processAgentFileUpload).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('preserves a deferred extracted-text policy error from image processing', async () => {
|
||||
await createAgent({
|
||||
id: agentCustomId,
|
||||
|
|
@ -518,7 +565,7 @@ describe('POST /images - Agent Upload Permission Check (Integration)', () => {
|
|||
});
|
||||
|
||||
it('sends an image the config routes to text through the agent upload path', async () => {
|
||||
resolvesToTextDelivery.mockResolvedValueOnce(true);
|
||||
resolveEffectiveToolResource.mockResolvedValueOnce('context');
|
||||
const app = createAppWithUser(otherUserId);
|
||||
|
||||
const response = await request(app).post('/images').send({
|
||||
|
|
|
|||
|
|
@ -20,11 +20,14 @@ const {
|
|||
} = require('librechat-data-provider');
|
||||
const {
|
||||
processAgentFileUpload,
|
||||
resolvesToTextDelivery,
|
||||
processImageFile,
|
||||
filterFile,
|
||||
} = require('~/server/services/Files/process');
|
||||
const { resolveUploadEndpoint, resolveUploadAgent } = require('~/server/services/Files/agent');
|
||||
const {
|
||||
resolveEffectiveToolResource,
|
||||
resolveUploadEndpoint,
|
||||
resolveUploadAgent,
|
||||
} = require('~/server/services/Files/routing');
|
||||
const { checkPermission } = require('~/server/services/PermissionService');
|
||||
|
||||
const router = express.Router();
|
||||
|
|
@ -54,11 +57,17 @@ router.post('/', async (req, res) => {
|
|||
});
|
||||
filterFile({ req, image: true, endpoint: effectiveEndpoint });
|
||||
|
||||
/* A unified upload the config routes to text is processed as a context resource, so
|
||||
* the preflight has to judge that destination. Told only the request's empty tool
|
||||
* resource, it cannot see the extraction step and fail-closes on a derived field it
|
||||
* would in fact be able to inspect. */
|
||||
const effectiveToolResource = await resolveEffectiveToolResource({ req, metadata });
|
||||
|
||||
await assertUploadContentAllowed({
|
||||
filters: req.config?.filters,
|
||||
file: req.file,
|
||||
endpoint: metadata.endpoint,
|
||||
toolResource: metadata.tool_resource,
|
||||
toolResource: effectiveToolResource,
|
||||
fileConfig: mergeFileConfig(req.config?.fileConfig),
|
||||
ocrConfigured: req.config?.ocr != null,
|
||||
ragConfigured: !!process.env.RAG_API_URL,
|
||||
|
|
@ -72,8 +81,7 @@ router.post('/', async (req, res) => {
|
|||
* path, which extracts and stores the text. The image pipeline would persist the
|
||||
* routing without any text, leaving the file out of provider delivery and out of
|
||||
* the text context both. */
|
||||
const takesAgentUploadPath =
|
||||
metadata.tool_resource != null || (await resolvesToTextDelivery({ req, metadata }));
|
||||
const takesAgentUploadPath = effectiveToolResource != null;
|
||||
|
||||
if (!isAssistantsEndpoint(metadata.endpoint) && takesAgentUploadPath) {
|
||||
const denied = await verifyAgentUploadPermission({
|
||||
|
|
|
|||
|
|
@ -1,37 +0,0 @@
|
|||
const db = require('~/models');
|
||||
|
||||
/**
|
||||
* Reads the upload's agent once per request. Routing, authorization and processing each
|
||||
* need it, and this runs before any bytes are handled, so repeating the read adds a
|
||||
* round trip to every upload.
|
||||
*
|
||||
* @param {ServerRequest} req
|
||||
* @param {string} [agent_id]
|
||||
* @returns {Promise<object | null>}
|
||||
*/
|
||||
function resolveUploadAgent(req, agent_id) {
|
||||
if (!agent_id) {
|
||||
return Promise.resolve(null);
|
||||
}
|
||||
if (!req._uploadAgentCache) {
|
||||
req._uploadAgentCache = new Map();
|
||||
}
|
||||
if (!req._uploadAgentCache.has(agent_id)) {
|
||||
req._uploadAgentCache.set(agent_id, db.getAgent({ id: agent_id }));
|
||||
}
|
||||
return req._uploadAgentCache.get(agent_id);
|
||||
}
|
||||
|
||||
/** Agent uploads carry endpoint=agents; the agent's own provider governs both the file
|
||||
* configuration used for validation and the delivery-path routing. */
|
||||
async function resolveUploadEndpoint({ endpoint, agent_id, req }) {
|
||||
if (!agent_id) {
|
||||
return endpoint;
|
||||
}
|
||||
const uploadAgent = req
|
||||
? await resolveUploadAgent(req, agent_id)
|
||||
: await db.getAgent({ id: agent_id });
|
||||
return uploadAgent?.provider || endpoint;
|
||||
}
|
||||
|
||||
module.exports = { resolveUploadAgent, resolveUploadEndpoint };
|
||||
|
|
@ -17,7 +17,6 @@ const {
|
|||
removeNullishValues,
|
||||
isAssistantsEndpoint,
|
||||
getEndpointFileConfig,
|
||||
resolveDefaultLLMDeliveryPath,
|
||||
} = require('librechat-data-provider');
|
||||
const { logger, runAsSystem } = require('@librechat/data-schemas');
|
||||
const {
|
||||
|
|
@ -53,7 +52,10 @@ const { getRetentionExpiry, getAgentFileRetentionExpiry } = require('./retention
|
|||
const { getStrategyFunctions } = require('./strategies');
|
||||
const { determineFileType } = require('~/server/utils');
|
||||
const { STTService } = require('./Audio/STTService');
|
||||
const { resolveUploadEndpoint } = require('~/server/services/Files/agent');
|
||||
const {
|
||||
resolveUploadEndpoint,
|
||||
resolveUploadLLMDeliveryPath,
|
||||
} = require('~/server/services/Files/routing');
|
||||
const db = require('~/models');
|
||||
|
||||
/**
|
||||
|
|
@ -450,20 +452,6 @@ const processFileURL = async ({
|
|||
}
|
||||
};
|
||||
|
||||
const resolveDefaultUploadLLMDeliveryPath = ({ file, endpointConfig, fileConfig, endpoint }) => {
|
||||
const isLegacyFileUploadUX = endpointConfig?.legacyFileUploadUX === true;
|
||||
if (isLegacyFileUploadUX) {
|
||||
return 'provider';
|
||||
}
|
||||
|
||||
return resolveDefaultLLMDeliveryPath(
|
||||
file.mimetype,
|
||||
endpointConfig?.defaultLLMDeliveryPath,
|
||||
fileConfig?.defaultLLMDeliveryPath,
|
||||
endpoint,
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Applies the current strategy for image uploads.
|
||||
* Saves file metadata to the database with an expiry TTL.
|
||||
|
|
@ -485,7 +473,7 @@ const processImageFile = async ({ req, res, metadata, returnFile = false, sseStr
|
|||
const fileConfig = mergeFileConfig(appConfig?.fileConfig);
|
||||
const configEndpoint = await resolveUploadEndpoint({ endpoint, agent_id, req });
|
||||
const endpointConfig = getEndpointFileConfig({ fileConfig, endpoint: configEndpoint });
|
||||
const llmDeliveryPath = resolveDefaultUploadLLMDeliveryPath({
|
||||
const llmDeliveryPath = resolveUploadLLMDeliveryPath({
|
||||
file,
|
||||
endpointConfig,
|
||||
fileConfig,
|
||||
|
|
@ -690,56 +678,6 @@ const processFileUpload = async ({ req, res, metadata, sseStream }) => {
|
|||
sendUploadSuccess(res, sseStream, 'File uploaded and processed successfully', result);
|
||||
};
|
||||
|
||||
const resolveUploadLLMDeliveryPath = ({
|
||||
tool_resource,
|
||||
file,
|
||||
endpointConfig,
|
||||
fileConfig,
|
||||
endpoint,
|
||||
}) => {
|
||||
if (tool_resource === EToolResources.context || tool_resource === EToolResources.ocr) {
|
||||
return 'text';
|
||||
}
|
||||
|
||||
if (
|
||||
tool_resource === EToolResources.file_search ||
|
||||
tool_resource === EToolResources.execute_code
|
||||
) {
|
||||
return 'none';
|
||||
}
|
||||
|
||||
return resolveDefaultUploadLLMDeliveryPath({ file, endpointConfig, fileConfig, endpoint });
|
||||
};
|
||||
|
||||
/**
|
||||
* Whether an image upload with no explicit tool resource is routed to text delivery.
|
||||
* The image pipeline stores pixels and never extracts text, so such an upload has to
|
||||
* take the agent upload path or it reaches neither the model nor a text context.
|
||||
*
|
||||
* @param {Object} params
|
||||
* @param {ServerRequest} params.req
|
||||
* @param {Object} params.metadata
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
const resolvesToTextDelivery = async ({ req, metadata }) => {
|
||||
const fileConfig = mergeFileConfig(req.config?.fileConfig);
|
||||
const endpoint = await resolveUploadEndpoint({
|
||||
endpoint: metadata.endpoint,
|
||||
agent_id: metadata.agent_id,
|
||||
req,
|
||||
});
|
||||
const endpointConfig = getEndpointFileConfig({ fileConfig, endpoint });
|
||||
return (
|
||||
resolveUploadLLMDeliveryPath({
|
||||
tool_resource: metadata.tool_resource,
|
||||
file: req.file,
|
||||
endpointConfig,
|
||||
fileConfig,
|
||||
endpoint,
|
||||
}) === 'text'
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Applies the current strategy for file uploads.
|
||||
* Saves file metadata to the database with an expiry TTL.
|
||||
|
|
@ -1550,7 +1488,6 @@ function filterFile({ req, image, isAvatar, endpoint: endpointOverride }) {
|
|||
|
||||
module.exports = {
|
||||
filterFile,
|
||||
resolvesToTextDelivery,
|
||||
processFileURL,
|
||||
saveBase64Image,
|
||||
processImageFile,
|
||||
|
|
|
|||
133
api/server/services/Files/routing.js
Normal file
133
api/server/services/Files/routing.js
Normal file
|
|
@ -0,0 +1,133 @@
|
|||
const {
|
||||
EToolResources,
|
||||
mergeFileConfig,
|
||||
resolveDefaultLLMDeliveryPath,
|
||||
getEndpointFileConfig,
|
||||
} = require('librechat-data-provider');
|
||||
const db = require('~/models');
|
||||
|
||||
/**
|
||||
* Reads the upload's agent once per request. Routing, authorization and processing each
|
||||
* need it, and this runs before any bytes are handled, so repeating the read adds a
|
||||
* round trip to every upload.
|
||||
*
|
||||
* @param {ServerRequest} req
|
||||
* @param {string} [agent_id]
|
||||
* @returns {Promise<object | null>}
|
||||
*/
|
||||
function resolveUploadAgent(req, agent_id) {
|
||||
if (!agent_id) {
|
||||
return Promise.resolve(null);
|
||||
}
|
||||
if (!req._uploadAgentCache) {
|
||||
req._uploadAgentCache = new Map();
|
||||
}
|
||||
if (!req._uploadAgentCache.has(agent_id)) {
|
||||
req._uploadAgentCache.set(agent_id, db.getAgent({ id: agent_id }));
|
||||
}
|
||||
return req._uploadAgentCache.get(agent_id);
|
||||
}
|
||||
|
||||
/** Agent uploads carry endpoint=agents; the agent's own provider governs both the file
|
||||
* configuration used for validation and the delivery-path routing. */
|
||||
async function resolveUploadEndpoint({ endpoint, agent_id, req }) {
|
||||
if (!agent_id) {
|
||||
return endpoint;
|
||||
}
|
||||
const uploadAgent = req
|
||||
? await resolveUploadAgent(req, agent_id)
|
||||
: await db.getAgent({ id: agent_id });
|
||||
return uploadAgent?.provider || endpoint;
|
||||
}
|
||||
|
||||
const resolveDefaultUploadLLMDeliveryPath = ({ file, endpointConfig, fileConfig, endpoint }) => {
|
||||
const isLegacyFileUploadUX = endpointConfig?.legacyFileUploadUX === true;
|
||||
if (isLegacyFileUploadUX) {
|
||||
return 'provider';
|
||||
}
|
||||
|
||||
return resolveDefaultLLMDeliveryPath(
|
||||
file.mimetype,
|
||||
endpointConfig?.defaultLLMDeliveryPath,
|
||||
fileConfig?.defaultLLMDeliveryPath,
|
||||
endpoint,
|
||||
);
|
||||
};
|
||||
|
||||
const resolveUploadLLMDeliveryPath = ({
|
||||
tool_resource,
|
||||
file,
|
||||
endpointConfig,
|
||||
fileConfig,
|
||||
endpoint,
|
||||
}) => {
|
||||
if (tool_resource === EToolResources.context || tool_resource === EToolResources.ocr) {
|
||||
return 'text';
|
||||
}
|
||||
|
||||
if (
|
||||
tool_resource === EToolResources.file_search ||
|
||||
tool_resource === EToolResources.execute_code
|
||||
) {
|
||||
return 'none';
|
||||
}
|
||||
|
||||
return resolveDefaultUploadLLMDeliveryPath({ file, endpointConfig, fileConfig, endpoint });
|
||||
};
|
||||
|
||||
/**
|
||||
* Whether an image upload with no explicit tool resource is routed to text delivery.
|
||||
* The image pipeline stores pixels and never extracts text, so such an upload has to
|
||||
* take the agent upload path or it reaches neither the model nor a text context.
|
||||
*
|
||||
* @param {Object} params
|
||||
* @param {ServerRequest} params.req
|
||||
* @param {Object} params.metadata
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
const resolvesToTextDelivery = async ({ req, metadata }) => {
|
||||
const fileConfig = mergeFileConfig(req.config?.fileConfig);
|
||||
const endpoint = await resolveUploadEndpoint({
|
||||
endpoint: metadata.endpoint,
|
||||
agent_id: metadata.agent_id,
|
||||
req,
|
||||
});
|
||||
const endpointConfig = getEndpointFileConfig({ fileConfig, endpoint });
|
||||
return (
|
||||
resolveUploadLLMDeliveryPath({
|
||||
tool_resource: metadata.tool_resource,
|
||||
file: req.file,
|
||||
endpointConfig,
|
||||
fileConfig,
|
||||
endpoint,
|
||||
}) === 'text'
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* The destination this upload will actually be processed under. Unified uploads carry no
|
||||
* tool resource but are promoted to a text context when routing sends them there, and the
|
||||
* content preflight has to judge the same destination the processing path will use.
|
||||
*
|
||||
* @param {object} params
|
||||
* @param {ServerRequest} params.req
|
||||
* @param {object} params.metadata
|
||||
* @returns {Promise<string | undefined>}
|
||||
*/
|
||||
async function resolveEffectiveToolResource({ req, metadata }) {
|
||||
if (metadata.tool_resource === EToolResources.ocr) {
|
||||
return EToolResources.context;
|
||||
}
|
||||
if (metadata.tool_resource) {
|
||||
return metadata.tool_resource;
|
||||
}
|
||||
return (await resolvesToTextDelivery({ req, metadata })) ? EToolResources.context : undefined;
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
resolveUploadAgent,
|
||||
resolveUploadEndpoint,
|
||||
resolveUploadLLMDeliveryPath,
|
||||
resolveEffectiveToolResource,
|
||||
resolvesToTextDelivery,
|
||||
};
|
||||
Loading…
Add table
Add a link
Reference in a new issue