🧷 fix: Align Agent File Attachment Ownership (#14149)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled

* fix: Align agent file attachment ownership

* fix: Harden agent file unlink validation

* test: Align file preview agent attachment access

* test: Add agent file ownership e2e regression
This commit is contained in:
Danny Avila 2026-07-07 16:23:48 -04:00 committed by GitHub
parent 0347d4a7dc
commit 96367828e1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 792 additions and 158 deletions

View file

@ -289,36 +289,45 @@ const filterAuthorizedTools = async ({
};
/**
* Removes file IDs from tool resources unless the referenced file is owned by
* the agent owner.
* Removes file IDs from tool resources unless they are already attached to the
* agent or owned by an allowed uploader.
* @param {object} params
* @param {object} params.tool_resources
* @param {string | object} params.ownerId
* @param {string | object | Array<string | object>} params.ownerIds
* @param {object} [params.existingToolResources]
* @param {string} params.logPrefix
* @returns {Promise<number>} Count of removed file references.
*/
const pruneToolResourceFileIdsForOwner = async ({ tool_resources, ownerId, logPrefix }) => {
const pruneToolResourceFileIdsForAgent = async ({
tool_resources,
ownerIds,
existingToolResources,
logPrefix,
}) => {
const referencedFileIds = collectToolResourceFileIds(tool_resources);
if (referencedFileIds.length === 0) {
return 0;
}
if (!ownerId) {
return stripFileIdsFromToolResources(tool_resources, referencedFileIds).removedCount;
}
const ownerIdStr = ownerId.toString();
const ownerIdSet = new Set(
(Array.isArray(ownerIds) ? ownerIds : [ownerIds])
.filter(Boolean)
.map((ownerId) => ownerId.toString()),
);
const existingFileIds = new Set(collectToolResourceFileIds(existingToolResources ?? {}));
try {
const ownerFiles = await db.getFiles(
{ file_id: { $in: referencedFileIds }, user: ownerIdStr },
null,
{
file_id: 1,
user: 1,
},
);
const files = await db.getFiles({ file_id: { $in: referencedFileIds } }, null, {
file_id: 1,
user: 1,
});
const allowedIds = new Set(
(ownerFiles ?? [])
.filter((file) => file.user && file.user.toString() === ownerIdStr)
(files ?? [])
.filter((file) => {
if (!file.user) {
return false;
}
return existingFileIds.has(file.file_id) || ownerIdSet.has(file.user.toString());
})
.map((file) => file.file_id),
);
const disallowedIds = referencedFileIds.filter((id) => !allowedIds.has(id));
@ -358,9 +367,9 @@ const createAgentHandler = async (req, res) => {
const { id: userId, role: userRole } = req.user;
if (agentData.tool_resources) {
await pruneToolResourceFileIdsForOwner({
await pruneToolResourceFileIdsForAgent({
tool_resources: agentData.tool_resources,
ownerId: userId,
ownerIds: userId,
logPrefix: '[/Agents]',
});
}
@ -680,9 +689,10 @@ const updateAgentHandler = async (req, res) => {
}
if (updateData.tool_resources) {
await pruneToolResourceFileIdsForOwner({
await pruneToolResourceFileIdsForAgent({
tool_resources: updateData.tool_resources,
ownerId: existingAgent.author,
ownerIds: req.user.id,
existingToolResources: existingAgent.tool_resources,
logPrefix: `[/Agents/:id] Agent ${id}`,
});
}
@ -904,9 +914,9 @@ const duplicateAgentHandler = async (req, res) => {
}
if (newAgentData.tool_resources) {
await pruneToolResourceFileIdsForOwner({
await pruneToolResourceFileIdsForAgent({
tool_resources: newAgentData.tool_resources,
ownerId: userId,
ownerIds: userId,
logPrefix: '[/Agents/:id/duplicate]',
});
}
@ -1287,9 +1297,10 @@ const revertAgentVersionHandler = async (req, res) => {
}
if (updatedAgent.tool_resources) {
const removedCount = await pruneToolResourceFileIdsForOwner({
const removedCount = await pruneToolResourceFileIdsForAgent({
tool_resources: updatedAgent.tool_resources,
ownerId: existingAgent.author,
ownerIds: req.user.id,
existingToolResources: updatedAgent.tool_resources,
logPrefix: '[/Agents/:id/revert]',
});
if (removedCount > 0) {

View file

@ -807,7 +807,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
expect(updatedAgent.name).toBe('Admin Update');
});
test('should prune admin-supplied file_ids against the agent author', async () => {
test('should allow an editor to add their own file but not another user file', async () => {
const File = mongoose.models.File;
const adminUserId = new mongoose.Types.ObjectId().toString();
const authorFileId = `file_${uuidv4()}`;
@ -846,7 +846,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
await updateAgentHandler(mockReq, mockRes);
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([authorFileId]);
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([adminFileId]);
});
test('should validate tool_resources in updates', async () => {
@ -1153,6 +1153,31 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([keeper]);
});
test('preserves existing attached file_ids owned by another user', async () => {
const authorFile = `file_${uuidv4()}`;
const editorFile = `file_${uuidv4()}`;
const editorId = new mongoose.Types.ObjectId();
await createFileDoc(authorFile, existingAgentAuthorId);
await createFileDoc(editorFile, editorId);
await Agent.updateOne(
{ id: existingAgentId },
{ $set: { tool_resources: { file_search: { file_ids: [editorFile] } } } },
);
mockReq.user.id = existingAgentAuthorId.toString();
mockReq.params.id = existingAgentId;
mockReq.body = {
tool_resources: {
file_search: { file_ids: [authorFile, editorFile] },
},
};
await updateAgentHandler(mockReq, mockRes);
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([authorFile, editorFile]);
});
});
});
@ -1202,11 +1227,12 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
expect(agent.tool_resources.context.file_ids).toEqual([cloneAuthorFileId]);
});
test('revertAgentVersionHandler should prune restored file_ids not owned by the agent author', async () => {
test('revertAgentVersionHandler should preserve restored attached file_ids with metadata', async () => {
const agentAuthorId = new mongoose.Types.ObjectId();
const otherUserId = new mongoose.Types.ObjectId();
const ownedFileId = `file_${uuidv4()}`;
const otherFileId = `file_${uuidv4()}`;
const orphanFileId = `file_${uuidv4()}`;
await createFileDoc(ownedFileId, agentAuthorId);
await createFileDoc(otherFileId, otherUserId);
@ -1223,7 +1249,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
provider: 'openai',
model: 'gpt-4',
tool_resources: {
file_search: { file_ids: [ownedFileId, otherFileId] },
file_search: { file_ids: [ownedFileId, otherFileId, orphanFileId] },
},
},
],
@ -1237,7 +1263,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
expect(mockRes.json).toHaveBeenCalled();
const agentInDb = await Agent.findOne({ id: agent.id }).lean();
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([ownedFileId]);
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([ownedFileId, otherFileId]);
});
});

View file

@ -5,10 +5,15 @@ const { getAgents, getFiles } = require('~/models');
/**
* Checks if user has access to a file through agent permissions
* Files inherit permissions from agents authored by the file owner
* Files inherit permissions from agents they are attached to.
*/
const checkAgentBasedFileAccess = async ({ userId, role, fileId, fileOwner }) => {
try {
const fileOwnerId = fileOwner?.toString();
if (!fileOwnerId) {
return false;
}
/** Agents that have this file in their tool_resources */
const agentsWithFile = await getAgents({
$or: [
@ -24,11 +29,10 @@ const checkAgentBasedFileAccess = async ({ userId, role, fileId, fileOwner }) =>
return false;
}
const fileOwnerId = fileOwner?.toString();
const userIdStr = userId.toString();
for (const agent of agentsWithFile) {
const agentAuthorId = agent.author?.toString();
if (!agentAuthorId || !fileOwnerId || agentAuthorId !== fileOwnerId) {
if (!agentAuthorId) {
continue;
}
@ -74,7 +78,7 @@ const denyFileAccess = (res) =>
/**
* Middleware to check if user can access a file
* Checks: 1) File ownership, 2) Agent-based access through a file-owner agent
* Checks: 1) File ownership, 2) Agent-based access through attached agents
*/
const fileAccess = async (req, res, next) => {
try {

View file

@ -212,7 +212,7 @@ describe('fileAccess middleware', () => {
});
});
test('should deny access when user authored an agent with another user file id', async () => {
test('should allow access when user authored an agent with an attached file from another owner', async () => {
await createAgent({
id: `agent_${Date.now()}`,
name: 'Test Agent',
@ -229,8 +229,8 @@ describe('fileAccess middleware', () => {
req.params.file_id = 'shared_file_via_agent';
await fileAccess(req, res, next);
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(403);
expect(next).toHaveBeenCalled();
expect(req.fileAccess).toBeDefined();
});
test('should allow access when user has VIEW permission on agent with file', async () => {
@ -478,7 +478,7 @@ describe('fileAccess middleware', () => {
await fileAccess(req, res, next);
/**
* Should succeed because testUser has access to the file owner's agent,
* Should succeed because testUser has access to an attached agent,
* even though a non-owner agent without access is found first.
*/
expect(next).toHaveBeenCalled();
@ -535,7 +535,7 @@ describe('fileAccess middleware', () => {
grantedBy: otherUser._id,
});
// Agent 3: same file in ocr (bad reference from a non-owner agent)
// Agent 3: same file in ocr on the requesting user's own agent
await createAgent({
id: 'agent_ocr',
name: 'OCR Agent',
@ -553,7 +553,7 @@ describe('fileAccess middleware', () => {
await fileAccess(req, res, next);
/**
* Should succeed through the file owner's execute_code agent,
* Should succeed through an attached agent,
* even if other agents with the file are found first.
*/
expect(next).toHaveBeenCalled();

View file

@ -223,7 +223,7 @@ describe('File Routes - Agent Files Endpoint', () => {
author: authorId,
tool_resources: {
file_search: {
file_ids: [fileId1, fileId2],
file_ids: [fileId1, fileId2, fileId3],
},
},
});
@ -249,9 +249,10 @@ describe('File Routes - Agent Files Endpoint', () => {
expect(response.status).toBe(200);
expect(Array.isArray(response.body)).toBe(true);
expect(response.body).toHaveLength(2);
expect(response.body).toHaveLength(3);
expect(response.body.map((f) => f.file_id)).toContain(fileId1);
expect(response.body.map((f) => f.file_id)).toContain(fileId2);
expect(response.body.map((f) => f.file_id)).toContain(fileId3);
});
it('should return 400 when agent_id is not provided', async () => {
@ -333,27 +334,8 @@ describe('File Routes - Agent Files Endpoint', () => {
expect(response.body).toHaveLength(2);
});
it('should not return files owned by other users through agent file references', async () => {
const anotherUserId = new mongoose.Types.ObjectId();
const otherUserFileId = uuidv4();
await User.create({
_id: anotherUserId,
username: 'another',
email: 'another@test.com',
});
await createFile({
user: anotherUserId,
file_id: otherUserFileId,
filename: 'other-user-file.txt',
filepath: '/uploads/other-user-file.txt',
bytes: 400,
type: 'text/plain',
});
// Create agent to include the file uploaded by another user
await createAgent({
it('should return attached files uploaded by another editor', async () => {
const agent = await createAgent({
id: agentId,
name: 'Test Agent',
provider: 'openai',
@ -361,11 +343,21 @@ describe('File Routes - Agent Files Endpoint', () => {
author: authorId,
tool_resources: {
file_search: {
file_ids: [fileId1, otherUserFileId],
file_ids: [fileId1, fileId3],
},
},
});
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,
});
// Create a new app instance with author authentication
const authorApp = express();
authorApp.use(express.json());
@ -380,9 +372,9 @@ describe('File Routes - Agent Files Endpoint', () => {
expect(response.status).toBe(200);
expect(Array.isArray(response.body)).toBe(true);
expect(response.body).toHaveLength(1);
expect(response.body).toHaveLength(2);
expect(response.body.map((f) => f.file_id)).toContain(fileId1);
expect(response.body.map((f) => f.file_id)).not.toContain(otherUserFileId);
expect(response.body.map((f) => f.file_id)).toContain(fileId3);
});
});

View file

@ -14,6 +14,7 @@ const {
FileSources,
ResourceType,
EModelEndpoint,
EToolResources,
PermissionBits,
checkOpenAIStorage,
isAssistantsEndpoint,
@ -29,13 +30,22 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies');
const { getOpenAIClient } = require('~/server/controllers/assistants/helpers');
const { hasCapability } = require('~/server/middleware/roles/capabilities');
const { checkPermission } = require('~/server/services/PermissionService');
const { hasAccessToFilesViaAgent } = require('~/server/services/Files');
const { cleanFileName, getContentDisposition } = require('~/server/utils/files');
const { getLogStores } = require('~/cache');
const { Readable } = require('stream');
const db = require('~/models');
const router = express.Router();
const AGENT_TOOL_RESOURCE_KEYS = new Set([
EToolResources.execute_code,
EToolResources.file_search,
EToolResources.image_edit,
EToolResources.context,
EToolResources.ocr,
]);
const isAgentToolResourceKey = (toolResource) =>
typeof toolResource === 'string' && AGENT_TOOL_RESOURCE_KEYS.has(toolResource);
router.get('/', async (req, res) => {
try {
@ -94,20 +104,20 @@ router.get('/agent/:agent_id', async (req, res) => {
}
}
const agentFileIds = [];
const agentFileIds = new Set();
if (agent.tool_resources) {
for (const [, resource] of Object.entries(agent.tool_resources)) {
if (resource?.file_ids && Array.isArray(resource.file_ids)) {
agentFileIds.push(...resource.file_ids);
resource.file_ids.forEach((fileId) => agentFileIds.add(fileId));
}
}
}
if (agentFileIds.length === 0) {
if (agentFileIds.size === 0) {
return res.status(200).json([]);
}
const files = await db.getFiles({ file_id: { $in: agentFileIds }, user: agent.author }, null, {
const files = await db.getFiles({ file_id: { $in: [...agentFileIds] } }, null, {
text: 0,
});
@ -156,6 +166,52 @@ router.delete('/', async (req, res) => {
const fileIds = files.map((file) => file.file_id);
const dbFiles = await db.getFiles({ file_id: { $in: fileIds } });
if (req.body.agent_id && req.body.tool_resource) {
if (!isAgentToolResourceKey(req.body.tool_resource)) {
return res.status(400).json({ message: 'Invalid agent tool resource' });
}
const agent = await db.getAgent({
id: req.body.agent_id,
});
if (!agent) {
return res.status(404).json({ message: 'Agent not found' });
}
const hasAgentEditAccess =
agent.author?.toString() === req.user.id.toString() ||
(await checkPermission({
userId: req.user.id,
role: req.user.role,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
requiredPermission: PermissionBits.EDIT,
}));
if (!hasAgentEditAccess) {
return res.status(403).json({
message: 'You can only delete files you have access to',
unauthorizedFiles: files.map((file) => file.file_id),
});
}
const toolResourceFiles = agent.tool_resources?.[req.body.tool_resource]?.file_ids ?? [];
const agentFiles = files
.filter((f) => toolResourceFiles.includes(f.file_id))
.map((file) => ({ tool_resource: req.body.tool_resource, file_id: file.file_id }));
if (agentFiles.length === 0) {
res.status(200).json({ message: 'File associations removed successfully from agent' });
return;
}
await db.removeAgentResourceFiles({
agent_id: req.body.agent_id,
files: agentFiles,
});
res.status(200).json({ message: 'File associations removed successfully from agent' });
return;
}
const ownedFiles = [];
const nonOwnedFiles = [];
@ -179,73 +235,16 @@ router.delete('/', async (req, res) => {
return;
}
let authorizedFiles = [...ownedFiles];
let unauthorizedFiles = [];
if (req.body.agent_id && nonOwnedFiles.length > 0) {
const nonOwnedFileIds = nonOwnedFiles.map((f) => f.file_id);
const accessMap = await hasAccessToFilesViaAgent({
userId: req.user.id,
role: req.user.role,
fileIds: nonOwnedFileIds,
agentId: req.body.agent_id,
isDelete: true,
files: nonOwnedFiles,
});
for (const file of nonOwnedFiles) {
if (accessMap.get(file.file_id)) {
authorizedFiles.push(file);
} else {
unauthorizedFiles.push(file);
}
}
} else {
unauthorizedFiles = nonOwnedFiles;
}
const authorizedFiles = [...ownedFiles];
const unauthorizedFiles = nonOwnedFiles;
if (unauthorizedFiles.length > 0) {
return res.status(403).json({
message: 'You can only delete files you have access to',
message: 'You can only delete files you own',
unauthorizedFiles: unauthorizedFiles.map((f) => f.file_id),
});
}
/* Handle agent unlinking even if no valid files to delete */
if (req.body.agent_id && req.body.tool_resource && dbFiles.length === 0) {
const agent = await db.getAgent({
id: req.body.agent_id,
});
const toolResourceFiles = agent.tool_resources?.[req.body.tool_resource]?.file_ids ?? [];
const agentFiles = files
.filter((f) => toolResourceFiles.includes(f.file_id))
.map((file) => ({ tool_resource: req.body.tool_resource, file_id: file.file_id }));
const hasAgentEditAccess =
agent.author?.toString() === req.user.id.toString() ||
(await checkPermission({
userId: req.user.id,
role: req.user.role,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
requiredPermission: PermissionBits.EDIT,
}));
const unauthorizedFiles = hasAgentEditAccess ? [] : agentFiles;
if (unauthorizedFiles.length > 0) {
return res.status(403).json({
message: 'You can only delete files you have access to',
unauthorizedFiles: unauthorizedFiles.map((file) => file.file_id),
});
}
await db.removeAgentResourceFiles({
agent_id: req.body.agent_id,
files: agentFiles,
});
res.status(200).json({ message: 'File associations removed successfully from agent' });
return;
}
/* Handle assistant unlinking even if no valid files to delete */
if (req.body.assistant_id && req.body.tool_resource && dbFiles.length === 0) {
const assistant = await db.getAssistant({

View file

@ -222,13 +222,12 @@ describe('File Routes - Delete with Agent Access', () => {
});
expect(response.status).toBe(403);
expect(response.body.message).toBe('You can only delete files you have access to');
expect(response.body.message).toBe('You can only delete files you own');
expect(response.body.unauthorizedFiles).toContain(fileId);
expect(processDeleteRequest).not.toHaveBeenCalled();
});
it('should allow deleting files accessible through shared agent', async () => {
// Create an agent with the file attached
it('should prevent physically deleting non-owned files accessible through shared agent', async () => {
const agent = await createAgent({
id: uuidv4(),
name: 'Test Agent',
@ -242,7 +241,6 @@ describe('File Routes - Delete with Agent Access', () => {
},
});
// Grant EDIT permission to user on the agent
const { grantPermission } = require('~/server/services/PermissionService');
await grantPermission({
principalType: PrincipalType.USER,
@ -265,12 +263,143 @@ describe('File Routes - Delete with Agent Access', () => {
],
});
expect(response.status).toBe(200);
expect(response.body.message).toBe('Files deleted successfully');
expect(processDeleteRequest).toHaveBeenCalled();
expect(response.status).toBe(403);
expect(response.body.message).toBe('You can only delete files you own');
expect(response.body.unauthorizedFiles).toContain(fileId);
expect(processDeleteRequest).not.toHaveBeenCalled();
});
it('should prevent deleting files not owned by the agent author', async () => {
it('unlinks attached agent files without invoking storage deletion', async () => {
const agent = await createAgent({
id: uuidv4(),
name: 'Test Agent',
provider: 'openai',
model: 'gpt-4',
author: authorId,
tool_resources: {
file_search: {
file_ids: [fileId],
},
},
});
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 response = await request(app)
.delete('/files')
.send({
agent_id: agent.id,
tool_resource: 'file_search',
files: [
{
file_id: fileId,
filepath: '/uploads/test.txt',
},
],
});
expect(response.status).toBe(200);
expect(response.body.message).toBe('File associations removed successfully from agent');
expect(processDeleteRequest).not.toHaveBeenCalled();
const updatedAgent = await Agent.findOne({ id: agent.id }).lean();
expect(updatedAgent.tool_resources.file_search.file_ids).toEqual([]);
});
it('rejects invalid agent tool_resource values before unlinking', async () => {
const agent = await createAgent({
id: uuidv4(),
name: 'Test Agent',
provider: 'openai',
model: 'gpt-4',
author: otherUserId,
tool_resources: {
file_search: {
file_ids: [fileId],
},
},
});
const response = await request(app)
.delete('/files')
.send({
agent_id: agent.id,
tool_resource: 'file_search.$pullAll',
files: [{ file_id: fileId, filepath: '/uploads/test.txt' }],
});
expect(response.status).toBe(400);
expect(response.body.message).toBe('Invalid agent tool resource');
expect(processDeleteRequest).not.toHaveBeenCalled();
const updatedAgent = await Agent.findOne({ id: agent.id }).lean();
expect(updatedAgent.tool_resources.file_search.file_ids).toEqual([fileId]);
});
it('allows an agent author to unlink an editor-owned attached file', async () => {
const editorFileId = uuidv4();
await createFile({
user: otherUserId,
file_id: editorFileId,
filename: 'editor-file.txt',
filepath: '/uploads/editor-file.txt',
bytes: 300,
type: 'text/plain',
});
const agent = await createAgent({
id: uuidv4(),
name: 'Test Agent',
provider: 'openai',
model: 'gpt-4',
author: authorId,
tool_resources: {
file_search: {
file_ids: [editorFileId],
},
},
});
const authorApp = express();
authorApp.use(express.json());
authorApp.use((req, res, next) => {
req.user = {
id: authorId.toString(),
role: SystemRoles.USER,
};
req.app.locals = {};
next();
});
authorApp.use('/files', router);
const response = await request(authorApp)
.delete('/files')
.send({
agent_id: agent.id,
tool_resource: 'file_search',
files: [{ file_id: editorFileId, filepath: '/uploads/editor-file.txt' }],
});
expect(response.status).toBe(200);
expect(response.body.message).toBe('File associations removed successfully from agent');
expect(processDeleteRequest).not.toHaveBeenCalled();
const updatedAgent = await Agent.findOne({ id: agent.id }).lean();
expect(updatedAgent.tool_resources.file_search.file_ids).toEqual([]);
const retainedFile = await File.findOne({ file_id: editorFileId }).lean();
expect(retainedFile).toBeTruthy();
});
it('should prevent physically deleting attached files owned by another user', async () => {
const thirdUserId = new mongoose.Types.ObjectId();
const thirdUserFileId = uuidv4();
await createFile({
@ -318,12 +447,12 @@ describe('File Routes - Delete with Agent Access', () => {
});
expect(response.status).toBe(403);
expect(response.body.message).toBe('You can only delete files you have access to');
expect(response.body.message).toBe('You can only delete files you own');
expect(response.body.unauthorizedFiles).toContain(thirdUserFileId);
expect(processDeleteRequest).not.toHaveBeenCalled();
});
it('should prevent deleting files not attached to the specified agent', async () => {
it('should prevent physically deleting non-owned files not attached to the specified agent', async () => {
// Create another file not attached to the agent
const unattachedFileId = uuidv4();
await createFile({
@ -373,7 +502,7 @@ describe('File Routes - Delete with Agent Access', () => {
});
expect(response.status).toBe(403);
expect(response.body.message).toBe('You can only delete files you have access to');
expect(response.body.message).toBe('You can only delete files you own');
expect(response.body.unauthorizedFiles).toContain(unattachedFileId);
expect(processDeleteRequest).not.toHaveBeenCalled();
});
@ -438,12 +567,12 @@ describe('File Routes - Delete with Agent Access', () => {
});
expect(response.status).toBe(403);
expect(response.body.message).toBe('You can only delete files you have access to');
expect(response.body.message).toBe('You can only delete files you own');
expect(response.body.unauthorizedFiles).toContain(unauthorizedFileId);
expect(processDeleteRequest).not.toHaveBeenCalled();
});
it('should prevent deleting files when user lacks EDIT permission on agent', async () => {
it('should prevent unlinking attached files when user lacks EDIT permission on agent', async () => {
// Create an agent with the file attached
const agent = await createAgent({
id: uuidv4(),
@ -473,6 +602,7 @@ describe('File Routes - Delete with Agent Access', () => {
.delete('/files')
.send({
agent_id: agent.id,
tool_resource: 'file_search',
files: [
{
file_id: fileId,

View file

@ -124,7 +124,7 @@ describe('GET /files/:file_id/preview', () => {
expect(mockFindFileById).not.toHaveBeenCalled();
});
it('does not disclose preview text through an attacker-authored agent file reference', async () => {
it('allows preview text through an attached agent file reference', async () => {
mockGetFiles.mockResolvedValueOnce([
{ file_id: 'victim-file', user: 'victim-user', filename: 'secret.xlsx', status: 'ready' },
]);
@ -135,13 +135,23 @@ describe('GET /files/:file_id/preview', () => {
tool_resources: { execute_code: { file_ids: ['victim-file'] } },
},
]);
mockFindFileById.mockResolvedValueOnce({
file_id: 'victim-file',
text: 'shared secret',
textFormat: 'text',
});
const res = await request(buildApp({ user: { id: 'attacker-user', role: 'user' } })).get(
'/files/victim-file/preview',
);
expect(res.status).toBe(403);
expect(mockFindFileById).not.toHaveBeenCalled();
expect(res.status).toBe(200);
expect(res.body).toEqual({
file_id: 'victim-file',
status: 'ready',
text: 'shared secret',
textFormat: 'text',
});
});
it('returns status:pending without text/textFormat while the deferred render is in flight', async () => {

View file

@ -33,7 +33,7 @@ function getFilesById(files) {
/**
* Checks if a user has access to multiple files through a shared agent (batch operation).
* Access is scoped to files attached to the agent and owned by the agent author.
* Access is scoped to files attached to the agent with valid file metadata.
* @param {Object} params - Parameters object
* @param {string} params.userId - The user ID to check access for
* @param {string} [params.role] - Optional user role to avoid DB query
@ -64,7 +64,7 @@ const hasAccessToFilesViaAgent = async ({ userId, role, fileIds, agentId, isDele
await getFiles({ file_id: { $in: fileIds } }, null, { file_id: 1, user: 1 }),
);
const canInheritFromAgent = (fileId) =>
attachedFileIds.has(fileId) && filesById.get(fileId)?.user?.toString() === agentAuthorId;
attachedFileIds.has(fileId) && filesById.get(fileId)?.user != null;
if (agentAuthorId === userId.toString()) {
fileIds.forEach((fileId) => {

View file

@ -158,7 +158,7 @@ describe('filterFilesByAgentAccess', () => {
expect(result.map((f) => f.file_id)).not.toContain('not-attached');
});
it('should not return a file referenced from an agent that is not authored by the file owner', async () => {
it('should return an attached file even when the file owner is not the agent author', async () => {
getAgent.mockResolvedValue(makeAgent({ author: USER_ID }));
checkPermission.mockResolvedValue(true);
@ -169,7 +169,7 @@ describe('filterFilesByAgentAccess', () => {
agentId: AGENT_ID,
});
expect(result).toEqual([]);
expect(result).toEqual([sharedFile]);
});
it('should return only owned files when user lacks VIEW permission', async () => {
@ -321,7 +321,7 @@ describe('hasAccessToFilesViaAgent', () => {
expect(checkPermission).not.toHaveBeenCalled();
});
it('should deny attached files not owned by the agent author', async () => {
it('should grant attached files not owned by the agent author', async () => {
getAgent.mockResolvedValue(makeAgent({ author: USER_ID }));
getFiles.mockResolvedValue([makeFile('attached-1', AUTHOR_ID)]);
@ -331,7 +331,7 @@ describe('hasAccessToFilesViaAgent', () => {
agentId: AGENT_ID,
});
expect(result.get('attached-1')).toBe(false);
expect(result.get('attached-1')).toBe(true);
});
});
@ -360,6 +360,22 @@ describe('hasAccessToFilesViaAgent', () => {
});
});
it('should grant access to attached files owned by another collaborator', async () => {
const collaboratorFile = makeFile('attached-2', 'editor-user-id');
getAgent.mockResolvedValue(makeAgent());
checkPermission.mockResolvedValue(true);
const result = await hasAccessToFilesViaAgent({
userId: USER_ID,
role: 'USER',
fileIds: ['attached-2'],
agentId: AGENT_ID,
files: [collaboratorFile],
});
expect(result.get('attached-2')).toBe(true);
});
it('should deny all when VIEW permission is missing', async () => {
getAgent.mockResolvedValue(makeAgent());
checkPermission.mockResolvedValue(false);

View file

@ -20,6 +20,7 @@ const CREATE_SKILL_MARKER = 'E2E_CREATE_SKILL:';
const EDIT_SKILL_MARKER = 'E2E_EDIT_SKILL:';
const ASSERT_MODEL_SPEC_SKILLS_MARKER = 'E2E_ASSERT_MODEL_SPEC_SKILLS';
const ASSERT_PROVIDER_FILE_MARKER = 'E2E_ASSERT_PROVIDER_FILE:';
const ASSERT_AGENT_CONTEXT_MARKER = 'E2E_ASSERT_AGENT_CONTEXT:';
const ASSERT_QUOTE_MARKER = 'E2E_ASSERT_QUOTE:';
const REPLY_MARKER = 'E2E_REPLY:';
const COUNTED_REPLY_MARKER = 'E2E_COUNTED_REPLY:';
@ -32,6 +33,7 @@ const CREATE_FILE_AUTHORING_FINAL_TEXT = 'E2E file authoring complete';
const EDIT_FILE_AUTHORING_FINAL_TEXT = 'E2E file edit complete';
const MODEL_SPEC_SKILL_ASSERTION_FINAL_TEXT = 'E2E model spec skill assertion passed';
const PROVIDER_FILE_ASSERTION_FINAL_TEXT = 'E2E provider file assertion passed';
const AGENT_CONTEXT_ASSERTION_FINAL_TEXT = 'E2E agent context assertion passed';
const QUOTE_ASSERTION_FINAL_TEXT = 'E2E quote assertion passed';
const SLOW_CHUNK_DELAY_MS = Number(process.env.MOCK_LLM_SLOW_CHUNK_DELAY_MS) || 35;
const SLOW_REPLY_CHUNKS = 160;
@ -163,6 +165,32 @@ function collectAdditionalInstructions(agents) {
.join('\n');
}
function collectPromptText(value, parts = []) {
if (value == null) {
return parts;
}
if (typeof value === 'string') {
parts.push(value);
return parts;
}
if (Array.isArray(value)) {
for (const item of value) {
collectPromptText(item, parts);
}
return parts;
}
if (typeof value === 'object') {
for (const child of Object.values(value)) {
collectPromptText(child, parts);
}
}
return parts;
}
function collectSkillPrimeMessages(messages) {
return (messages ?? [])
.filter((message) => message?.additional_kwargs?.source === 'skill')
@ -234,6 +262,28 @@ function providerFileAssertionResponses({ messages, text }) {
};
}
function agentContextAssertionResponses({ messages, text }) {
const expected = getMarkerValue(text, ASSERT_AGENT_CONTEXT_MARKER);
if (!expected) {
return null;
}
const promptText = collectPromptText(messages).join('\n');
if (promptText.includes(expected)) {
return {
responses: [`${AGENT_CONTEXT_ASSERTION_FINAL_TEXT}: ${expected}`],
};
}
return {
responses: [
`E2E agent context assertion failed: expected ${expected}; saw ${
promptText ? 'prompt context without marker' : 'no prompt context'
}`,
],
};
}
/**
* Verifies the quote feature end to end: scans every user message in the prompt
* the model actually received for a Markdown blockquote line containing the
@ -354,9 +404,46 @@ function replyResponses(text) {
* streaming pattern) so token-usage SSE events flow end to end in mock runs.
*/
class UsageEmittingFakeChatModel extends FakeChatModel {
constructor({ resolveOnStream, sleep, ...options }) {
super({ ...options, sleep });
this.resolveOnStream = resolveOnStream;
this.streamSleep = sleep ?? CHUNK_DELAY_MS;
}
async *streamDynamicResponseChunks({ responses, options, runManager }) {
if (this.emitCustomEvent) {
await runManager?.handleCustomEvent('some_test_event', {
someval: true,
});
}
const response = responses[0] ?? '';
const chunks = response.split(/(?<=\s+)|(?=\s+)/);
for await (const chunk of chunks) {
await new Promise((resolve) => setTimeout(resolve, this.streamSleep));
if (options.thrownErrorString != null && options.thrownErrorString) {
throw new Error(options.thrownErrorString);
}
const responseChunk = this._createResponseChunk(chunk);
yield responseChunk;
void runManager?.handleLLMNewToken(chunk);
}
}
async *_streamResponseChunks(messages, options, runManager) {
let outputChars = 0;
for await (const chunk of super._streamResponseChunks(messages, options, runManager)) {
const dynamicResponse = this.resolveOnStream?.(messages);
const chunkStream = dynamicResponse
? this.streamDynamicResponseChunks({
responses: dynamicResponse.responses,
options,
runManager,
})
: super._streamResponseChunks(messages, options, runManager);
for await (const chunk of chunkStream) {
outputChars += typeof chunk.text === 'string' ? chunk.text.length : 0;
yield chunk;
}
@ -376,13 +463,14 @@ class UsageEmittingFakeChatModel extends FakeChatModel {
}
}
function overrideModel({ graph, responses, sleep, toolCalls, thrownError }) {
function overrideModel({ graph, responses, sleep, toolCalls, thrownError, resolveOnStream }) {
if (!thrownError) {
graph.overrideModel = new UsageEmittingFakeChatModel({
responses,
sleep: sleep ?? CHUNK_DELAY_MS,
emitCustomEvent: true,
toolCalls,
resolveOnStream,
});
return;
}
@ -511,6 +599,14 @@ function resolveResponses({ agents, messages, text, toolNames }) {
return reply;
}
if (text.includes(ASSERT_AGENT_CONTEXT_MARKER)) {
return {
responses: [MOCK_REPLY],
resolveOnStream: (streamMessages) =>
agentContextAssertionResponses({ messages: streamMessages, text }),
};
}
const providerFileAssertion = providerFileAssertionResponses({ messages, text });
if (providerFileAssertion) {
return providerFileAssertion;
@ -566,11 +662,11 @@ module.exports = function fakeModelHook(run, context) {
const text = getLatestUserText(context?.messages);
const toolNames = collectToolNames(context?.agents);
const { responses, sleep, toolCalls, thrownError } = resolveResponses({
const { responses, sleep, toolCalls, thrownError, resolveOnStream } = resolveResponses({
agents: context?.agents,
messages: context?.messages,
text,
toolNames,
});
overrideModel({ graph, responses, sleep, toolCalls, thrownError });
overrideModel({ graph, responses, sleep, toolCalls, thrownError, resolveOnStream });
};

View file

@ -0,0 +1,350 @@
import { expect, test } from '@playwright/test';
import type { APIRequestContext } from '@playwright/test';
import { ObjectId } from 'mongodb';
import type { Db, WithId, Document } from 'mongodb';
import cleanupUser from '../../setup/cleanupUser';
import { getPrimaryE2EUser, getSecondaryE2EUser } from '../../setup/users.mock';
import type { User } from '../../types';
import { openAgentBuilder } from './agents.helpers';
import { withMongo } from './db';
import {
MOCK_ENDPOINTS,
NEW_CHAT_PATH,
fetchJson,
getAccessToken,
messagesView,
sendMessage,
} from './helpers';
const OWNER_PERM_BITS = 1 | 2 | 4 | 8;
type UserDoc = WithId<Document> & {
email: string;
name?: string;
tenantId?: string;
};
type AgentFile = {
file_id: string;
filename: string;
text?: string;
};
type PreviewResponse = {
file_id: string;
status: string;
text?: string;
textFormat?: string | null;
};
async function registerUser(request: APIRequestContext, user: User) {
await cleanupUser(user);
const registerResponse = await request.post('/api/auth/register', {
data: {
email: user.email,
name: user.name,
password: user.password,
confirm_password: user.password,
},
});
expect(registerResponse.ok()).toBeTruthy();
}
async function getUserDoc(db: Db, email: string): Promise<UserDoc> {
const user = await db.collection<UserDoc>('users').findOne({ email });
if (!user) {
throw new Error(`Expected e2e user ${email} to exist`);
}
return user;
}
function makeReadyTextFile({
fileId,
filename,
ownerId,
text,
tenantId,
embedded = false,
}: {
fileId: string;
filename: string;
ownerId: ObjectId;
text: string;
tenantId?: string;
embedded?: boolean;
}) {
const now = new Date();
return {
user: ownerId,
file_id: fileId,
bytes: Buffer.byteLength(text),
filename,
filepath: `/tmp/${fileId}.txt`,
object: 'file',
embedded,
type: 'text/plain',
text,
textFormat: 'text',
status: 'ready',
usage: 0,
source: 'local',
...(tenantId ? { tenantId } : {}),
createdAt: now,
updatedAt: now,
};
}
async function seedAgentWithCrossOwnerFiles({
agentId,
agentObjectId,
agentName,
primaryUser,
secondaryUser,
contextFileId,
contextFilename,
contextText,
searchFileId,
searchFilename,
searchText,
}: {
agentId: string;
agentObjectId: ObjectId;
agentName: string;
primaryUser: UserDoc;
secondaryUser: UserDoc;
contextFileId: string;
contextFilename: string;
contextText: string;
searchFileId: string;
searchFilename: string;
searchText: string;
}) {
const now = new Date();
const tenantId = primaryUser.tenantId;
const toolResources = {
context: { file_ids: [contextFileId] },
file_search: { file_ids: [searchFileId] },
};
await withMongo(async (db) => {
await db.collection('files').insertMany([
makeReadyTextFile({
fileId: contextFileId,
filename: contextFilename,
ownerId: secondaryUser._id,
text: contextText,
tenantId,
}),
makeReadyTextFile({
fileId: searchFileId,
filename: searchFilename,
ownerId: secondaryUser._id,
text: searchText,
tenantId,
embedded: true,
}),
]);
await db.collection('agents').insertOne({
_id: agentObjectId,
id: agentId,
name: agentName,
description: 'E2E agent with files owned by a different editor.',
instructions: 'Use the attached context file for provider-file e2e assertions.',
provider: MOCK_ENDPOINTS[0].label,
model: MOCK_ENDPOINTS[0].model,
author: primaryUser._id,
authorName: primaryUser.name,
tools: ['file_search', 'context'],
category: 'general',
tool_resources: toolResources,
versions: [
{
id: agentId,
name: agentName,
description: 'E2E agent with files owned by a different editor.',
instructions: 'Use the attached context file for provider-file e2e assertions.',
provider: MOCK_ENDPOINTS[0].label,
model: MOCK_ENDPOINTS[0].model,
tools: ['file_search', 'context'],
category: 'general',
tool_resources: toolResources,
createdAt: now,
updatedAt: now,
},
],
...(tenantId ? { tenantId } : {}),
createdAt: now,
updatedAt: now,
});
await db.collection('aclentries').insertMany([
{
principalType: 'user',
principalId: primaryUser._id,
principalModel: 'User',
resourceType: 'agent',
resourceId: agentObjectId,
permBits: OWNER_PERM_BITS,
grantedBy: primaryUser._id,
grantedAt: now,
...(tenantId ? { tenantId } : {}),
createdAt: now,
updatedAt: now,
},
{
principalType: 'user',
principalId: primaryUser._id,
principalModel: 'User',
resourceType: 'remoteAgent',
resourceId: agentObjectId,
permBits: OWNER_PERM_BITS,
grantedBy: primaryUser._id,
grantedAt: now,
...(tenantId ? { tenantId } : {}),
createdAt: now,
updatedAt: now,
},
]);
});
}
async function cleanupSeededRecords({
agentObjectId,
agentId,
fileIds,
conversationId,
}: {
agentObjectId: ObjectId;
agentId: string;
fileIds: string[];
conversationId?: string;
}) {
await withMongo(async (db) => {
await db.collection('agents').deleteMany({ $or: [{ _id: agentObjectId }, { id: agentId }] });
await db.collection('files').deleteMany({ file_id: { $in: fileIds } });
await db.collection('aclentries').deleteMany({ resourceId: agentObjectId });
if (conversationId) {
await db.collection('conversations').deleteMany({ conversationId });
await db.collection('messages').deleteMany({ conversationId });
}
});
}
test.describe('agent file ownership', () => {
test('allows an agent author to list, preview, and use attached files owned by another user', async ({
page,
request,
}) => {
test.setTimeout(120000);
const suffix = `${Date.now()}-${Math.floor(Math.random() * 1e4)}`;
const primary = getPrimaryE2EUser();
const secondary = getSecondaryE2EUser();
const agentObjectId = new ObjectId();
const agentId = `agent_e2e_cross_owner_${suffix}`;
const agentName = `E2E Cross Owner Files ${suffix}`;
const contextFileId = `file_e2e_context_${suffix}`;
const contextFilename = `agent-context-${suffix}.txt`;
const contextMarker = `cross_owner_context_${suffix.replace(/-/g, '_')}`;
const contextText = `Context file owned by the secondary user for ${suffix}. ${contextMarker}`;
const searchFileId = `file_e2e_search_${suffix}`;
const searchFilename = `agent-search-${suffix}.txt`;
const searchText = `Search file owned by the secondary user for ${suffix}.`;
let conversationId: string | undefined;
try {
await registerUser(request, secondary);
const { primaryUser, secondaryUser } = await withMongo(async (db) => ({
primaryUser: await getUserDoc(db, primary.email),
secondaryUser: await getUserDoc(db, secondary.email),
}));
await seedAgentWithCrossOwnerFiles({
agentId,
agentObjectId,
agentName,
primaryUser,
secondaryUser,
contextFileId,
contextFilename,
contextText,
searchFileId,
searchFilename,
searchText,
});
await page.goto(NEW_CHAT_PATH, { timeout: 10000 });
const token = await getAccessToken(page);
const agentFiles = await fetchJson<AgentFile[]>(
page,
`/api/files/agent/${encodeURIComponent(agentId)}`,
token,
);
expect(agentFiles.map((file) => file.file_id).sort()).toEqual(
[contextFileId, searchFileId].sort(),
);
expect(agentFiles.find((file) => file.file_id === contextFileId)?.text).toBeUndefined();
await expect
.poll(
async () =>
fetchJson<PreviewResponse>(
page,
`/api/files/${encodeURIComponent(contextFileId)}/preview`,
token,
),
{ timeout: 10000 },
)
.toMatchObject({
file_id: contextFileId,
status: 'ready',
text: contextText,
textFormat: 'text',
});
await expect
.poll(
async () =>
fetchJson<PreviewResponse>(
page,
`/api/files/${encodeURIComponent(searchFileId)}/preview`,
token,
),
{ timeout: 10000 },
)
.toMatchObject({
file_id: searchFileId,
status: 'ready',
text: searchText,
textFormat: 'text',
});
const form = await openAgentBuilder(page);
await form.getByRole('combobox', { name: 'Agent', exact: true }).click();
await page.getByRole('option', { name: agentName }).click();
await form.getByRole('button', { name: 'Select Agent' }).click();
const response = await sendMessage(page, `E2E_ASSERT_AGENT_CONTEXT:${contextMarker}`);
expect(response.ok()).toBeTruthy();
await expect(
messagesView(page).getByText(`E2E agent context assertion passed: ${contextMarker}`),
).toBeVisible({ timeout: 30000 });
const match = page.url().match(/\/c\/([0-9a-fA-F-]{36})$/);
conversationId = match?.[1];
} finally {
await cleanupSeededRecords({
agentObjectId,
agentId,
fileIds: [contextFileId, searchFileId],
conversationId,
});
await cleanupUser(secondary);
}
});
});