mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(mcp): upload_trip_file tool for adding documents to a trip (#1566)
This commit is contained in:
Maurice
@@ -47,3 +47,17 @@ export const BLOCKED_EXTENSIONS = [
|
||||
// the extra '..' keeps the same absolute <server>/uploads/files under both the
|
||||
// src (vitest) and dist (runtime) layouts.
|
||||
export const filesDir = path.join(__dirname, '../../../uploads/files');
|
||||
|
||||
/**
|
||||
* Whether a trip-file upload named `originalname` passes the extension rules:
|
||||
* never a blocked extension or an SVG, otherwise on the operator's list (or `*`),
|
||||
* and video regardless of that list (#823). Shared by the multipart filter and
|
||||
* the MCP upload tool so the two ingestion paths cannot drift apart.
|
||||
*/
|
||||
export function isUploadTypeAllowed(originalname: string, mimetype: string, allowedList: string): boolean {
|
||||
const ext = path.extname(originalname).toLowerCase();
|
||||
if (BLOCKED_EXTENSIONS.includes(ext) || mimetype.includes('svg')) return false;
|
||||
const allowed = allowedList.split(',').map((e) => e.trim().toLowerCase());
|
||||
const fileExt = ext.replace('.', '');
|
||||
return allowed.includes(fileExt) || isVideoExtension(fileExt) || allowed.includes('*');
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ import { CurrentUser } from '../auth/current-user.decorator';
|
||||
import { TripAccessGuard } from '../permissions/trip-access.guard';
|
||||
import type { TripAccess } from '../database/database.service';
|
||||
import { Trip } from '../permissions/trip.decorator';
|
||||
import { MAX_FILE_SIZE, BLOCKED_EXTENSIONS, isVideoExtension } from './files.constants';
|
||||
import { MAX_FILE_SIZE, isUploadTypeAllowed, isVideoExtension } from './files.constants';
|
||||
import { FileUploadDto, FileUpdateDto, FileLinkDto } from './files.dto';
|
||||
import { AllowedFileTypesService } from './allowed-file-types.service';
|
||||
|
||||
@@ -44,19 +44,11 @@ import { AllowedFileTypesService } from './allowed-file-types.service';
|
||||
*/
|
||||
export function filesUploadFileFilter(allowedTypes: AllowedFileTypesService): Options['fileFilter'] {
|
||||
return (_req, file, cb) => {
|
||||
const ext = path.extname(file.originalname).toLowerCase();
|
||||
const reject = () => {
|
||||
// i18n key — the client resolves it via t() (see translateApiError).
|
||||
const err: Error & { statusCode?: number } = new Error('files.uploadErrorType');
|
||||
err.statusCode = 400;
|
||||
cb(err);
|
||||
};
|
||||
if (BLOCKED_EXTENSIONS.includes(ext) || file.mimetype.includes('svg')) return reject();
|
||||
const allowed = allowedTypes.get().split(',').map((e) => e.trim().toLowerCase());
|
||||
const fileExt = ext.replace('.', '');
|
||||
// Video is accepted as media regardless of the admin doc-types allowlist (#823).
|
||||
if (allowed.includes(fileExt) || isVideoExtension(fileExt) || (allowed.includes('*') && !BLOCKED_EXTENSIONS.includes(ext))) return cb(null, true);
|
||||
reject();
|
||||
if (isUploadTypeAllowed(file.originalname, file.mimetype, allowedTypes.get())) return cb(null, true);
|
||||
// i18n key — the client resolves it via t() (see translateApiError).
|
||||
const err: Error & { statusCode?: number } = new Error('files.uploadErrorType');
|
||||
err.statusCode = 400;
|
||||
cb(err);
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -10,9 +10,21 @@ import { noAccess, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { AuthService } from '../auth/auth.service';
|
||||
import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
|
||||
import { FilesService, FileContentError, FILE_CONTENT_MAX } from './files.service';
|
||||
import { AllowedFileTypesService } from './allowed-file-types.service';
|
||||
import { MAX_FILE_SIZE, isUploadTypeAllowed } from './files.constants';
|
||||
import { contentTypeFor } from '../storage/content-type';
|
||||
|
||||
const CONTENT_MAX_MB = Math.round(FILE_CONTENT_MAX / (1024 * 1024));
|
||||
|
||||
/**
|
||||
* The upload tool's cap: the same 10 MB a read hands back, or the operator's
|
||||
* upload limit when that is lower. The bytes travel base64 inside one JSON-RPC
|
||||
* message, so the multipart route's 500 MB video allowance has no place here.
|
||||
*/
|
||||
const UPLOAD_MAX = Math.min(FILE_CONTENT_MAX, MAX_FILE_SIZE);
|
||||
const UPLOAD_MAX_MB = Math.max(1, Math.floor(UPLOAD_MAX / (1024 * 1024)));
|
||||
const BASE64 = /^[A-Za-z0-9+/]*={0,2}$/;
|
||||
|
||||
/**
|
||||
* Bytes worth handing over as text. Everything else goes back base64: a model
|
||||
* can pass those on to a converter, but it cannot read them, and decoding a PDF
|
||||
@@ -42,9 +54,10 @@ function contentRefusal(err: FileContentError): string {
|
||||
* two listing tools carry no file_* right because the routes they mirror carry
|
||||
* none either; trip access is the whole gate there.
|
||||
*
|
||||
* There is no upload tool on purpose. MCP has no file transport, and an upload
|
||||
* faked through base64 would be a second, unpoliced ingestion path beside the
|
||||
* multipart route with its extension filter and per-type size caps.
|
||||
* upload_trip_file (#1566) takes the bytes base64-encoded, since MCP has no file
|
||||
* transport. It is policed like the multipart route: the same extension rules
|
||||
* through isUploadTypeAllowed, the file_upload right, the demo block and the
|
||||
* same-trip check on what it attaches to, plus a tighter size cap.
|
||||
*/
|
||||
@McpController()
|
||||
export class FilesMcp {
|
||||
@@ -52,6 +65,7 @@ export class FilesMcp {
|
||||
private readonly files: FilesService,
|
||||
private readonly auth: AuthService,
|
||||
private readonly guards: McpToolGuardsService,
|
||||
private readonly allowedTypes: AllowedFileTypesService,
|
||||
) {}
|
||||
|
||||
@Tool({
|
||||
@@ -103,6 +117,51 @@ export class FilesMcp {
|
||||
}
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'upload_trip_file',
|
||||
description: `Add a document to a trip, e.g. a booking confirmation PDF or a ticket, and optionally attach it to a booking or a place in the same call. Pass the bytes base64-encoded in content, up to ${UPLOAD_MAX_MB} MB decoded. The file name has to carry an extension the trip's file manager accepts; the type is derived from it. Returns the new file, whose ID works with read_trip_file and link_trip_file.`,
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
filename: z.string().trim().min(1).max(255).describe('File name including its extension, e.g. "hotel-confirmation.pdf"'),
|
||||
content: z.string().min(1).max(Math.ceil(UPLOAD_MAX / 3) * 4).describe('The file bytes, base64-encoded'),
|
||||
description: z.string().max(1000).optional().describe('Free-text description'),
|
||||
reservation_id: z.number().int().positive().optional().describe('Booking on the same trip to attach the file to'),
|
||||
place_id: z.number().int().positive().optional().describe('Place on the same trip to attach the file to'),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_WRITE,
|
||||
access: { group: 'files', mode: 'write' },
|
||||
})
|
||||
async uploadTripFile(
|
||||
{ tripId, filename, content, description, reservation_id, place_id }: {
|
||||
tripId: number; filename: string; content: string; description?: string; reservation_id?: number; place_id?: number;
|
||||
},
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (!this.files.verifyTripAccess(tripId, ctx.userId)) return noAccess();
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.guards.hasTripPermission('file_upload', tripId, ctx.userId)) return permissionDenied();
|
||||
// A path someone pasted in is not a name; keep only the last segment.
|
||||
const originalname = filename.split(/[\\/]/).pop()!.trim();
|
||||
const mimetype = contentTypeFor(originalname);
|
||||
if (!originalname || !isUploadTypeAllowed(originalname, mimetype, this.allowedTypes.get())) {
|
||||
return errorResult('This file type is not allowed on this TREK instance.');
|
||||
}
|
||||
const clean = content.replace(/\s+/g, '');
|
||||
if (!BASE64.test(clean)) return errorResult('content is not valid base64.');
|
||||
const bytes = Buffer.from(clean, 'base64');
|
||||
if (bytes.length === 0) return errorResult('The file is empty.');
|
||||
if (bytes.length > UPLOAD_MAX) return errorResult(`File is too large (over ${UPLOAD_MAX_MB} MB). Ask the user to upload it in TREK instead.`);
|
||||
const foreign = this.files.findForeignLinkTarget(tripId, { reservation_id, place_id });
|
||||
if (foreign) return errorResult(`Linked item does not belong to this trip (${foreign}).`);
|
||||
const file = await this.files.createFileFromBytes(tripId, { originalname, mimetype, bytes }, ctx.userId, {
|
||||
reservation_id: reservation_id ?? null,
|
||||
place_id: place_id ?? null,
|
||||
description: description ?? null,
|
||||
});
|
||||
this.guards.safeBroadcast(tripId, 'file:created', { file });
|
||||
return ok({ file });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'update_trip_file',
|
||||
description: 'Set a file\'s description and attach it to a booking or a place. Fields left out keep their current value, null detaches. place_id and reservation_id are the file\'s primary attachment, the one the file manager shows next to it. To attach one document to several bookings or places at once, use link_trip_file instead.',
|
||||
|
||||
@@ -40,7 +40,9 @@ import { MAX_FILE_SIZE, MAX_VIDEO_SIZE } from './files.constants';
|
||||
// AuthModule + McpSharedModule feed FilesMcp's demo and RBAC guards. Neither is
|
||||
// @Global, and AuthModule reaches this domain only through the leaf
|
||||
// AllowedFileTypesModule, so importing it here stays cycle-free.
|
||||
EphemeralTokenModule, PermissionsModule, AppConfigModule, RealtimeModule, PluginGuardsModule, AuthModule, McpSharedModule],
|
||||
EphemeralTokenModule, PermissionsModule, AppConfigModule, RealtimeModule, PluginGuardsModule, AuthModule, McpSharedModule,
|
||||
// FilesMcp's upload tool checks the same extension list as the multipart filter.
|
||||
AllowedFileTypesModule],
|
||||
controllers: [FilesController, FilesDownloadController],
|
||||
providers: [FilesService, FilesRpc, FilesMcp],
|
||||
exports: [FilesService],
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import path from 'path';
|
||||
import type { Readable } from 'node:stream';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { Readable } from 'node:stream';
|
||||
import type { Request } from 'express';
|
||||
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
@@ -282,6 +283,22 @@ export class FilesService {
|
||||
return formatFile(created);
|
||||
}
|
||||
|
||||
/**
|
||||
* Store bytes that arrived without multipart (the MCP upload tool, #1566) and
|
||||
* record them like a multipart upload: same random storage key, same row. The
|
||||
* caller has already run the type and size checks.
|
||||
*/
|
||||
async createFileFromBytes(
|
||||
tripId: string | number,
|
||||
upload: { originalname: string; mimetype: string; bytes: Buffer },
|
||||
uploadedBy: number,
|
||||
opts: { place_id?: number | null; reservation_id?: number | null; description?: string | null },
|
||||
) {
|
||||
const filename = `${randomUUID()}${path.extname(upload.originalname)}`;
|
||||
await this.storage.put('files', filename, Readable.from([upload.bytes]), { contentType: upload.mimetype });
|
||||
return this.createFile(tripId, { filename, originalname: upload.originalname, size: upload.bytes.length, mimetype: upload.mimetype }, uploadedBy, opts);
|
||||
}
|
||||
|
||||
updateFile(
|
||||
id: string | number,
|
||||
current: TripFile,
|
||||
|
||||
@@ -233,7 +233,7 @@ export function createMcpTestRegistry(): McpRegistry {
|
||||
new DaysMcp(daysService, authService, guards, dayRemovalService),
|
||||
new RoadtripMcp(new RoadtripService(dbService, realtimeService), dbService, guards, authService, addonsService),
|
||||
new RoadtripPreferencesMcp(new RoadtripPreferencesService(dbService, realtimeService), authService, addonsService, dbService, guards),
|
||||
new FilesMcp(new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage), authService, guards),
|
||||
new FilesMcp(new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage), authService, guards, new AllowedFileTypesService(dbService)),
|
||||
new AccommodationsMcp(accommodationsService, dbService, placesService, authService, guards),
|
||||
new AssignmentsMcp(assignmentsService, daysService, authService, guards),
|
||||
new CollabMcp(collabService, authService, addonsService, guards),
|
||||
|
||||
@@ -107,7 +107,7 @@ interface FileRowOverrides {
|
||||
deleted_at: string | null;
|
||||
}
|
||||
|
||||
/** A trip_files row, straight in: there is no upload path through MCP to make one. */
|
||||
/** A trip_files row, straight in, so each case controls every column. */
|
||||
function insertFile(tripId: number, overrides: Partial<FileRowOverrides> = {}) {
|
||||
const info = testDb.prepare(`
|
||||
INSERT INTO trip_files (trip_id, filename, original_name, file_size, mime_type, description, uploaded_by, place_id, reservation_id, starred, deleted_at)
|
||||
@@ -501,6 +501,126 @@ describe('Tool: update_trip_file', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// upload_trip_file (#1566)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Tool: upload_trip_file', () => {
|
||||
const pdf = Buffer.from('%PDF-1.4 booking').toString('base64');
|
||||
|
||||
it('stores the bytes, records the row and attaches it to a booking', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const booking = createReservation(testDb, trip.id);
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
const data = parseToolResult(await h.client.callTool({
|
||||
name: 'upload_trip_file',
|
||||
arguments: { tripId: trip.id, filename: 'C:\\mail\\hotel.pdf', content: pdf, description: 'Hotel', reservation_id: booking.id },
|
||||
})) as any;
|
||||
expect(data.file).toMatchObject({ original_name: 'hotel.pdf', mime_type: 'application/pdf', file_size: 16, description: 'Hotel', reservation_id: booking.id, uploaded_by: user.id });
|
||||
expect(data.file.filename).toMatch(/^[0-9a-f-]{36}\.pdf$/);
|
||||
expect(fs.readFileSync(nodePath.join(fixture.root, data.file.filename), 'utf8')).toBe('%PDF-1.4 booking');
|
||||
// And the read tool hands the same bytes back.
|
||||
const read = parseToolResult(await h.client.callTool({ name: 'read_trip_file', arguments: { tripId: trip.id, fileId: data.file.id } })) as any;
|
||||
expect(Buffer.from(read.file.content, 'base64').toString()).toBe('%PDF-1.4 booking');
|
||||
});
|
||||
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'file:created', expect.objectContaining({ _source: 'mcp' }));
|
||||
});
|
||||
|
||||
it('refuses an extension the file manager would refuse', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
for (const filename of ['evil.html', 'logo.svg', 'tool.exe', 'noext']) {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename, content: pdf } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(JSON.stringify(result.content)).toContain('not allowed');
|
||||
}
|
||||
});
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM trip_files').get()).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('follows the operator list, including the wildcard', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', 'pdf')").run();
|
||||
await withHarness(user.id, async (h) => {
|
||||
const refused = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'notes.txt', content: pdf } });
|
||||
expect(refused.isError).toBe(true);
|
||||
testDb.prepare("UPDATE app_settings SET value = '*' WHERE key = 'allowed_file_types'").run();
|
||||
const data = parseToolResult(await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'route.gpx', content: pdf } })) as any;
|
||||
expect(data.file.mime_type).toBe('application/gpx+xml');
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses content that is not base64, or decodes to nothing', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const bad = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'a.pdf', content: 'not base64!' } });
|
||||
expect(JSON.stringify(bad.content)).toContain('not valid base64');
|
||||
const empty = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'a.pdf', content: '==' } });
|
||||
expect(JSON.stringify(empty.content)).toContain('empty');
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a file over the cap', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
// Just past the cap after decoding, still inside the schema's base64 length.
|
||||
const big = Buffer.alloc(FILE_CONTENT_MAX + 1, 1).toString('base64');
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'big.pdf', content: big } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(JSON.stringify(result.content)).toContain('too large');
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a place from another trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const foreignTrip = createTrip(testDb, user.id);
|
||||
const foreignPlace = createPlace(testDb, foreignTrip.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'a.pdf', content: pdf, place_id: foreignPlace.id } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(JSON.stringify(result.content)).toContain('does not belong to this trip');
|
||||
});
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM trip_files').get()).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('refuses a member once file_upload is owner-only', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
setPermission('file_upload', 'trip_owner');
|
||||
await withHarness(member.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'a.pdf', content: pdf } });
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('denies a non-member, and blocks a demo user', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const foreign = createTrip(testDb, other.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: foreign.id, filename: 'a.pdf', content: pdf } });
|
||||
expect(JSON.stringify(result.content)).toContain('Trip not found or access denied.');
|
||||
});
|
||||
process.env.DEMO_MODE = 'true';
|
||||
const { user: demo } = createUser(testDb, { email: 'demo@trek.app' });
|
||||
const own = createTrip(testDb, demo.id);
|
||||
await withHarness(demo.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: own.id, filename: 'a.pdf', content: pdf } });
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM trip_files').get()).toEqual({ n: 0 });
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// link_trip_file
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -818,7 +938,7 @@ describe('Tool: list_trip_file_links', () => {
|
||||
|
||||
describe('File tools: scope gating', () => {
|
||||
const READ_TOOLS = ['list_trip_files', 'list_trip_file_links'];
|
||||
const WRITE_TOOLS = ['update_trip_file', 'link_trip_file', 'unlink_trip_file'];
|
||||
const WRITE_TOOLS = ['upload_trip_file', 'update_trip_file', 'link_trip_file', 'unlink_trip_file'];
|
||||
|
||||
async function listToolNames(userId: number, scopes: string[] | null): Promise<string[]> {
|
||||
const h = await createMcpHarness({ userId, withResources: false, scopes });
|
||||
@@ -829,7 +949,7 @@ describe('File tools: scope gating', () => {
|
||||
}
|
||||
}
|
||||
|
||||
it('registers all six tools for a full-access session', async () => {
|
||||
it('registers all seven tools for a full-access session', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const names = await listToolNames(user.id, null);
|
||||
for (const tool of [...READ_TOOLS, ...WRITE_TOOLS, 'read_trip_file']) expect(names).toContain(tool);
|
||||
|
||||
Reference in New Issue
Block a user