🪤 chore: Scope Assistant Tool-Resource Lookups (#15579)

`DELETE /api/files` looked the assistant up by `id`, but the Assistant schema
stores it as `assistant_id`. `api/db/connect.js` sets `strictQuery`, under which
Mongoose drops filter keys absent from the schema — so the lookup degraded to
`findOne({})` and returned whichever assistant happened to be first in the
collection, regardless of owner.

The unlink path then read that stranger's `tool_resources` and intersected it
with the caller's requested files, so a file the caller named could be unlinked
on the strength of another user's assistant record. With an empty collection the
same lookup returned `null` and the unguarded `assistant.tool_resources` threw,
answering 400 instead of completing the request.

Filter on `assistant_id` and guard the dereference. Adds three route tests that
mirror the server's `strictQuery` setting, without which the harness cannot
reproduce either failure.
This commit is contained in:
Danny Avila 2026-09-04 00:14:44 -04:00 • committed by GitHub
parent bd83da552d
commit 9a43362663
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 79 additions and 2 deletions

View file

@ -289,10 +289,10 @@ router.delete('/', async (req, res) => {
/* 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({
id: req.body.assistant_id,
assistant_id: req.body.assistant_id,
});
const toolResourceFiles = assistant.tool_resources?.[req.body.tool_resource]?.file_ids ?? [];
const toolResourceFiles = assistant?.tool_resources?.[req.body.tool_resource]?.file_ids ?? [];
const assistantFiles = files.filter((f) => toolResourceFiles.includes(f.file_id));
const result = await processDeleteRequest({ req, files: assistantFiles });

View file

@ -694,6 +694,83 @@ describe('File Routes - Delete with Agent Access', () => {
});
});
/* Mirrors api/db/connect.js, which sets strictQuery for the running server. Under it
Mongoose DROPS filter keys absent from the schema, so a lookup on a misspelled path
degrades to findOne({}) — the first document in the collection, whoever owns it. */
describe('DELETE /files - assistant tool resource unlinking', () => {
let previousStrictQuery;
beforeAll(() => {
previousStrictQuery = mongoose.get('strictQuery');
mongoose.set('strictQuery', true);
});
afterAll(async () => {
mongoose.set('strictQuery', previousStrictQuery);
await mongoose.connection.collection('assistants').deleteMany({});
});
beforeEach(async () => {
await mongoose.connection.collection('assistants').deleteMany({});
});
it("does not unlink another user's assistant files when the requested assistant is missing", async () => {
const strangerFileId = uuidv4();
/* Written straight to the collection: `tool_resources` predates the current schema. */
await mongoose.connection.collection('assistants').insertOne({
user: new mongoose.Types.ObjectId(),
assistant_id: 'asst_belonging_to_someone_else',
tool_resources: { file_search: { file_ids: [strangerFileId] } },
});
const response = await request(app)
.delete('/files')
.send({
assistant_id: 'asst_that_does_not_exist',
tool_resource: 'file_search',
files: [{ file_id: strangerFileId, filepath: '/uploads/stranger.txt' }],
});
expect(response.status).toBe(200);
expect(processDeleteRequest).toHaveBeenCalledWith(expect.objectContaining({ files: [] }));
});
it('unlinks the requested assistant own files', async () => {
const ownFileId = uuidv4();
await mongoose.connection.collection('assistants').insertOne({
user: new mongoose.Types.ObjectId(),
assistant_id: 'asst_requested',
tool_resources: { file_search: { file_ids: [ownFileId] } },
});
const response = await request(app)
.delete('/files')
.send({
assistant_id: 'asst_requested',
tool_resource: 'file_search',
files: [{ file_id: ownFileId, filepath: '/uploads/own.txt' }],
});
expect(response.status).toBe(200);
expect(processDeleteRequest).toHaveBeenCalledWith(
expect.objectContaining({ files: [expect.objectContaining({ file_id: ownFileId })] }),
);
});
it('answers instead of throwing when no assistants exist at all', async () => {
const response = await request(app)
.delete('/files')
.send({
assistant_id: 'asst_that_does_not_exist',
tool_resource: 'file_search',
files: [{ file_id: uuidv4(), filepath: '/uploads/ghost.txt' }],
});
expect(response.status).toBe(200);
expect(processDeleteRequest).toHaveBeenCalledWith(expect.objectContaining({ files: [] }));
});
});
describe('GET /files/download-url/:userId/:file_id', () => {
it('returns a direct signed download URL when the strategy supports it', async () => {
const userFileId = uuidv4();