diff --git a/.gitignore b/.gitignore index 02554be26a..59ab18a925 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ lerna-debug.log # forest-bff openapi --output default destination openapi.json + # yarn yarn-error.log .vscode/settings.json diff --git a/packages/_example/src/forest/agent.ts b/packages/_example/src/forest/agent.ts index 436e6627ec..9214c69a50 100644 --- a/packages/_example/src/forest/agent.ts +++ b/packages/_example/src/forest/agent.ts @@ -93,7 +93,9 @@ export default function makeAgent() { return resultBuilder.value((rows?.[0]?.value as number) ?? 0); }) - .mountAiMcpServer(allowedOAuthClients ? { allowedOAuthClients } : undefined) + .mountAiMcpServer({ + ...(allowedOAuthClients && { allowedOAuthClients }), + }) .customizeCollection('card', customizeCard) .customizeCollection('account', customizeAccount) diff --git a/packages/_example/src/forest/customizations/review.ts b/packages/_example/src/forest/customizations/review.ts index 0cd5bdf7c1..f94a4fdd8b 100644 --- a/packages/_example/src/forest/customizations/review.ts +++ b/packages/_example/src/forest/customizations/review.ts @@ -1,4 +1,35 @@ import type { ReviewCustomizer } from '../typings'; export default (collection: ReviewCustomizer) => - collection.addManyToOneRelation('store', 'store', { foreignKey: 'storeId' }); + collection + .addManyToOneRelation('store', 'store', { foreignKey: 'storeId' }) + + .addAction('Attach a document', { + scope: 'Single', + form: [ + { label: 'Document', type: 'File', isRequired: true }, + { label: 'Extra pages', type: 'FileList' }, + { label: 'Note', type: 'String' }, + ], + execute: async (context, resultBuilder) => { + const document = context.formValues.Document as { + name: string; + mimeType: string; + buffer: Buffer; + }; + const extras = (context.formValues['Extra pages'] ?? []) as (typeof document)[]; + + const describe = (file: typeof document) => + `${file?.name} (${file?.mimeType}, ${file?.buffer?.length} bytes)`; + + return resultBuilder.success( + [ + `Received ${describe(document)}`, + extras.length + ? `plus ${extras.length}: ${extras.map(describe).join(', ')}` + : 'no extras', + `note: ${context.formValues.Note ?? '-'}`, + ].join(' — '), + ); + }, + }); diff --git a/packages/agent-bff/src/action/action-form-mapper.ts b/packages/agent-bff/src/action/action-form-mapper.ts index 56fe55e018..adfd79d4ad 100644 --- a/packages/agent-bff/src/action/action-form-mapper.ts +++ b/packages/agent-bff/src/action/action-form-mapper.ts @@ -5,7 +5,8 @@ const ENUM_TYPE = 'Enum'; export interface ActionFormFieldResponse { name: string; - type: string; + /** Verbatim from the agent, so a list type is `['String']` rather than `'StringList'`. */ + type: string | [string]; value: unknown; isRequired: boolean; enumValues?: string[] | null; diff --git a/packages/agent-bff/src/action/agent-action-client.ts b/packages/agent-bff/src/action/agent-action-client.ts index c0695961e0..cd597ef960 100644 --- a/packages/agent-bff/src/action/agent-action-client.ts +++ b/packages/agent-bff/src/action/agent-action-client.ts @@ -7,7 +7,8 @@ import createAgentHttpRequester from '../agent/create-agent-http-requester'; export interface ActionFormField { getName(): string; - getType(): string; + /** A list type is the array the agent sent, `['String']`, not `'StringList'`. */ + getType(): string | [string]; getValue(): unknown; isRequired(): boolean | undefined; } diff --git a/packages/agent-client/src/action-fields/action-field.ts b/packages/agent-client/src/action-fields/action-field.ts index cd5e13cfba..cee6ed924e 100644 --- a/packages/agent-client/src/action-fields/action-field.ts +++ b/packages/agent-client/src/action-fields/action-field.ts @@ -1,4 +1,5 @@ import type FieldFormStates from './field-form-states'; +import type { PlainField } from './types'; export default abstract class ActionField { private readonly fieldsFormStates: FieldFormStates; @@ -17,10 +18,14 @@ export default abstract class ActionField { return this.name; } - getType(): string { + getType(): PlainField['type'] { return this.field?.getType(); } + getTypeName(): string { + return this.field?.getTypeName(); + } + getValue() { return this.field?.getValue(); } diff --git a/packages/agent-client/src/action-fields/field-form-states.ts b/packages/agent-client/src/action-fields/field-form-states.ts index 1f68f3307a..d0297ff391 100644 --- a/packages/agent-client/src/action-fields/field-form-states.ts +++ b/packages/agent-client/src/action-fields/field-form-states.ts @@ -7,6 +7,7 @@ import type { import HttpRequester from '../http-requester'; import ActionFieldMultipleChoice from './action-field-multiple-choice'; import FieldGetter from './field-getter'; +import encodeFileFieldValue from './file-value'; export default class FieldFormStates { private readonly fields: FieldGetter[]; @@ -70,7 +71,7 @@ export default class FieldFormStates { const field = this.getField(name); if (!field) throw new Error(`Field "${name}" not found in action "${this.actionName}"`); - field.getPlainField().value = value; + field.getPlainField().value = encodeFileFieldValue(field.getTypeName(), value, name); const fieldHasHook = field.getPlainField().hook; diff --git a/packages/agent-client/src/action-fields/field-getter.ts b/packages/agent-client/src/action-fields/field-getter.ts index 15e1b42514..cf182910fe 100644 --- a/packages/agent-client/src/action-fields/field-getter.ts +++ b/packages/agent-client/src/action-fields/field-getter.ts @@ -19,7 +19,19 @@ export default class FieldGetter { return this.plainField.field; } - getType(): string { + /** Exactly what the agent sent: a list type is the array `['File']`, not `'FileList'`. */ + getType(): PlainField['type'] { return this.plainField.type; } + + /** + * The same type as a single name, `['File']` becoming `'FileList'`. For dispatching on the type + * and for reporting it to a reader; never for anything that goes back to an agent, which echoes + * `plainField` verbatim through loadChanges and matches only the array form. + */ + getTypeName(): string { + const { type } = this.plainField; + + return Array.isArray(type) ? `${type[0]}List` : type; + } } diff --git a/packages/agent-client/src/action-fields/file-value.ts b/packages/agent-client/src/action-fields/file-value.ts new file mode 100644 index 0000000000..e4f09b343c --- /dev/null +++ b/packages/agent-client/src/action-fields/file-value.ts @@ -0,0 +1,68 @@ +import type { File } from '@forestadmin/datasource-toolkit'; + +import { makeDataUri } from '@forestadmin/datasource-toolkit'; + +function isFileType(type: string): boolean { + return type === 'File'; +} + +function isFileListType(type: string): boolean { + return type === 'FileList'; +} + +function isFile(value: unknown): value is File { + const candidate = value as File; + + return ( + typeof value === 'object' && + value !== null && + Buffer.isBuffer(candidate.buffer) && + typeof candidate.mimeType === 'string' && + typeof candidate.name === 'string' + ); +} + +function fileError(fieldName: string, detail: string): Error { + return new Error(`Field "${fieldName}" ${detail}`); +} + +function encodeFileValue(value: unknown, fieldName: string): unknown { + if (value === null || value === undefined) return value; + + // Callers that address the file indirectly (mcp-server upload handles) keep their sentinel: + // validating strings here would break them, and the agent owns the final validation. + if (typeof value === 'string') return value; + + if (isFile(value)) return makeDataUri(value); + + throw fileError( + fieldName, + 'expects a file: pass { buffer, mimeType, name } or a string holding a data uri.', + ); +} + +export default function encodeFileFieldValue( + type: string, + value: unknown, + fieldName: string, +): unknown { + if (isFileListType(type)) { + if (value === null || value === undefined) return value; + + if (!Array.isArray(value)) { + throw fileError(fieldName, 'expects a list of files: pass an array.'); + } + + return value.map(item => encodeFileValue(item, fieldName)); + } + + if (isFileType(type)) return encodeFileValue(value, fieldName); + + // A file reaching a field that is not declared as one is never intentional, and it would be + // JSON-serialized into the column as {"buffer":{"type":"Buffer",...}} without any error. + if (isFile(value)) { + throw fileError(fieldName, `is a ${type} field and cannot hold a file.`); + } + + return value; +} diff --git a/packages/agent-client/src/action-fields/types.ts b/packages/agent-client/src/action-fields/types.ts index 58855a256d..708f395f17 100644 --- a/packages/agent-client/src/action-fields/types.ts +++ b/packages/agent-client/src/action-fields/types.ts @@ -12,7 +12,8 @@ export type PlainFieldOption = { export type PlainField = { field: string; - type: string; + // Agents emit list types as a single-element array, e.g. ['File'] or ['String']. + type: string | [string]; description?: string; value?: unknown; isRequired: boolean; diff --git a/packages/agent-client/src/index.ts b/packages/agent-client/src/index.ts index c763fa75ab..9d4427c00e 100644 --- a/packages/agent-client/src/index.ts +++ b/packages/agent-client/src/index.ts @@ -68,3 +68,4 @@ export function createRemoteAgentClient(params: { } export type { RecordId, SelectOptions } from './types'; +export type { File } from '@forestadmin/datasource-toolkit'; diff --git a/packages/agent-client/test/action-fields/file-value.test.ts b/packages/agent-client/test/action-fields/file-value.test.ts new file mode 100644 index 0000000000..13af88a019 --- /dev/null +++ b/packages/agent-client/test/action-fields/file-value.test.ts @@ -0,0 +1,226 @@ +import type { PlainField } from '../../src/action-fields/types'; +import type HttpRequester from '../../src/http-requester'; + +import FieldFormStates from '../../src/action-fields/field-form-states'; + +jest.mock('../../src/http-requester', () => { + const actual = jest.requireActual('../../src/http-requester'); + + return { __esModule: true, default: actual.default }; +}); + +const pdf = { mimeType: 'application/pdf', buffer: Buffer.from('%PDF-1.4'), name: 'report.pdf' }; +const pdfDataUri = `data:application/pdf;name=report.pdf;base64,${Buffer.from('%PDF-1.4').toString( + 'base64', +)}`; + +describe('file values in action forms', () => { + let httpRequester: jest.Mocked; + let fieldFormStates: FieldFormStates; + + const setupFields = async (fields: Partial[]) => { + httpRequester.query.mockResolvedValue({ + fields: fields.map(f => ({ isRequired: false, isReadOnly: false, ...f })), + layout: [], + }); + await fieldFormStates.loadInitialState(); + httpRequester.query.mockClear(); + }; + + beforeEach(() => { + jest.clearAllMocks(); + httpRequester = { query: jest.fn() } as unknown as jest.Mocked; + fieldFormStates = new FieldFormStates( + 'attachDocument', + '/forest/actions/attach-document', + 'operations', + httpRequester, + ['1'], + ); + }); + + describe('on a File field', () => { + it('encodes a file object as the data uri the agent expects', async () => { + await setupFields([{ field: 'document', type: 'File' }]); + + await fieldFormStates.setFieldValue('document', pdf); + + expect(fieldFormStates.getFieldValues()).toEqual({ document: pdfDataUri }); + }); + + it('percent-encodes the name so a filename cannot break the header', async () => { + await setupFields([{ field: 'document', type: 'File' }]); + + await fieldFormStates.setFieldValue('document', { ...pdf, name: 'rapport final;v2.pdf' }); + + expect(fieldFormStates.getFieldValues().document).toBe( + `data:application/pdf;name=rapport%20final%3Bv2.pdf;base64,${Buffer.from( + '%PDF-1.4', + ).toString('base64')}`, + ); + }); + + it('encodes the charset when the file carries one', async () => { + await setupFields([{ field: 'document', type: 'File' }]); + + await fieldFormStates.setFieldValue('document', { + mimeType: 'text/csv', + buffer: Buffer.from('a,b'), + name: 'data.csv', + charset: 'utf-8', + }); + + expect(fieldFormStates.getFieldValues().document).toContain(';charset=utf-8;'); + }); + + it('leaves an already encoded data uri byte-identical', async () => { + await setupFields([{ field: 'document', type: 'File' }]); + + await fieldFormStates.setFieldValue('document', pdfDataUri); + + expect(fieldFormStates.getFieldValues()).toEqual({ document: pdfDataUri }); + }); + + it('leaves an opaque string reference untouched', async () => { + await setupFields([{ field: 'document', type: 'File' }]); + + await fieldFormStates.setFieldValue('document', '$uploadedFile:some-token'); + + expect(fieldFormStates.getFieldValues()).toEqual({ document: '$uploadedFile:some-token' }); + }); + + it.each([ + ['null', null], + ['undefined', undefined], + ])('leaves %s untouched', async (_, value) => { + await setupFields([{ field: 'document', type: 'File' }]); + + await expect(fieldFormStates.setFieldValue('document', value)).resolves.not.toThrow(); + }); + + it.each([ + ['a bare buffer', Buffer.from('%PDF-1.4')], + ['an object that is not a file', { foo: 1 }], + ['a file without a mime type', { buffer: Buffer.from('x'), name: 'x.pdf' }], + ])('rejects %s with an actionable message', async (_, value) => { + await setupFields([{ field: 'document', type: 'File' }]); + + await expect(fieldFormStates.setFieldValue('document', value)).rejects.toThrow( + 'Field "document" expects a file: pass { buffer, mimeType, name } ' + + 'or a string holding a data uri.', + ); + }); + }); + + describe('on a file list field', () => { + it.each([['FileList'], [['File']]])('encodes each item of a %p field', async type => { + await setupFields([{ field: 'attachments', type: type as PlainField['type'] }]); + + await fieldFormStates.setFieldValue('attachments', [pdf, '$uploadedFile:token']); + + expect(fieldFormStates.getFieldValues()).toEqual({ + attachments: [pdfDataUri, '$uploadedFile:token'], + }); + }); + + it('leaves null untouched', async () => { + await setupFields([{ field: 'attachments', type: ['File'] }]); + + await fieldFormStates.setFieldValue('attachments', null); + + expect(fieldFormStates.getFieldValues()).toEqual({ attachments: null }); + }); + + it('rejects a single file where a list is expected, instead of shipping it unencoded', async () => { + await setupFields([{ field: 'attachments', type: ['File'] }]); + + await expect(fieldFormStates.setFieldValue('attachments', pdf)).rejects.toThrow( + 'Field "attachments" expects a list of files: pass an array.', + ); + }); + + it('rejects an invalid item inside the list', async () => { + await setupFields([{ field: 'attachments', type: ['File'] }]); + + await expect(fieldFormStates.setFieldValue('attachments', [{ foo: 1 }])).rejects.toThrow( + 'Field "attachments" expects a file', + ); + }); + }); + + describe('on a field that is not a file', () => { + it('leaves ordinary values alone', async () => { + await setupFields([{ field: 'comment', type: 'String' }]); + + await fieldFormStates.setFieldValue('comment', 'plain text'); + + expect(fieldFormStates.getFieldValues()).toEqual({ comment: 'plain text' }); + }); + + // A resolved upload landing on the wrong field would otherwise be JSON-serialized into the + // column as {"buffer":{"type":"Buffer",...}} with no error at any layer. + it('rejects a file, which would otherwise be serialized into the column', async () => { + await setupFields([{ field: 'comment', type: 'String' }]); + + await expect(fieldFormStates.setFieldValue('comment', pdf)).rejects.toThrow( + 'Field "comment" is a String field and cannot hold a file.', + ); + }); + }); + + describe('type normalization', () => { + // getType() feeds API responses in agent-bff and workflow-executor, so it has to stay the + // value the agent sent. Only getTypeName() collapses it, and only for dispatch and display. + it('keeps getType() on the wire form and offers the collapsed name apart', async () => { + await setupFields([{ field: 'attachments', type: ['File'] }]); + + expect(fieldFormStates.getField('attachments')?.getType()).toEqual(['File']); + expect(fieldFormStates.getField('attachments')?.getTypeName()).toBe('FileList'); + }); + + it('keeps the wire form in the payload echoed back to the agent', async () => { + await setupFields([{ field: 'attachments', type: ['File'], hook: 'changeHook' }]); + httpRequester.query.mockResolvedValue({ fields: [], layout: [] }); + + await fieldFormStates.setFieldValue('attachments', [pdf]); + + expect(httpRequester.query).toHaveBeenCalledWith( + expect.objectContaining({ + body: { + data: { + attributes: expect.objectContaining({ + fields: [expect.objectContaining({ type: ['File'] })], + }), + type: 'custom-action-hook-requests', + }, + }, + }), + ); + }); + }); + + describe('when the file field declares a change hook', () => { + it('sends the encoded value to the agent', async () => { + await setupFields([{ field: 'document', type: 'File', hook: 'changeHook' }]); + httpRequester.query.mockResolvedValue({ fields: [], layout: [] }); + + await fieldFormStates.setFieldValue('document', pdf); + + expect(httpRequester.query).toHaveBeenCalledWith({ + method: 'post', + path: '/forest/actions/attach-document/hooks/change', + body: { + data: { + attributes: { + collection_name: 'operations', + changed_field: 'document', + ids: ['1'], + fields: [expect.objectContaining({ field: 'document', value: pdfDataUri })], + }, + type: 'custom-action-hook-requests', + }, + }, + }); + }); + }); +}); diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 4764ed97f3..f0f9070ac7 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -17,7 +17,7 @@ import type { } from '@forestadmin/datasource-customizer'; import type { DataSource, DataSourceFactory } from '@forestadmin/datasource-toolkit'; import type { ForestSchema } from '@forestadmin/forestadmin-client'; -import type { TokenTtlOptions, ToolName } from '@forestadmin/mcp-server'; +import type { FileUploadsOptions, TokenTtlOptions, ToolName } from '@forestadmin/mcp-server'; import { DataSourceCustomizer } from '@forestadmin/datasource-customizer'; import bodyParser from '@koa/bodyparser'; @@ -57,6 +57,7 @@ export default class Agent extends FrameworkMounter private mcpBasePath?: string; private mcpTokenTtl?: TokenTtlOptions; private mcpAllowedOAuthClients?: string[]; + private mcpFileUploads?: false | FileUploadsOptions; /** In-process workflow executor, created only when addWorkflowExecutor() is called. */ private embeddedExecutor: EmbeddedWorkflowExecutor | null = null; @@ -264,18 +265,25 @@ export default class Agent extends FrameworkMounter * // Example: only accept approved OAuth client applications, matched by the domain of their * // registered redirect URIs (subdomains included). Other clients get invalid_client. * agent.mountAiMcpServer({ allowedOAuthClients: ['dust.tt'] }); + * // Example: action File fields work over MCP out of the box, with the files held in memory. + * // Experimental. Point them at a real backend when one instance is not enough, or pass + * // false to turn the feature off. + * agent.mountAiMcpServer({ fileUploads: { storage } }); + * agent.mountAiMcpServer({ fileUploads: false }); */ mountAiMcpServer(options?: { enabledTools?: ToolName[]; basePath?: string; tokenTtl?: TokenTtlOptions; allowedOAuthClients?: string[]; + fileUploads?: false | FileUploadsOptions; }): this { this.mcpEnabled = true; this.mcpEnabledTools = options?.enabledTools; this.mcpBasePath = options?.basePath; this.mcpTokenTtl = options?.tokenTtl; this.mcpAllowedOAuthClients = options?.allowedOAuthClients; + this.mcpFileUploads = options?.fileUploads; return this; } @@ -396,6 +404,7 @@ export default class Agent extends FrameworkMounter basePath: this.mcpBasePath, tokenTtl: this.mcpTokenTtl, allowedOAuthClients: this.mcpAllowedOAuthClients, + fileUploads: this.mcpFileUploads, agentDispatcher: this.getInProcessDispatcher(), }); diff --git a/packages/agent/src/utils/forest-schema/action-values.ts b/packages/agent/src/utils/forest-schema/action-values.ts index d2a3291540..495a53c139 100644 --- a/packages/agent/src/utils/forest-schema/action-values.ts +++ b/packages/agent/src/utils/forest-schema/action-values.ts @@ -1,5 +1,7 @@ import type { ActionField, CompositeId, DataSource, File } from '@forestadmin/datasource-toolkit'; +import { isDataUri, makeDataUri, parseDataUri } from '@forestadmin/datasource-toolkit'; + import ActionFields from './action-fields'; import SchemaGeneratorActions from './generator-actions'; import IdUtils from '../id'; @@ -28,9 +30,9 @@ export default class ForestValueConverter { data[key] = IdUtils.unpackId(collection.schema, value as string); } else if (ActionFields.isFileField(field) && value) { - data[key] = this.parseDataUri(value as string); + data[key] = parseDataUri(value as string); } else if (ActionFields.isFileListField(field) && value) { - data[key] = (value as string[])?.map(v => this.parseDataUri(v)); + data[key] = (value as string[])?.map(v => parseDataUri(v)); } else { data[key] = value; } @@ -56,9 +58,11 @@ export default class ForestValueConverter { const collection = dataSource.getCollection(collectionName); data[field.field] = IdUtils.unpackId(collection.schema, field.value as string); } else if (field.type === 'File') { - data[field.field] = this.parseDataUri(field.value as string); + data[field.field] = isDataUri(field.value) ? parseDataUri(field.value) : field.value; } else if (Array.isArray(field.type) && field.type[0] === 'File') { - data[field.field] = (field.value as string[])?.map(v => this.parseDataUri(v)); + data[field.field] = (field.value as string[])?.map(v => + isDataUri(v) ? parseDataUri(v) : v, + ); } else { data[field.field] = field.value; } @@ -83,10 +87,10 @@ export default class ForestValueConverter { for (const [key, value] of Object.entries(rawData)) { // Skip fields from the default form if (!SchemaGeneratorActions.defaultFields.map(f => f.field).includes(key)) { - if (Array.isArray(value) && value.every(v => this.isDataUri(v))) { - data[key] = value.map(uri => this.parseDataUri(uri)); - } else if (this.isDataUri(value)) { - data[key] = this.parseDataUri(value as string); + if (Array.isArray(value) && value.every(v => isDataUri(v))) { + data[key] = value.map(uri => parseDataUri(uri)); + } else if (isDataUri(value)) { + data[key] = parseDataUri(value as string); } else { data[key] = value; } @@ -110,50 +114,13 @@ export default class ForestValueConverter { } if (field.type === 'File') { - return this.makeDataUri(value as File); + return makeDataUri(value as File); } if (field.type === 'FileList') { - return (value as File[])?.map(f => this.makeDataUri(f)); + return (value as File[])?.map(f => makeDataUri(f)); } return value; } - - private static parseDataUri(dataUri: string): File { - if (!dataUri) return null; - - // Poor man's data uri parser (spec compliants one don't get the filename). - // Hopefully this does not break. - const [header, data] = dataUri.substring(5).split(','); - const [mimeType, ...mediaTypes] = header.split(';'); - const result = { mimeType, buffer: Buffer.from(data, 'base64') }; - - for (const mediaType of mediaTypes) { - const index = mediaType.indexOf('='); - - if (index !== -1) { - result[mediaType.substring(0, index)] = decodeURIComponent(mediaType.substring(index + 1)); - } - } - - return result as File; - } - - private static makeDataUri(file: File): string { - if (!file) return null; - - const { mimeType, buffer, ...rest } = file; - const mediaTypes = Object.entries(rest) - .map(([key, value]) => `${key}=${encodeURIComponent(value)}`) - .join(';'); - - return mediaTypes.length - ? `data:${file.mimeType};${mediaTypes};base64,${buffer.toString('base64')}` - : `data:${file.mimeType};base64,${buffer.toString('base64')}`; - } - - private static isDataUri(value: unknown): boolean { - return typeof value === 'string' && value.startsWith('data:'); - } } diff --git a/packages/datasource-toolkit/src/index.ts b/packages/datasource-toolkit/src/index.ts index 5a60731a29..97048ff3e8 100644 --- a/packages/datasource-toolkit/src/index.ts +++ b/packages/datasource-toolkit/src/index.ts @@ -61,4 +61,5 @@ export { default as TypeGetter } from './validation/type-getter'; export { default as CollectionUtils } from './utils/collection'; export { default as RecordUtils } from './utils/record'; export { default as SchemaUtils } from './utils/schema'; +export { isDataUri, makeDataUri, parseDataUri } from './utils/data-uri'; export { default as Deferred } from './deferred'; diff --git a/packages/datasource-toolkit/src/utils/data-uri.ts b/packages/datasource-toolkit/src/utils/data-uri.ts new file mode 100644 index 0000000000..0f217fc67b --- /dev/null +++ b/packages/datasource-toolkit/src/utils/data-uri.ts @@ -0,0 +1,67 @@ +import type { File } from '../interfaces/action'; + +import { ValidationError } from '../errors'; + +const METADATA_KEYS = ['name', 'charset'] as const; + +export function isDataUri(value: unknown): value is string { + return typeof value === 'string' && value.startsWith('data:'); +} + +export function makeDataUri(file: File): string { + if (!file) return null; + + const { mimeType, buffer, ...mediaTypes } = file; + const encoded = Object.entries(mediaTypes) + .filter(([, value]) => value) + .map(([key, value]) => `${key}=${encodeURIComponent(value as string)}`) + .join(';'); + + const header = encoded ? `data:${mimeType};${encoded}` : `data:${mimeType}`; + + return `${header};base64,${buffer.toString('base64')}`; +} + +// Hand-rolled rather than a spec-compliant parser: Forest carries the filename in a +// non-standard `name=` media type that RFC 2397 does not define, and spec parsers drop it. +export function parseDataUri(dataUri: string): File { + if (!dataUri) return null; + + // Everything below is reachable from an action value, which a model populates freely, and every + // raw failure here is opaque: no comma makes Buffer.from raise a TypeError, and a lone '%' in a + // media type makes decodeURIComponent raise a URIError. Both surface as a generic 500, so the + // caller never learns what to send instead. + const malformed = () => + new ValidationError( + `Expected a file, got "${dataUri.slice(0, 32)}". A file value must be a data uri.`, + ); + + if (!dataUri.startsWith('data:') || !dataUri.includes(',')) throw malformed(); + + const [header, data] = dataUri.substring(5).split(','); + const [mimeType, ...mediaTypes] = header.split(';'); + + // Buffer.from(x, 'base64') never throws: it skips what it cannot read and stops at padding. So a + // uri whose payload is not base64 at all — `data:text/plain,hello` — would decode to plausible + // garbage and reach the end user as a corrupt file, reported as a success. + if (!mediaTypes.includes('base64')) throw malformed(); + + const result = { mimeType, buffer: Buffer.from(data, 'base64') }; + + for (const mediaType of mediaTypes) { + const index = mediaType.indexOf('='); + const key = index === -1 ? '' : mediaType.substring(0, index); + + // Assigning any key would let "buffer=oops" replace the decoded bytes with a string, or + // "mimeType=..." contradict the header. Only the metadata Forest carries is read back. + if (METADATA_KEYS.includes(key as (typeof METADATA_KEYS)[number])) { + try { + result[key] = decodeURIComponent(mediaType.substring(index + 1)); + } catch { + throw malformed(); + } + } + } + + return result as File; +} diff --git a/packages/datasource-toolkit/test/utils/data-uri.test.ts b/packages/datasource-toolkit/test/utils/data-uri.test.ts new file mode 100644 index 0000000000..ce3a27a476 --- /dev/null +++ b/packages/datasource-toolkit/test/utils/data-uri.test.ts @@ -0,0 +1,143 @@ +import { ValidationError } from '../../src/errors'; +import { isDataUri, makeDataUri, parseDataUri } from '../../src/utils/data-uri'; + +describe('DataUri', () => { + describe('isDataUri', () => { + it.each([ + ['data:text/plain;base64,aGk=', true], + ['https://example.com/file.pdf', false], + ['$uploadedFile:token', false], + [42, false], + [null, false], + [undefined, false], + ])('answers %p for %p', (value, expected) => { + expect(isDataUri(value)).toBe(expected); + }); + }); + + describe('makeDataUri', () => { + it('encodes a file without media types', () => { + const uri = makeDataUri({ mimeType: 'text/plain', buffer: Buffer.from('hi'), name: '' }); + + expect(uri).toBe('data:text/plain;base64,aGk='); + }); + + it('percent-encodes the name so it cannot break the header parsing', () => { + const uri = makeDataUri({ + mimeType: 'application/pdf', + buffer: Buffer.from('%PDF'), + name: 'rapport final;v2,def.pdf', + }); + + expect(uri).toBe( + `data:application/pdf;name=rapport%20final%3Bv2%2Cdef.pdf;base64,${Buffer.from( + '%PDF', + ).toString('base64')}`, + ); + }); + + it('encodes the charset when it is set', () => { + const uri = makeDataUri({ + mimeType: 'text/csv', + buffer: Buffer.from('a,b'), + name: 'data.csv', + charset: 'utf-8', + }); + + expect(uri).toBe( + `data:text/csv;name=data.csv;charset=utf-8;base64,${Buffer.from('a,b').toString('base64')}`, + ); + }); + + it('omits media types that are empty instead of emitting them as undefined', () => { + const uri = makeDataUri({ + mimeType: 'text/plain', + buffer: Buffer.from('hi'), + name: 'note.txt', + charset: undefined, + }); + + expect(uri).toBe('data:text/plain;name=note.txt;base64,aGk='); + expect(uri).not.toContain('charset'); + }); + + it('returns null when there is no file', () => { + expect(makeDataUri(null)).toBeNull(); + }); + }); + + describe('parseDataUri', () => { + it('decodes the mime type and the buffer', () => { + const file = parseDataUri('data:text/plain;base64,aGk='); + + expect(file.mimeType).toBe('text/plain'); + expect(file.buffer.toString()).toBe('hi'); + }); + + it('percent-decodes the media types', () => { + const file = parseDataUri('data:text/plain;name=rapport%20final.txt;base64,aGk='); + + expect(file.name).toBe('rapport final.txt'); + }); + + it('returns null when there is no data uri', () => { + expect(parseDataUri(null)).toBeNull(); + }); + + it.each([ + ['a plain filename', 'report.pdf'], + ['an upload sentinel', '$uploadedFile:eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ.sig'], + ['an http url', 'https://example.com/f.pdf'], + ])('rejects %s with a readable message instead of a TypeError', (_, value) => { + expect(() => parseDataUri(value)).toThrow('A file value must be a data uri'); + }); + + // A bare Error surfaces as a generic 500 at the agent boundary, hiding the message. + it('throws a ValidationError so the message reaches the caller', () => { + expect(() => parseDataUri('report.pdf')).toThrow(ValidationError); + }); + + // These used to raise a raw TypeError / URIError, which the agent renders as a 500. + it.each([ + ['no comma at all', 'data:text/plain;base64'], + ['a lone percent in a media type', 'data:text/plain;name=%;base64,aGk='], + ['an incomplete escape in the name', 'data:text/plain;name=100%.pdf;base64,aGk='], + ])('rejects a data uri with %s', (_, value) => { + expect(() => parseDataUri(value)).toThrow(ValidationError); + }); + + // Assigning any key would let a model-supplied uri replace the decoded bytes with a string. + it.each([ + ['buffer', 'data:text/plain;buffer=oops;base64,aGk='], + ['mimeType', 'data:text/plain;mimeType=image/png;base64,aGk='], + ])('ignores a %p media type rather than letting it overwrite the parsed file', (_, uri) => { + const file = parseDataUri(uri); + + expect(Buffer.isBuffer(file.buffer)).toBe(true); + expect(file.buffer.toString()).toBe('hi'); + expect(file.mimeType).toBe('text/plain'); + }); + + it('still accepts a percent-encoded name', () => { + expect(parseDataUri('data:text/plain;name=100%25.pdf;base64,aGk=').name).toBe('100%.pdf'); + }); + }); + + describe('round trip', () => { + it.each([ + 'simple.pdf', + 'rapport final.txt', + 'facture;2026,janvier.pdf', + 'reçu-café.png', + 'a=b&c.txt', + ])('preserves the name %p', name => { + const file = { mimeType: 'application/octet-stream', buffer: Buffer.from([1, 2, 3]), name }; + + const parsed = parseDataUri(makeDataUri(file)); + + expect(parsed.name).toBe(name); + expect(parsed.buffer).toEqual(file.buffer); + expect(parsed.mimeType).toBe(file.mimeType); + }); + }); +}); diff --git a/packages/mcp-server/CLAUDE.md b/packages/mcp-server/CLAUDE.md index 23cadc4e31..4358f33e30 100644 --- a/packages/mcp-server/CLAUDE.md +++ b/packages/mcp-server/CLAUDE.md @@ -16,6 +16,7 @@ Key flows that only make sense across files: - **OAuth = pass-through to Forest Admin.** `ForestOAuthProvider` (`src/forest-oauth-provider.ts`) does not store tokens. `/oauth/authorize` redirects to the Forest app; `exchange*` calls relay to the Forest server's `/oauth/token`, then `generateAccessToken` re-signs a JWT with `authSecret` carrying the Forest token under the `serverToken` claim. `verifyAccessToken` verifies that JWT and builds `AuthInfo.extra` with `forestServerToken: decoded.serverToken` plus `environmentApiEndpoint` — the latter is **not** in the JWT, it's read from the provider's own `this.environmentApiEndpoint` field (set during env discovery). Both lifetimes mirror the Forest token's own `exp`, optionally shortened by the `tokenTtl` option (`src/utils/token-ttl.ts`) — always a `min`, never a `max`, and capped at the point the TTL is computed so the advertised `expires_in` matches the signed JWT (except on the already-expired-upstream branch, which advertises 3600). The refresh cap is anchored on a `sessionStartedAt` claim carried across refreshes and never re-stamped: Forest grants a full refresh lifetime on every refresh, so a per-refresh cap would slide forever and never force a re-login. `clientsStore.getClient` is the single choke point all three OAuth paths (authorize, code exchange, refresh) resolve clients through — when the `allowedOAuthClients` option is set, it rejects any client whose registered redirect URIs are not all http(s) URIs on an allowed domain or subdomain, by throwing `InvalidClientError` (rendered as a 400 `invalid_client` by the SDK handlers, never a redirect). DCR registration itself happens on the Forest server and is never blocked here. - **Tools call the live agent, not this server.** Each tool in `src/tools/*` is a `declareXxxTool(mcpServer, forestServerClient, logger, collectionNames)` factory. At call time `buildClient(extra)` (`src/utils/agent-caller.ts`) reads `extra.authInfo` (`forestServerToken` + `environmentApiEndpoint` from `AuthInfo.extra`) and builds a `createRemoteAgentClient` from `@forestadmin/agent-client` — i.e. the tool RPCs into the user's actual running agent. `forestServerClient` (`src/http-client`, wrapping `@forestadmin/forestadmin-client`'s `SchemaService`/`ActivityLogsService`) is used only for schema fetch and activity logging, not data. - **Two cross-cutting wrappers, always used together.** `registerToolWithLogging` (`src/utils/tool-with-logging.ts`) registers the tool and converts thrown errors into `{ isError: true }` results (per MCP spec) instead of protocol errors. Inside the handler, `withActivityLog` (`src/utils/with-activity-log.ts`) brackets the operation with a pending→succeeded/failed Forest activity log and runs `parseAgentError` + optional `errorEnhancer` (e.g. `list` appends sortable field names on "Invalid sort"). +- **Action file uploads are an opt-in side-channel** (`src/file-uploads/`), on by default: `EphemeralStorage` holds the objects in memory unless `fileUploads.storage` provides a backend. `fileUploads: false` is the off switch (it drops `requestActionFileUpload` from the enabled set, so the tool, the upload endpoint and the `executeAction` instructions all follow); leaving the tool out of `enabledTools` does the same, at the cost of freezing the allowlist. The `requestActionFileUpload` tool (`src/tools/request-action-file-upload.ts`) returns a pre-authorized upload URL plus a user-bound JWT handle signed with `authSecret`. `FieldGetter.getType()` returns the wire value (`['File']`) because agent-bff and workflow-executor put it straight into API responses; `getTypeName()` is the collapsed `'FileList'`, for dispatch and for what this server reports to a model. It is a tool rather than an HTTP route so clients discover it through `tools/list` instead of a sentence in a description. It checks the `mcp:action` scope itself, because `/mcp` only requires `mcp:read` while the route it replaced required `mcp:action`. `executeAction` swaps `"$uploadedFile:"` values for the downloaded `File` object (`resolve.ts`) and lets **agent-client** encode it — this package never builds a data uri itself. `parseFileReference` (`file-reference.ts`) is the single place that recognizes a reference, so SEP-2631 file URIs can be added there without touching the resolution path. `getActionForm` leaves handles unresolved on purpose, because it echoes values back into the model's context — and withholds them from `tryToSetFields` so a change hook fired by another field never reads `.buffer` off a handle string, while still echoing them back as the field's value and counting them as filling a required field, or `canExecute` could never become true. `download` is **not** consume-on-read: `resolve.ts` fetches every reference before `setFields` and `execute`, so a later failure must leave the objects retryable. With no `storage`, `EphemeralStorage` holds the objects in memory and serves a PUT endpoint under `${prefix}/mcp/uploads` — registered **before the body parsers** (they would consume the raw stream) and before `allowedMethods(['POST'])`; `makeIsMcpRoute` already claims everything under `/mcp/`, so no routing change was needed. Per-instance, so it warns on the first `createUploadUrl` rather than at startup — uploads being on by default, a boot warning would reach agents that have no file field at all. - **`collectionNames` → `z.enum`.** `fetchCollectionNames()` populates the schema's collection list; tools turn it into a `z.enum` for `collectionName` so the LLM gets autocomplete/validation. If schema fetch fails the server logs a warning and runs "degraded" with `z.string()`. ## Commands diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index 8757681d97..437293e87d 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -20,6 +20,7 @@ This MCP server provides HTTP REST API access to Forest Admin operations, enabli | `dissociate` | Dissociate records from a relation | | `getActionForm` | Get the form fields for a custom action | | `executeAction` | Execute a custom action | +| `requestActionFileUpload` | Get a destination to upload a file to, for an action `File` field (only with `fileUploads`) | ## Usage @@ -67,6 +68,8 @@ yarn start:dev # Development (loads .env file automatically) | `FOREST_AGENT_URL` | No | your environment's back-end URL | URL the MCP server uses to reach the back-end's data layer. Set it when the server runs next to a self-hosted back-end at an internal address (e.g. `http://localhost:3310`), instead of the public URL registered in Forest | | `FOREST_MCP_ACCESS_TOKEN_TTL_SECONDS` | No | `3600` (1 hour) | Maximum lifetime of the OAuth access tokens the server issues (`tokenTtl.accessTokenSeconds`). Minimum `60` | | `FOREST_MCP_REFRESH_TOKEN_TTL_SECONDS` | No | unbounded | Maximum time between two interactive logins (`tokenTtl.refreshTokenSeconds`). Unset, a client that keeps refreshing never signs in again. Minimum `60` | +| `FOREST_MCP_FILE_UPLOADS` | No | - | `false` turns action file uploads off (they are **on** by default, in memory). Any other value than `true`/`false` fails at startup | +| `FOREST_MCP_UPLOAD_STORAGE_MODULE` | No | - | Path to a module providing the `fileUploads` options, for a real storage backend. `FOREST_MCP_FILE_UPLOADS=false` wins over it | #### Example Configuration @@ -164,6 +167,250 @@ The two settings differ in what the user notices: The minimum for either value is 60 seconds; anything lower is raised to it. An invalid value (zero, negative, fractional) fails at startup rather than silently leaving the tokens uncapped. +## Action File Uploads + +> **Experimental.** The MCP specification is still designing its own file transfer story +> ([SEP-2631](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2631)). The +> `UploadStorage` contract is expected to survive, but the `requestActionFileUpload` tool and the handle +> format may change to follow the specification once it lands. + +Actions with **File fields** cannot normally run over MCP. The agent expects file values as data +uris, which would transit the model's context window and exceed most MCP clients' payload limits. +The `fileUploads` option enables them through an upload side-channel that keeps the bytes out of +the conversation: + +1. The client calls the `requestActionFileUpload` tool with `{ "filename", "mimeType", "sha256"? }` and receives a pre-authorized upload URL plus a signed `fileHandle` string. +2. The client uploads the raw bytes directly to the storage backend, so they never pass through the MCP server or the model. +3. The client passes the handle (`"$uploadedFile:<...>"`) as the field value in `executeAction`. The server downloads the object and hands it to the agent. The model only ever exchanges the small handle. + +`requestActionFileUpload` follows `enabledTools` like every other tool, so a server that leaves it out never advertises it and never serves the upload endpoint. + +The upload URL is unauthenticated — the model holds no agent credential, and must not — so the URL +itself is the authorization, as with an S3 presigned PUT. It carries a random uuid, is refused +before a byte is read unless this server issued it, expires with `uploadUrlTtlSeconds`, and serves +nothing but the `PUT`. Against the in-memory store it also **accepts a single upload**: once the +bytes land, a leaked URL can no longer replace them. Writing is not consuming either — redemption +needs the signed handle, which is bound to the user who requested it. + +A presigned backend URL is a different animal: it is typically **replayable** until it expires — S3 +accepts as many `PUT`s as fit in `expiresInSeconds` — so there, a URL leaked to an access log can +overwrite the bytes *after* the legitimate upload and before the action runs. The `sha256` pin is +the defense that covers every backend at once: S3 signs it into the URL, so a different payload is +rejected at upload time, and redemption re-verifies the digest regardless of what the backend +checked. The tool instructs the model to pin by default; treat an unpinned upload as accepting that +window. + +### Nothing to provision, and nothing to switch on + +It is on by default. With no `storage`, the server holds the objects in memory and serves its own +upload endpoint under `/mcp/uploads`. The `fileUploads` option only *configures* that — a +backend, size limits, ttls: + +```typescript +agent.mountAiMcpServer(); // in memory, single instance +agent.mountAiMcpServer({ fileUploads: { storage } }); // a real backend +``` + +To turn the feature off, pass `fileUploads: false` — on the standalone server, +`FOREST_MCP_FILE_UPLOADS=false`. The tool is not registered, the upload endpoint is never mounted, +and `executeAction` stops mentioning either. Going through `enabledTools` would work too, but it is +an allowlist — declining this one feature that way means naming every other tool and opting out of +everything shipped after. + +> **Single instance only.** The upload and the redemption are two separate requests. With several +> replicas, in cluster mode, or on a serverless runtime, one of them lands +> on an instance that never saw the other and the action fails — intermittently, which reads as a +> flaky feature rather than a misconfiguration. Objects are also lost on restart. The server warns +> the first time an upload destination is asked for, and the failure names this cause. **Those deployments need a `storage`.** + +`ephemeralMaxTotalBytes` bounds what the in-memory store holds across all pending uploads, 64 MiB by +default. Redeeming a file does not free it — the object lives until `handleTtlSeconds` so a retry +after a failed action still finds it — so on the defaults the store holds about three max-size +files per 45-minute window rather than a rolling 64 MiB. Size it against that, or shorten +`handleTtlSeconds`. It is deliberately absolute rather than a multiple of `maxBytes`: derived, raising the +per-file limit would multiply what the process can hold. + +### With a storage backend + +Provide `storage` for anything beyond a single instance. Any backend that can pre-authorize an +upload and read the object back works — S3 presigned URLs (below), GCS signed URLs, Azure SAS. The +package has no storage dependency of its own. + +### On the standalone server + +Uploads are on with the in-memory store, with the same single-instance caveat. + +For a real backend, a storage is an object with methods, so unlike every other standalone option it +cannot travel through an environment variable. Point `FOREST_MCP_UPLOAD_STORAGE_MODULE` at a module +that default-exports the options instead — a bad path, or a module that exports nothing, fails at +startup rather than running with uploads silently disabled: + +```javascript +// forest-upload-storage.js +module.exports = { + storage: { + /* createUploadUrl / download / getSize, as below */ + }, + maxBytes: 50 * 1024 * 1024, + downloadTimeoutSeconds: 10, +}; +``` + +```bash +FOREST_MCP_UPLOAD_STORAGE_MODULE=./forest-upload-storage.js npx forest-mcp-server +``` + +The module may also export a function, sync or async, returning the same options — useful when the +backend needs credentials fetched at boot. + +### The client must be able to upload + +Step 2 is an ordinary HTTPS request, made by the client, outside the MCP protocol. The client has to +be able to make it: + +- **Claude Code** and custom agents: works. The shell runs on the same machine as the developer, so + it reaches a `localhost` agent too — this is the one place the whole flow can be tried end to end + against a local agent. Verified. +- **Claude Desktop, Claude.ai and Cowork**: the attached file lands in the code execution sandbox + and the model can `curl -X PUT -T ` — applying every header the tool returned, + since a pinned `sha256` is signed into `x-amz-checksum-sha256` on S3 and the PUT is rejected + without it. Two conditions, and both are needed: + 1. **`uploadUrl` must be publicly reachable.** That sandbox is hosted and runs on its own + network, so a `localhost` or private address is never reachable from it, whatever else is + configured. An agent running on a developer's machine cannot be tested this way. + 2. **Its host must be allowed for outbound traffic** in that sandbox. Where this is configured + differs between clients and versions — and on a managed workspace the setting may not be + exposed to the end user at all, in which case only an administrator can unblock the upload. + Do not promise your users a menu path; give them the host to get allowed. + + Both are client-side and outside this server's control, so document your upload host for your + users. + + **Verified end to end** from a Claude Desktop chat and from a Cowork cloud session: an agent + behind a public HTTPS URL, that host added to the sandbox's allowed domains, and the model's + `PUT` goes through. Before the host was allowed, the same sandbox answered `Host not in + allowlist` even for an ordinary public domain — so the allowlist is the whole of condition 2, + and satisfying it is enough. The Cowork run started from a single natural sentence, with no tool + named: the model found the form, requested a destination and pinned the `sha256` unprompted, and + no tool argument carried base64 — checked against the request bodies at the tunnel, not the + transcript. + + One wrinkle observed there: the filename the action stores is whatever the client reports, and a + sandbox may normalize it (`rapport-1815.pdf` arrived as `rapport1815.pdf` while the bytes and + mime type were exact). Treat it as a label, not an identifier. + +The tool states this prerequisite in its description and repeats it in its response, so a model +whose upload was blocked has the diagnosis in context. + +### Trying it locally + +`packages/_example` needs no configuration for this — no cloud account, no storage code. +Its `review` collection carries an `Attach a document` action with a `File` and a `FileList` field. +Start the example agent, connect an MCP client to it, and ask for that action with a file — the +action reports the name, mime type and byte count it received. + +```mermaid +sequenceDiagram + participant Client as MCP client + participant Server as MCP server + participant Storage as Storage backend + participant Agent as Forest Admin agent + + Client->>Server: requestActionFileUpload {filename, mimeType, sha256?} + Server-->>Client: uploadUrl + fileHandle (user-bound JWT) + Client->>Storage: PUT raw bytes to uploadUrl + Note over Client,Storage: bytes bypass the server and the model + Client->>Server: executeAction {values: {field: "$uploadedFile:..."}} + Server->>Storage: download object + Note over Server: verify user, TTL, maxBytes, sha256 pin + Server->>Agent: executeAction with the file + Agent-->>Server: action result + Server-->>Client: result (the model only saw the handle) +``` + +The storage backend is pluggable, and this package has no storage dependency. Provide an +implementation of `UploadStorage`; any backend that can pre-authorize an upload and read the object +back works, such as S3 presigned URLs (below), GCS signed URLs, Azure SAS, or an endpoint you serve +yourself. The only hard requirement is that the URL be reachable from the MCP client, since that is +what uploads the bytes. + +```typescript +import { + S3Client, + GetObjectCommand, + HeadObjectCommand, + PutObjectCommand, +} from '@aws-sdk/client-s3'; +import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; +import type { UploadStorage } from '@forestadmin/mcp-server'; + +const s3 = new S3Client({}); +const bucket = 'my-uploads-bucket'; + +const storage: UploadStorage = { + async createUploadUrl({ key, mimeType, sha256, expiresInSeconds }) { + const command = new PutObjectCommand({ + Bucket: bucket, + Key: key, + ContentType: mimeType, + ...(sha256 && { ChecksumSHA256: sha256 }), + }); + const url = await getSignedUrl(s3, command, { + expiresIn: expiresInSeconds, + // Load-bearing: without it the checksum is hoisted to the query string, which S3 does not + // enforce — the pin would silently stop protecting the upload. + ...(sha256 && { unhoistableHeaders: new Set(['x-amz-checksum-sha256']) }), + }); + return { + url, + headers: { 'Content-Type': mimeType, ...(sha256 && { 'x-amz-checksum-sha256': sha256 }) }, + }; + }, + async getSize(key) { + const head = await s3.send(new HeadObjectCommand({ Bucket: bucket, Key: key })); + return head.ContentLength; + }, + async download(key) { + const object = await s3.send(new GetObjectCommand({ Bucket: bucket, Key: key })); + return Buffer.from(await object.Body.transformToByteArray()); + }, +}; + +const server = new ForestMCPServer({ + // ... + fileUploads: { storage }, +}); +``` + +The other options are `keyPrefix` (default `mcp-uploads/`), `uploadUrlTtlSeconds` (default 15 min), +`handleTtlSeconds` (default 45 min, longer than the upload URL so a slow upload still leaves time to +run the action), `maxBytes` (default 20 MiB), `maxConcurrentDownloads` (default 5), and +`downloadTimeoutSeconds` (default 15 s), and `ephemeralMaxTotalBytes` (default 64 MiB, in-memory store only). + +Lower `downloadTimeoutSeconds` if the clients calling your agent cut requests sooner than that. The +whole `executeAction` has to fit inside their timeout: reading the object, encoding it, and running +your action. A read that outlives the caller is wasted work — and left unbounded it would hold its +concurrency slot after the caller gave up. + +A few properties matter in production. + +- The server stays stateless. The handle is a JWT signed with `authSecret`, so there is no database and no session affinity, and any replica can redeem a handle issued by another. +- A handle is bound to the user it was issued to. Only that user's Bearer token can redeem it, and it expires with `handleTtlSeconds`. It stays redeemable until then, so keep the TTL short. +- When the client sends `sha256` (hex or base64), the upload URL is pinned to that digest and the digest is checked again on the downloaded bytes at redemption. Content substituted after an upload URL leak cannot be redeemed. +- **`maxBytes` does not bound memory on its own.** A pre-authorized upload URL cannot always cap the object size, so the limit is enforced at redemption: before downloading when `getSize` reports a size, and only after the bytes are in memory when it returns `undefined`. Implement `getSize` whenever the backend can answer it cheaply. +- **`maxConcurrentDownloads` bounds concurrent downloads, not peak memory.** All the files one `executeAction` call references are held together until the call completes, so a form with N file fields holds up to N × `maxBytes` whatever the concurrency limit is. Size `maxBytes` against the number of file fields your actions declare. +- **A timed-out read is abandoned, not cancelled.** `UploadStorage.download` takes no `AbortSignal`, so when `downloadTimeoutSeconds` fires the concurrency slot is freed while the underlying read keeps running. Against a backend slower than that timeout the number of reads in flight can therefore exceed `maxConcurrentDownloads`. Keep `downloadTimeoutSeconds` low so a slow backend fails fast instead of accumulating. +- The server never deletes objects, and does not delete them once redeemed either: `executeAction` downloads every reference before it sets fields or runs, so consuming on read would leave a model retrying after any later failure with handles whose objects are gone, told the upload failed when it had not. Configure a lifecycle rule on the storage backend, for example deleting objects under `keyPrefix` after one day; the in-memory store reclaims on `handleTtlSeconds` and on its own total. + +Only `executeAction` resolves handles. `getActionForm` echoes field values back to the model, so a +handle stays a handle there: resolving it would put the file content back into the model's context. + +**Change hooks never see a handle.** On the `getActionForm` path, file references are withheld from +`tryToSetFields`, so a hook fired by another field reads the file field as unset rather than as a +string it would call `.buffer` on. On the `executeAction` path the handles are already resolved, so +a hook receives the file as the data uri it expects. Either way a hook never has to know this +side-channel exists. + ## API Endpoints Once running, the MCP server exposes the following endpoints: diff --git a/packages/mcp-server/src/cli.ts b/packages/mcp-server/src/cli.ts index cdfb430f62..67eb6fa05c 100644 --- a/packages/mcp-server/src/cli.ts +++ b/packages/mcp-server/src/cli.ts @@ -1,28 +1,54 @@ #!/usr/bin/env node import ForestMCPServer from './server'; +import loadFileUploads from './utils/load-file-uploads'; import parseDomainList from './utils/parse-domain-list'; import parseToolList from './utils/parse-tool-list'; const toSeconds = (value?: string) => (value === undefined ? undefined : Number(value)); -// Start the server when run directly as CLI -const server = new ForestMCPServer({ - forestServerUrl: process.env.FOREST_SERVER_URL || 'https://api.forestadmin.com', - forestAppUrl: process.env.FOREST_APP_URL || 'https://app.forestadmin.com', - envSecret: process.env.FOREST_ENV_SECRET, - authSecret: process.env.FOREST_AUTH_SECRET, - enabledTools: parseToolList(process.env.FOREST_MCP_ENABLED_TOOLS), - allowedOAuthClients: parseDomainList(process.env.FOREST_MCP_ALLOWED_OAUTH_CLIENTS), - agentUrl: process.env.FOREST_AGENT_URL, - // normalizeTokenTtl rejects NaN and non-positive values, so a bad variable fails at startup. - tokenTtl: { - accessTokenSeconds: toSeconds(process.env.FOREST_MCP_ACCESS_TOKEN_TTL_SECONDS), - refreshTokenSeconds: toSeconds(process.env.FOREST_MCP_REFRESH_TOKEN_TTL_SECONDS), - }, -}); +async function main() { + // Uploads are on by default, so this variable only means one thing: 'false' turns them off. + // Anything else fails at startup like every other option — 'FALSE' or '0' silently leaving the + // feature on is the kind of surprise an operator meets in production. + const rawFileUploadsFlag = process.env.FOREST_MCP_FILE_UPLOADS; + + if (rawFileUploadsFlag !== undefined && !['true', 'false'].includes(rawFileUploadsFlag)) { + throw new Error( + `Invalid FOREST_MCP_FILE_UPLOADS "${rawFileUploadsFlag}": use 'false' to turn action file ` + + 'uploads off. They are on by default.', + ); + } + + // Loaded before constructing, so a bad module fails at startup like every other option. Uploads + // are on without it, held in memory; this only points them at a real backend. 'false' wins over + // a configured module — an operator setting both is turning the feature off. + const fileUploads = + rawFileUploadsFlag === 'false' + ? (false as const) + : await loadFileUploads(process.env.FOREST_MCP_UPLOAD_STORAGE_MODULE); + + const server = new ForestMCPServer({ + forestServerUrl: process.env.FOREST_SERVER_URL || 'https://api.forestadmin.com', + forestAppUrl: process.env.FOREST_APP_URL || 'https://app.forestadmin.com', + envSecret: process.env.FOREST_ENV_SECRET, + authSecret: process.env.FOREST_AUTH_SECRET, + enabledTools: parseToolList(process.env.FOREST_MCP_ENABLED_TOOLS), + allowedOAuthClients: parseDomainList(process.env.FOREST_MCP_ALLOWED_OAUTH_CLIENTS), + agentUrl: process.env.FOREST_AGENT_URL, + // normalizeTokenTtl rejects NaN and non-positive values, so a bad variable fails at startup. + tokenTtl: { + accessTokenSeconds: toSeconds(process.env.FOREST_MCP_ACCESS_TOKEN_TTL_SECONDS), + refreshTokenSeconds: toSeconds(process.env.FOREST_MCP_REFRESH_TOKEN_TTL_SECONDS), + }, + // Not a truthy spread: `false` is a meaningful value and has to reach the constructor. + ...(fileUploads !== undefined && { fileUploads }), + }); + + await server.run(); +} -server.run().catch(error => { +main().catch(error => { console.error('[FATAL] Server crashed:', error); process.exit(1); }); diff --git a/packages/mcp-server/src/file-uploads/ephemeral-storage.ts b/packages/mcp-server/src/file-uploads/ephemeral-storage.ts new file mode 100644 index 0000000000..0d4d821fad --- /dev/null +++ b/packages/mcp-server/src/file-uploads/ephemeral-storage.ts @@ -0,0 +1,286 @@ +import type { UploadStorage } from './types'; +import type { Logger } from '../server'; +import type { Request, Response, Router } from 'express'; + +import express from 'express'; + +interface StoredObject { + body: Buffer; + expiresAt: number; +} + +// Issued keys are cheap next to a body — a few hundred bytes against up to maxBytes — but they +// outlive the request that asked for one, and nothing forces the caller to ever upload. This bounds +// them at about 2 MB, orders of magnitude beyond any real number of pending uploads. +const MAX_OUTSTANDING_KEYS = 10_000; + +interface EphemeralOptions { + maxBytes: number; + maxTotalBytes: number; + ttlSeconds: number; + issuedTtlSeconds: number; + publicBaseUrl: string; +} + +/** + * In-memory UploadStorage serving its own PUT endpoint on the MCP server's own origin, used when + * `fileUploads` is enabled without a storage backend. Nothing to provision — and nothing shared: + * objects live in this process only. + * + * The upload and the redemption are two separate requests, so behind several replicas, in cluster + * mode, or on a serverless runtime one lands on an instance that never saw the other. Those + * deployments need a real backend; this one is for a single instance. + */ +export default class EphemeralStorage implements UploadStorage { + private readonly objects = new Map(); + private readonly issued = new Map(); + private storedBytes = 0; + private inFlightBytes = 0; + private options!: EphemeralOptions; + private announced = false; + + constructor(private readonly logger: Logger) {} + + /** Separate from the constructor because the server only knows its own base url later. */ + configure(options: EphemeralOptions): void { + this.options = options; + } + + async createUploadUrl({ key }: { key: string }): Promise<{ url: string; method: string }> { + const { publicBaseUrl, issuedTtlSeconds } = this.options; + + // Said here rather than at startup: uploads are on by default, so a boot-time warning would + // reach every agent including those whose actions have no file field. This fires when the + // feature is actually used, which is when the limitation starts to matter. + if (!this.announced) { + this.announced = true; + this.logger( + 'Warn', + '[fileUploads] no storage backend: uploaded files are held in memory, on this instance ' + + 'only. They are lost on restart, and a deployment with several replicas or a serverless ' + + 'runtime needs a real backend on the fileUploads option.', + ); + } + + // Recorded so the endpoint only accepts keys it handed out. Without this, anything reaching the + // origin could fill the store under keys of its own and deny the feature to everyone else. + this.expire(); + + // Insertion order, so this drops the least recent pending upload. Refusing to issue instead + // would let one caller deny the feature to everyone; whoever is evicted gets a 404 telling + // them to ask again. + if (this.issued.size >= MAX_OUTSTANDING_KEYS) { + const [oldest] = this.issued.keys(); + + this.issued.delete(oldest); + this.logger( + 'Warn', + `[fileUploads] ${MAX_OUTSTANDING_KEYS} upload urls are outstanding: dropped ${oldest}, ` + + 'whose upload never came. Something is requesting destinations without uploading.', + ); + } + + this.issued.set(key, Date.now() + issuedTtlSeconds * 1000); + + return { + url: `${publicBaseUrl.replace(/\/+$/, '')}/${encodeURIComponent(key)}`, + method: 'PUT', + }; + } + + async download(key: string): Promise { + const stored = this.read(key); + + if (!stored) { + throw new Error( + 'not found in the in-memory store. Either the upload never completed — a refused one is ' + + 'answered with a 413 — or it expired after handleTtlSeconds, or it reached another ' + + 'instance: this store only holds what this instance received, so several replicas or a ' + + 'serverless runtime need a storage backend on the fileUploads option.', + ); + } + + // Deliberately NOT dropped on read. executeAction downloads every reference before it sets + // fields or runs, so any later failure — a mistyped field name, a throwing hook — would leave + // the model retrying with handles whose objects are gone, told the upload failed when it did + // not. expire() and maxTotalBytes reclaim instead. The upload url stays single-use; that is a + // different property, enforced on the issued key. + return stored.body; + } + + async getSize(key: string): Promise { + return this.read(key)?.body.length; + } + + createRouter(): Router { + const router = express.Router(); + const { logger } = this; + + // The uploads route is mounted ahead of the request logger, which needs the body parsers this + // one must precede, so each outcome is reported here instead. + const refuse = (res: Response, key: string, status: number, reason: string) => { + logger('Warn', `[fileUploads] refused ${key}: ${reason}`); + res.status(status).json({ error: reason }); + }; + + router.put('/:key', (req: Request, res: Response) => { + const { key } = req.params; + const { maxBytes, maxTotalBytes } = this.options; + + this.expire(); + + // Consumed here rather than in write(): two concurrent PUTs for one key would both pass a + // has() check and the later would silently overwrite the earlier. A failed attempt burns the + // url too, which is the honest reading of single-use — retrying needs a fresh destination. + if (!this.issued.delete(key)) { + // An object still sitting here means the url was already used, which is the case an + // integrator actually hits. Reported apart because "or it has expired" sends them looking + // at ttls instead. + if (this.objects.has(key)) { + refuse(res, key, 409, 'this upload url was already used; request a fresh one'); + } else { + refuse( + res, + key, + 404, + 'no upload was authorized for this key: it was never issued, already used, or ' + + 'expired — or it was issued by another instance, which cannot be seen from here', + ); + } + + return; + } + + // Answered before reading anything when the client declares a body that cannot fit. + const declared = Number(req.headers['content-length']); + + if (Number.isFinite(declared) && declared > maxBytes) { + refuse(res, key, 413, `the file is larger than the ${maxBytes} byte limit`); + + return; + } + + // Dropped now rather than in write(): counting bytes this upload is about to replace would + // refuse a replacement that fits, and keeping them until 'end' would hold both at once. + this.forget(key); + + // Refused before a byte is read whenever the store has no room at all, whatever this body + // turns out to be. `>=` rather than `>` so the answer is a 507 naming the store, instead of a + // 413 decided on the first chunk that reads as a problem with the file. + if (this.storedBytes + this.inFlightBytes >= maxTotalBytes) { + refuse(res, key, 507, `the in-memory store is full (${maxTotalBytes} bytes)`); + + return; + } + + const chunks: Uint8Array[] = []; + let reserved = 0; + let refused = ''; + + // Reserved as the bytes arrive, not once the body is complete: concurrent uploads would + // otherwise each weigh their own size against the same stale total and all be admitted. + const release = () => { + this.inFlightBytes -= reserved; + reserved = 0; + }; + + req.on('data', chunk => { + if (refused) return; + + const { length } = chunk as Uint8Array; + + if (reserved + length > maxBytes) { + refused = `the file is larger than the ${maxBytes} byte limit`; + } else if (this.storedBytes + this.inFlightBytes + length > maxTotalBytes) { + refused = `the in-memory store is full (${maxTotalBytes} bytes)`; + } + + if (refused) { + // Kept reading, but no longer kept: destroying the socket or answering now would reach a + // client that is still writing as a connection reset instead of a 413. + chunks.length = 0; + release(); + + return; + } + + reserved += length; + this.inFlightBytes += length; + chunks.push(chunk as Uint8Array); + }); + + req.on('end', () => { + release(); + + if (refused) { + refuse(res, key, 413, refused); + + return; + } + + // A throw here would be an uncaughtException and take the whole server down, not just this + // request: Buffer.concat allocates and can fail under memory pressure. + try { + const body = Buffer.concat(chunks); + + this.write(key, body); + logger('Debug', `[fileUploads] stored ${key} (${body.length} bytes)`); + res.status(200).end(); + } catch (error) { + logger('Error', `[fileUploads] could not store ${key}: ${(error as Error)?.message}`); + res.status(500).json({ error: 'could not store the upload' }); + } + }); + + // An aborted upload never reaches 'end', so its reservation would leak and shrink the store + // for good. Idempotent with the release above. + req.on('close', release); + + // The path a client abort takes, which is the most common real failure here — silence would + // leave a 20 MiB upload that vanished with no trace anywhere. + req.on('error', (error: Error) => { + logger( + 'Warn', + `[fileUploads] upload of ${key} failed after ${reserved} bytes: ${error.message}`, + ); + release(); + res.destroy(); + }); + }); + + return router; + } + + private write(key: string, body: Buffer): void { + this.forget(key); + this.objects.set(key, { body, expiresAt: Date.now() + this.options.ttlSeconds * 1000 }); + this.storedBytes += body.length; + } + + private read(key: string): StoredObject | undefined { + this.expire(); + + return this.objects.get(key); + } + + private forget(key: string): void { + const stored = this.objects.get(key); + + if (stored) { + this.objects.delete(key); + this.storedBytes -= stored.body.length; + } + } + + private expire(): void { + const now = Date.now(); + + this.objects.forEach((stored, key) => { + if (stored.expiresAt <= now) this.forget(key); + }); + + this.issued.forEach((expiresAt, key) => { + if (expiresAt <= now) this.issued.delete(key); + }); + } +} diff --git a/packages/mcp-server/src/file-uploads/file-reference.ts b/packages/mcp-server/src/file-uploads/file-reference.ts new file mode 100644 index 0000000000..c2427e24f2 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/file-reference.ts @@ -0,0 +1,17 @@ +/** + * Sentinel prefix used inside action form values, e.g. + * { "document": "$uploadedFile:" } + * A string rather than an object, so it passes the agent-client field validation and stays + * cheap when getActionForm echoes it back into the model's context. + */ +export const UPLOADED_FILE_PREFIX = '$uploadedFile:'; + +/** + * Isolated so that the file URIs the MCP specification is designing (SEP-2631) can be recognized + * here without touching the resolution path. + */ +export default function parseFileReference(value: unknown): string | null { + if (typeof value !== 'string' || !value.startsWith(UPLOADED_FILE_PREFIX)) return null; + + return value.slice(UPLOADED_FILE_PREFIX.length); +} diff --git a/packages/mcp-server/src/file-uploads/handles.ts b/packages/mcp-server/src/file-uploads/handles.ts new file mode 100644 index 0000000000..005d3f00d2 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/handles.ts @@ -0,0 +1,65 @@ +import jsonwebtoken from 'jsonwebtoken'; + +const HANDLE_TYPE = 'mcp-upload'; + +export interface UploadHandleClaims { + key: string; + name: string; + mimeType: string; + sha256Base64?: string; +} + +export function signUploadHandle( + claims: UploadHandleClaims & { userId: number | string }, + authSecret: string, + ttlSeconds: number, +): string { + return jsonwebtoken.sign( + { + type: HANDLE_TYPE, + key: claims.key, + name: claims.name, + mime: claims.mimeType, + uploader: String(claims.userId), + ...(claims.sha256Base64 && { sha256: claims.sha256Base64 }), + }, + authSecret, + { expiresIn: ttlSeconds }, + ); +} + +export function verifyUploadHandle( + handle: string, + userId: number | string, + authSecret: string, +): UploadHandleClaims { + const decoded = jsonwebtoken.verify(handle, authSecret, { algorithms: ['HS256'] }) as { + type?: string; + key: string; + name: string; + mime: string; + uploader?: string; + sha256?: string; + }; + + // The handle is signed with the secret that also signs access tokens, so the type claim is + // what keeps an access token from being redeemed as an upload handle. + if (decoded?.type !== HANDLE_TYPE) throw new Error('Not an upload handle'); + if (decoded.uploader !== String(userId)) throw new Error('Handle was issued to another user'); + + // key feeds storage.download and mime lands in the stored file, so neither may be undefined. + if ( + typeof decoded.key !== 'string' || + typeof decoded.name !== 'string' || + typeof decoded.mime !== 'string' + ) { + throw new Error('Malformed upload handle'); + } + + return { + key: decoded.key, + name: decoded.name, + mimeType: decoded.mime, + sha256Base64: decoded.sha256, + }; +} diff --git a/packages/mcp-server/src/file-uploads/resolve.ts b/packages/mcp-server/src/file-uploads/resolve.ts new file mode 100644 index 0000000000..3b18c36944 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/resolve.ts @@ -0,0 +1,172 @@ +import type { UploadHandleClaims } from './handles'; +import type { ResolvedFileUploads } from './types'; +import type { File } from '@forestadmin/agent-client'; +import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; + +import * as crypto from 'crypto'; + +import parseFileReference from './file-reference'; +import { verifyUploadHandle } from './handles'; + +// Keyed by reference so one used by several fields is downloaded once. The field is the first +// one that mentioned it, which is what error messages name. +function collectReferences( + values: Record, +): Map { + const references = new Map(); + + for (const [field, value] of Object.entries(values)) { + const candidates = Array.isArray(value) ? value : [value]; + + candidates.forEach(candidate => { + const handle = parseFileReference(candidate); + + if (handle && !references.has(candidate as string)) { + references.set(candidate as string, { field, handle }); + } + }); + } + + return references; +} + +// The contract puts no bound on a storage read. Without this, a backend that stops answering +// holds its concurrency slot forever while the caller's own request timeout fires, so the +// failure would be invisible here and the slots would drain away one by one. +function withTimeout(operation: string, seconds: number, promise: Promise): Promise { + let timer: NodeJS.Timeout; + + return Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`${operation} timed out after ${seconds}s`)), + seconds * 1000, + ); + }), + ]).finally(() => clearTimeout(timer)) as Promise; +} + +async function download( + field: string, + claims: UploadHandleClaims, + uploads: ResolvedFileUploads, +): Promise { + const tooLarge = (bytes: number) => + new Error( + `Field "${field}": uploaded file is ${bytes} bytes, above the ${uploads.maxBytes} byte limit`, + ); + + // A pre-authorized upload URL cannot always cap the object size, so the limit is enforced here, + // before the bytes are read whenever the backend can report a size. Wrapped like the download + // below: a backend that reports size from object metadata rejects on a missing key, which is the + // likeliest failure of the whole flow — the upload was blocked. Raw, that reaches the model as a + // bare SDK string with no field name and no diagnosis. + const size = await withTimeout( + `Field "${field}": reading the size of the uploaded file`, + uploads.downloadTimeoutSeconds, + uploads.storage.getSize(claims.key), + ).catch((error: Error) => { + throw new Error( + `Field "${field}": could not read the uploaded file. ` + + `Did the upload to uploadUrl succeed? (${error.message})`, + ); + }); + + if (typeof size === 'number' && Number.isFinite(size) && size > uploads.maxBytes) { + throw tooLarge(size); + } + + // A backend that honours the contract rejects a missing object, so this is the shape the most + // likely mistake takes: the handle was never uploaded to. The raw message alone reads as an + // infrastructure failure. The key it may contain is already in the model's context, inside the + // handle it just sent. + const buffer = await withTimeout( + `Field "${field}": reading the uploaded file`, + uploads.downloadTimeoutSeconds, + uploads.storage.download(claims.key), + ).catch((error: Error) => { + throw new Error( + `Field "${field}": could not read the uploaded file. ` + + `Did the upload to uploadUrl succeed? (${error.message})`, + ); + }); + + // Re-checked after download because getSize is advisory, and because the object can be + // replaced between the two calls. + if (buffer.length > uploads.maxBytes) throw tooLarge(buffer.length); + + // Even if the upload URL leaked and someone overwrote the object, content substituted after + // the client pinned a digest cannot be redeemed. + if (claims.sha256Base64) { + const digest = crypto.createHash('sha256').update(new Uint8Array(buffer)).digest('base64'); + + if (digest !== claims.sha256Base64) { + throw new Error(`Field "${field}": uploaded file does not match the sha256 it was pinned to`); + } + } + + return { buffer, mimeType: claims.mimeType, name: claims.name }; +} + +/** + * Only executeAction resolves references. getActionForm echoes field values back to the model, + * and a resolved file there would put the content back into the model's context. + */ +export default async function resolveUploadedFileValues( + values: Record, + authInfo: AuthInfo | undefined, + uploads: ResolvedFileUploads | undefined, +): Promise> { + const references = collectReferences(values); + + if (references.size === 0) return values; + + if (!uploads) { + throw new Error( + 'File uploads are not configured on this server. ' + + 'Ask the administrator to set the fileUploads option to enable action file fields.', + ); + } + + const userId = authInfo?.extra?.userId as number | string | undefined; + + if (userId === undefined || userId === null) { + throw new Error('Cannot resolve uploaded files without an authenticated user'); + } + + // Verified before acquiring a download slot: it is pure CPU, and it gates everything + // expensive, so a batch of forged handles is rejected instead of queueing behind the limit. + const verified = [...references].map(([reference, { field, handle }]) => { + try { + return { field, reference, claims: verifyUploadHandle(handle, userId, uploads.authSecret) }; + } catch (error) { + // Reaching the model as a bare "jwt expired" names no field, and with several file fields in + // one form it names none of them. + throw new Error( + `Field "${field}": this file handle is not usable (${(error as Error).message}). ` + + 'Call requestActionFileUpload again and upload the file to the new destination.', + ); + } + }); + + const files = new Map( + await Promise.all( + verified.map( + async ({ field, reference, claims }): Promise<[string, File]> => [ + reference, + await uploads.limitDownload(() => download(field, claims, uploads)), + ], + ), + ), + ); + + const substitute = (value: unknown) => files.get(value as string) ?? value; + + return Object.fromEntries( + Object.entries(values).map(([field, value]) => [ + field, + Array.isArray(value) ? value.map(substitute) : substitute(value), + ]), + ); +} diff --git a/packages/mcp-server/src/file-uploads/semaphore.ts b/packages/mcp-server/src/file-uploads/semaphore.ts new file mode 100644 index 0000000000..c452862a2c --- /dev/null +++ b/packages/mcp-server/src/file-uploads/semaphore.ts @@ -0,0 +1,37 @@ +export type RunExclusive = (task: () => Promise) => Promise; + +export default function createSemaphore(limit: number): RunExclusive { + let active = 0; + const queue: Array<() => void> = []; + + // The queue is deliberately unbounded: one action submits all its file references at once, so a + // cap would reject a form carrying more of them than the limit allows, every time. Each read is + // bounded by downloadTimeoutSeconds instead, which is what keeps the queue draining. + const acquire = () => + new Promise(resolve => { + if (active < limit) { + active += 1; + resolve(); + } else { + queue.push(resolve); + } + }); + + const release = () => { + const next = queue.shift(); + + // The slot transfers to the waiting task, so active stays unchanged. + if (next) next(); + else active -= 1; + }; + + return async (task: () => Promise): Promise => { + await acquire(); + + try { + return await task(); + } finally { + release(); + } + }; +} diff --git a/packages/mcp-server/src/file-uploads/types.ts b/packages/mcp-server/src/file-uploads/types.ts new file mode 100644 index 0000000000..fd09548ad8 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/types.ts @@ -0,0 +1,146 @@ +import type { RunExclusive } from './semaphore'; +import type { Logger } from '../server'; + +import createSemaphore from './semaphore'; + +/** Storage backend for the action file upload side-channel. See the README for the flow. */ +export interface UploadStorage { + /** The URL must be reachable by the MCP client, which is what uploads the bytes. */ + createUploadUrl(params: { + key: string; + mimeType: string; + /** Advisory: the server re-verifies the digest after download regardless. */ + sha256?: string; + expiresInSeconds: number; + }): Promise<{ url: string; method?: string; headers?: Record }>; + + /** Must reject when the object does not exist. */ + download(key: string): Promise; + + /** + * Size of the uploaded object, used to reject oversized uploads before downloading them. + * Return undefined when the backend cannot report it cheaply — `maxBytes` is then only + * enforced after the bytes are in memory, so the process holds the whole object either way. + * + * Rejecting is fine when the object is absent — reading its metadata is how most backends + * answer this, and a missing object is the likeliest outcome of the whole flow, the upload + * having been blocked. The rejection is reported to the caller with the field name and the + * message, so it does not have to be distinguished from an unreadable one here. + */ + getSize(key: string): Promise; +} + +/** + * @experimental The MCP specification is still designing its own file transfer story + * (SEP-2631). The storage contract is expected to survive, but the `requestActionFileUpload` tool + * and the handle format may change to follow the specification once it lands. + */ +export interface FileUploadsOptions { + /** + * Where uploaded objects live between the upload and the action. Omit it and the server keeps + * them in memory, on its own origin — nothing to provision, but only correct for a single + * instance: see the README. + */ + storage?: UploadStorage; + keyPrefix?: string; + uploadUrlTtlSeconds?: number; + /** Must stay longer than uploadUrlTtlSeconds, so a slow upload leaves time to run the action. */ + handleTtlSeconds?: number; + maxBytes?: number; + maxConcurrentDownloads?: number; + /** + * Total size the in-memory store holds across all pending uploads. Defaults to 64 MiB. Has no + * effect once a `storage` is given. + */ + ephemeralMaxTotalBytes?: number; + /** + * How long a single storage read may take. Keep it well under the request timeout of the + * clients calling the agent, so a slow backend fails here rather than being cut mid-flight. + * Defaults to 15 seconds. + */ + downloadTimeoutSeconds?: number; +} + +export interface ResolvedFileUploads { + storage: UploadStorage; + keyPrefix: string; + uploadUrlTtlSeconds: number; + handleTtlSeconds: number; + maxBytes: number; + downloadTimeoutSeconds: number; + ephemeralMaxTotalBytes: number; + authSecret: string; + limitDownload: RunExclusive; +} + +const DEFAULT_KEY_PREFIX = 'mcp-uploads/'; +const DEFAULT_UPLOAD_URL_TTL_SECONDS = 15 * 60; +const DEFAULT_HANDLE_TTL_SECONDS = 45 * 60; +const DEFAULT_MAX_BYTES = 20 * 1024 * 1024; +const DEFAULT_MAX_CONCURRENT_DOWNLOADS = 5; +const DEFAULT_DOWNLOAD_TIMEOUT_SECONDS = 15; +// Deliberately absolute rather than a multiple of maxBytes: derived, raising the per-file limit +// would multiply what the process can hold, which is the opposite of what setting it suggests. +export const DEFAULT_EPHEMERAL_MAX_TOTAL_BYTES = 64 * 1024 * 1024; + +function positiveInteger( + field: keyof FileUploadsOptions, + value: number | undefined, +): number | undefined { + if (value === undefined) return undefined; + + if (!Number.isInteger(value) || value <= 0) { + throw new Error(`Invalid fileUploads.${field} "${value}": it must be a positive integer.`); + } + + return value; +} + +export function resolveFileUploads( + options: FileUploadsOptions | undefined, + authSecret: string, + logger?: Logger, +): ResolvedFileUploads | undefined { + if (!options) return undefined; + + if (!options.storage) throw new Error('fileUploads.storage is required.'); + + const uploadUrlTtlSeconds = + positiveInteger('uploadUrlTtlSeconds', options.uploadUrlTtlSeconds) ?? + DEFAULT_UPLOAD_URL_TTL_SECONDS; + const handleTtlSeconds = + positiveInteger('handleTtlSeconds', options.handleTtlSeconds) ?? DEFAULT_HANDLE_TTL_SECONDS; + + const maxBytes = positiveInteger('maxBytes', options.maxBytes) ?? DEFAULT_MAX_BYTES; + const ephemeralMaxTotalBytes = + positiveInteger('ephemeralMaxTotalBytes', options.ephemeralMaxTotalBytes) ?? + DEFAULT_EPHEMERAL_MAX_TOTAL_BYTES; + + // A handle expiring before its upload URL means uploads succeed and every redemption then + // fails with a bare "jwt expired". + if (handleTtlSeconds <= uploadUrlTtlSeconds) { + logger?.( + 'Warn', + `fileUploads.handleTtlSeconds=${handleTtlSeconds} is shorter than ` + + `fileUploads.uploadUrlTtlSeconds=${uploadUrlTtlSeconds}: a slow upload will finish with ` + + 'an already expired handle.', + ); + } + + return { + storage: options.storage, + keyPrefix: options.keyPrefix ?? DEFAULT_KEY_PREFIX, + uploadUrlTtlSeconds, + handleTtlSeconds, + maxBytes, + downloadTimeoutSeconds: + positiveInteger('downloadTimeoutSeconds', options.downloadTimeoutSeconds) ?? + DEFAULT_DOWNLOAD_TIMEOUT_SECONDS, + ephemeralMaxTotalBytes, + authSecret, + limitDownload: createSemaphore( + positiveInteger('maxConcurrentDownloads', options.maxConcurrentDownloads) ?? + DEFAULT_MAX_CONCURRENT_DOWNLOADS, + ), + }; +} diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index 164da65311..1503b74b50 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -2,6 +2,7 @@ export { default as ForestMCPServer } from './server'; export type { ForestMCPServerOptions, HttpCallback, ToolName } from './server'; export type { TokenTtlOptions } from './utils/token-ttl'; +export type { FileUploadsOptions, UploadStorage } from './file-uploads/types'; export type { InProcessAgentDispatcher, InProcessDispatchRequest, diff --git a/packages/mcp-server/src/server.ts b/packages/mcp-server/src/server.ts index dab57f6252..d4c7bf9455 100644 --- a/packages/mcp-server/src/server.ts +++ b/packages/mcp-server/src/server.ts @@ -2,6 +2,7 @@ // This ensures URL.canParse is available for MCP SDK's Zod validation import './polyfills'; +import type { FileUploadsOptions, ResolvedFileUploads } from './file-uploads/types'; import type { ForestServerClient } from './http-client'; import type { InProcessAgentDispatcher } from './in-process-agent-dispatcher'; import type { ToolContext } from './tool-context'; @@ -23,6 +24,8 @@ import cors from 'cors'; import express from 'express'; import * as http from 'http'; +import EphemeralStorage from './file-uploads/ephemeral-storage'; +import { resolveFileUploads } from './file-uploads/types'; import ForestOAuthProvider from './forest-oauth-provider'; import { createForestServerClient } from './http-client'; import { makeIsMcpRoute, normalizeMountPath } from './mcp-paths'; @@ -35,6 +38,7 @@ import declareExecuteActionTool from './tools/execute-action'; import declareGetActionFormTool from './tools/get-action-form'; import declareListTool from './tools/list'; import declareListRelatedTool from './tools/list-related'; +import declareRequestActionFileUploadTool from './tools/request-action-file-upload'; import declareUpdateTool from './tools/update'; import normalizeAgentUrl from './utils/normalize-agent-url'; import normalizeDomainList from './utils/normalize-domain-list'; @@ -90,6 +94,7 @@ const SAFE_ARGUMENTS_FOR_LOGGING: Record = { describeCollection: ['collectionName'], getActionForm: ['collectionName', 'actionName', 'recordIds'], executeAction: ['collectionName', 'actionName', 'recordIds'], + requestActionFileUpload: ['mimeType'], associate: ['collectionName', 'relationName', 'parentRecordId', 'targetRecordId'], dissociate: ['collectionName', 'relationName', 'parentRecordId', 'targetRecordIds'], }; @@ -104,7 +109,8 @@ export type ToolName = | 'associate' | 'dissociate' | 'getActionForm' - | 'executeAction'; + | 'executeAction' + | 'requestActionFileUpload'; /** * Options for configuring the Forest Admin MCP Server @@ -154,6 +160,16 @@ export interface ForestMCPServerOptions { * Omit to accept any dynamically registered client. */ allowedOAuthClients?: string[]; + /** + * Action file uploads are on by default, with the objects held in memory. Pass an object to + * configure them — a `storage` backend, size limits, ttls — or `false` to turn the feature off: + * no `requestActionFileUpload` tool, no upload endpoint, and `executeAction` stops mentioning + * either. See the README for the flow and the storage contract. + * + * @experimental Expected to change to follow the MCP file transfer specification once it + * lands (SEP-2631). + */ + fileUploads?: false | FileUploadsOptions; } /** @@ -180,6 +196,9 @@ export default class ForestMCPServer { private agentDispatcher?: InProcessAgentDispatcher; private tokenTtl?: TokenTtlOptions; private allowedOAuthClients?: string[]; + private fileUploadsOptions?: FileUploadsOptions; + private fileUploads?: ResolvedFileUploads; + private ephemeralStorage?: EphemeralStorage; constructor(options?: ForestMCPServerOptions) { this.forestServerUrl = options?.forestServerUrl || 'https://api.forestadmin.com'; @@ -194,6 +213,26 @@ export default class ForestMCPServer { this.tokenTtl = normalizeTokenTtl(options?.tokenTtl, this.logger); this.allowedOAuthClients = normalizeDomainList(options?.allowedOAuthClients); + // Resolved in buildExpressApp, where the auth secret is known to be set. + this.fileUploadsOptions = options?.fileUploads || undefined; + + // `enabledTools` is an allowlist, so declining this one feature through it would mean naming + // every other tool and opting out of everything shipped later. Dropping it from the set here + // turns the whole feature off through the machinery that already gates it: registration, the + // upload endpoint, and the paragraph executeAction adds for it. + if (options?.fileUploads === false) { + // Said aloud when the two options contradict each other: resolveEnabledTools logs every + // other enablement surprise, and a tool the caller named vanishing without a line in the + // log reads as a bug in whichever config layer loses. + if (options.enabledTools?.includes('requestActionFileUpload')) { + this.logger( + 'Warn', + 'fileUploads: false removes requestActionFileUpload even though enabledTools lists it.', + ); + } + + this.enabledTools.delete('requestActionFileUpload'); + } // Use injected forestServerClient or create default this.forestServerClient = options?.forestServerClient ?? this.createDefaultForestServerClient(); @@ -230,6 +269,7 @@ export default class ForestMCPServer { logger: this.logger, collectionNames: this.collectionNames, agentDispatcher: this.agentDispatcher, + fileUploads: this.fileUploads, }; const allTools: Array<{ name: ToolName; register: () => string }> = [ @@ -243,6 +283,14 @@ export default class ForestMCPServer { { name: 'dissociate', register: () => declareDissociateTool(mcpServer, ctx) }, { name: 'getActionForm', register: () => declareGetActionFormTool(mcpServer, ctx) }, { name: 'executeAction', register: () => declareExecuteActionTool(mcpServer, ctx) }, + ...(this.fileUploads + ? [ + { + name: 'requestActionFileUpload' as const, + register: () => declareRequestActionFileUploadTool(mcpServer, ctx), + }, + ] + : []), ]; const enabledToolEntries = allTools.filter(tool => this.enabledTools.has(tool.name)); @@ -280,6 +328,7 @@ export default class ForestMCPServer { 'dissociate', 'getActionForm', 'executeAction', + 'requestActionFileUpload', ]; const enabled = new Set(options?.enabledTools ?? allToolNames); @@ -428,6 +477,46 @@ export default class ForestMCPServer { async buildExpressApp(baseUrl?: URL): Promise { const { envSecret, authSecret } = this.ensureSecretsAreSet(); + // On unless requestActionFileUpload is absent from enabledTools — dropped by the caller, or by + // the fileUploads: false branch in the constructor, which lands in the same set. Gating on + // that keeps executeAction from advertising an upload tool the server never registered. + // `!this.fileUploads` because an agent rebuilds its router on every customization refresh: + // re-initializing would hand the new app a different store than the urls already in flight. + if (this.enabledTools.has('requestActionFileUpload') && !this.fileUploads) { + // No backend given: hold the objects here. Correct for one instance only, which + // EphemeralStorage announces the first time something actually asks for a destination — + // warning at boot would tax every agent, including those with no file field at all. + if (!this.fileUploadsOptions?.storage) { + this.ephemeralStorage = new EphemeralStorage(this.logger); + } + + this.fileUploads = resolveFileUploads( + { + ...this.fileUploadsOptions, + ...(this.ephemeralStorage && { storage: this.ephemeralStorage }), + }, + authSecret, + this.logger, + ); + + // Said here rather than in resolveFileUploads, which cannot tell the in-memory store from a + // provided one: the tool reports maxBytes to the model verbatim, so a value above what the + // whole store holds promises a size every upload of which is refused — with "the store is + // full", on an empty store. + if ( + this.ephemeralStorage && + this.fileUploads.maxBytes > this.fileUploads.ephemeralMaxTotalBytes + ) { + this.logger( + 'Warn', + `fileUploads.maxBytes=${this.fileUploads.maxBytes} exceeds what the in-memory store ` + + `holds in total (${this.fileUploads.ephemeralMaxTotalBytes} bytes, ` + + 'ephemeralMaxTotalBytes), so an upload that large is always refused. Raise ' + + 'ephemeralMaxTotalBytes too, or configure a storage backend.', + ); + } + } + await this.fetchCollectionNames(); const app = express(); @@ -504,6 +593,22 @@ export default class ForestMCPServer { // Body parsers MUST come before OAuth handlers because the token handler // expects req.body to be parsed. When proxied from Koa, the body is already // available but Express needs to see it properly. + // Ahead of the body parsers: they would consume the stream for any content type they + // claim, and this endpoint needs the raw body. Also ahead of allowedMethods(['POST']). + if (this.ephemeralStorage && this.fileUploads) { + const uploadsPath = `${prefix}/mcp/uploads`; + + this.ephemeralStorage.configure({ + maxBytes: this.fileUploads.maxBytes, + maxTotalBytes: this.fileUploads.ephemeralMaxTotalBytes, + ttlSeconds: this.fileUploads.handleTtlSeconds, + issuedTtlSeconds: this.fileUploads.uploadUrlTtlSeconds, + publicBaseUrl: new URL(uploadsPath, effectiveBaseUrl).href, + }); + + app.use(uploadsPath, this.ephemeralStorage.createRouter()); + } + app.use(express.json()); app.use(express.urlencoded({ extended: true })); @@ -559,15 +664,17 @@ export default class ForestMCPServer { app.use(allowedMethods(['POST'])); + const resourceMetadataUrl = new URL( + `/.well-known/oauth-protected-resource${mcpResourceUrl.pathname}`, + effectiveBaseUrl, + ).href; + app.post( `${prefix}/mcp`, requireBearerAuth({ verifier: oauthProvider, requiredScopes: ['mcp:read'], - resourceMetadataUrl: new URL( - `/.well-known/oauth-protected-resource${mcpResourceUrl.pathname}`, - effectiveBaseUrl, - ).href, + resourceMetadataUrl, }), (req, res) => { this.handleMcpRequest(req, res).catch(error => { diff --git a/packages/mcp-server/src/tool-context.ts b/packages/mcp-server/src/tool-context.ts index 2b958d0e14..ce6a1e27f1 100644 --- a/packages/mcp-server/src/tool-context.ts +++ b/packages/mcp-server/src/tool-context.ts @@ -1,3 +1,4 @@ +import type { ResolvedFileUploads } from './file-uploads/types'; import type { ForestServerClient } from './http-client'; import type { InProcessAgentDispatcher } from './in-process-agent-dispatcher'; import type { Logger } from './server'; @@ -7,4 +8,5 @@ export interface ToolContext { logger: Logger; collectionNames: string[]; agentDispatcher?: InProcessAgentDispatcher; + fileUploads?: ResolvedFileUploads; } diff --git a/packages/mcp-server/src/tools/execute-action.ts b/packages/mcp-server/src/tools/execute-action.ts index a587859474..4fec95d218 100644 --- a/packages/mcp-server/src/tools/execute-action.ts +++ b/packages/mcp-server/src/tools/execute-action.ts @@ -3,6 +3,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; +import resolveUploadedFileValues from '../file-uploads/resolve'; import { createActionArgumentShape } from '../utils/action-helpers'; import { buildClientWithActions } from '../utils/agent-caller'; import registerToolWithLogging from '../utils/tool-with-logging'; @@ -41,7 +42,13 @@ Required workflow: 2. If getActionForm returns "canExecute": false, call it again with values until "canExecute": true 3. Only then call executeAction with the same values used in the last getActionForm call -If you call executeAction with missing required fields, it will return an error with the missing fields instead of executing the action.`, +If you call executeAction with missing required fields, it will return an error with the missing fields instead of executing the action.${ + ctx.fileUploads + ? ` + +To fill a file field, never inline base64 file content. Call requestActionFileUpload to get an upload destination, upload the raw bytes there, and pass the returned fileHandle string as the field value.` + : '' + }`, inputSchema: argumentShape, }, async (options: ExecuteActionArgument, extra) => { @@ -65,12 +72,16 @@ If you call executeAction with missing required fields, it will return an error }, logger, operation: async () => { + const values = options.values + ? await resolveUploadedFileValues(options.values, extra.authInfo, ctx.fileUploads) + : undefined; + const action = await rpcClient .collection(options.collectionName) .action(options.actionName, { recordIds }); - if (options.values) { - await action.setFields(options.values); + if (values) { + await action.setFields(values); } const result = await action.execute({ approvalRequestMessage: options.reasoning }); diff --git a/packages/mcp-server/src/tools/get-action-form.ts b/packages/mcp-server/src/tools/get-action-form.ts index 656b0fafc3..f47c269ee8 100644 --- a/packages/mcp-server/src/tools/get-action-form.ts +++ b/packages/mcp-server/src/tools/get-action-form.ts @@ -1,6 +1,7 @@ import type { ToolContext } from '../tool-context'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import parseFileReference from '../file-uploads/file-reference'; import { createActionArgumentShape } from '../utils/action-helpers'; import { buildClientWithActions } from '../utils/agent-caller'; import registerToolWithLogging from '../utils/tool-with-logging'; @@ -23,6 +24,28 @@ function toAllowedValue(option: unknown): { value: string | number | null; label return { value: option as string | number, label: String(option) }; } +// Setting a field that declares a change hook posts every field value to the agent, and a change +// hook reading `.buffer` off a handle string throws — a 500 on the very sequence executeAction's +// description prescribes. Handles are not resolved on this path on purpose (the bytes would land +// back in the model's context), so they are kept away from the agent and echoed back to the model +// instead: a required file field stays satisfiable, and the model sees its handle was received. +function splitFileReferences(values: Record): { + forAgent: Record; + withheld: Record; +} { + const forAgent: Record = {}; + const withheld: Record = {}; + + for (const [field, value] of Object.entries(values)) { + const candidates = Array.isArray(value) ? value : [value]; + + if (candidates.some(candidate => parseFileReference(candidate))) withheld[field] = value; + else forAgent[field] = value; + } + + return { forAgent, withheld }; +} + export default function declareGetActionFormTool(mcpServer: McpServer, ctx: ToolContext): string { const { forestServerClient, logger, collectionNames } = ctx; const argumentShape = createActionArgumentShape(collectionNames); @@ -64,16 +87,31 @@ The response includes: .action(options.actionName, { recordIds }); let skippedFields: string[] = []; + let withheld: Record = {}; if (options.values) { - skippedFields = await action.tryToSetFields(options.values); + const split = splitFileReferences(options.values); + + withheld = split.withheld; + skippedFields = await action.tryToSetFields(split.forAgent); } const fields = action.getFields(); + // A withheld handle satisfies its field: the agent never saw it, so getValue() is undefined, + // and counting it as missing would leave canExecute false with nothing the model could send + // to fix it — a data uri is what it is told never to send, and the handle is what it just + // sent. + // An own-property check, not `in`: withheld is keyed by model-sent field names, and `in` + // walks the prototype chain — a field literally named toString would read as filled by a + // function. hasOwnProperty.call because this package's lib predates Object.hasOwn. + const valueOf = (field: { getName(): string; getValue(): unknown }) => + Object.prototype.hasOwnProperty.call(withheld, field.getName()) + ? withheld[field.getName()] + : field.getValue(); const requiredFields = fields .filter(field => field.isRequired()) - .filter(field => field.getValue() === undefined || field.getValue() === null) + .filter(field => valueOf(field) === undefined || valueOf(field) === null) .map(field => field.getName()); const canExecute = requiredFields.length === 0; @@ -87,13 +125,13 @@ The response includes: const description = field.getPlainField()?.description; const baseField = { name: field.getName(), - type: field.getType(), - value: field.getValue(), + type: field.getTypeName(), + value: valueOf(field), isRequired: field.isRequired() ?? false, ...(description ? { description } : {}), }; - if (field.getType() === 'Enum') { + if (field.getTypeName() === 'Enum') { const enumField = action.getEnumField(field.getName()); return { ...baseField, enumValues: enumField.getOptions() ?? null }; diff --git a/packages/mcp-server/src/tools/request-action-file-upload.ts b/packages/mcp-server/src/tools/request-action-file-upload.ts new file mode 100644 index 0000000000..d249fdb7de --- /dev/null +++ b/packages/mcp-server/src/tools/request-action-file-upload.ts @@ -0,0 +1,165 @@ +import type { ToolContext } from '../tool-context'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; + +import * as crypto from 'crypto'; +import { z } from 'zod'; + +import { UPLOADED_FILE_PREFIX } from '../file-uploads/file-reference'; +import { signUploadHandle } from '../file-uploads/handles'; +import registerToolWithLogging from '../utils/tool-with-logging'; + +const MIME_TYPE_PATTERN = /^[\w.+-]+\/[\w.+-]+$/; +const SHA256_HEX_PATTERN = /^[0-9a-f]{64}$/i; +const SHA256_BASE64_PATTERN = /^[A-Za-z0-9+/]{43}=$/; +const MAX_FILENAME_LENGTH = 128; +const REQUIRED_SCOPE = 'mcp:action'; + +// Names the requirement rather than a menu path: the setting that satisfies it is not in the same +// place in every client, and on a managed workspace the end user may have no access to it at all. +const UPLOAD_PREREQUISITE = + 'Uploading requires an outbound HTTP request from your environment. A hosted code execution ' + + 'sandbox runs on its own network and reaches only allowed hosts, so the host of uploadUrl must ' + + 'be publicly reachable — a localhost or private address never is from there — and allowed for ' + + 'outbound traffic in that environment. Where that is configured depends on the client, and on a ' + + 'managed workspace it may be an administrator setting rather than a user one. A blocked or ' + + 'unreachable request is a client configuration issue, not an expired handle: report it as such ' + + 'instead of requesting another destination.'; + +interface RequestActionFileUploadArgument { + filename: string; + mimeType: string; + sha256?: string; +} + +// The storage key delimits segments with '/', so keep a conservative charset. Truncation keeps +// the tail so the extension survives. A name made only of dots would let a backend that joins +// paths climb out of the per-upload directory. +function sanitizeFilename(filename: string): string { + const safe = filename + .trim() + .slice(-MAX_FILENAME_LENGTH) + // Unicode-aware: ASCII \w turns `facture-été.pdf` into `facture-_t_.pdf`, so the accented + // letters vanish while the spaces and parentheses survive. Same key safety, no `/`. + .replace(/[^\p{L}\p{N}._\- ()]/gu, '_'); + + return !safe || /^\.+$/.test(safe) ? 'file' : safe; +} + +function normalizeSha256(sha256: string | undefined): string | undefined { + if (!sha256) return undefined; + if (SHA256_BASE64_PATTERN.test(sha256)) return sha256; + if (SHA256_HEX_PATTERN.test(sha256)) return Buffer.from(sha256, 'hex').toString('base64'); + + throw new Error('sha256 must be the file digest as hex or base64.'); +} + +export default function declareRequestActionFileUploadTool( + mcpServer: McpServer, + ctx: ToolContext, +): string { + const { logger, fileUploads } = ctx; + + return registerToolWithLogging( + mcpServer, + 'requestActionFileUpload', + { + title: 'Request an upload destination for an action file field', + description: `Get a destination to upload a file to, then reference it in an action form field. + +Call this whenever getActionForm shows a field of type "File" or "FileList". Never inline base64 file content as a field value: it would be far larger than any payload limit. + +Workflow: +1. Compute the file's sha256 and call this tool with the filename, mimeType and sha256. The digest pins the destination to that exact content — without it, on some backends anyone who reads the upload url in a log can replace the bytes before the action runs. Skip it only when you cannot compute a digest. +2. Upload the raw bytes to the returned uploadUrl, with the returned method and every returned header. Some backends sign a pinned sha256 into a checksum header and reject the upload without it, so apply the headers as returned rather than assuming which ones matter. A pinned digest is re-verified when the action runs either way. The bytes must not pass through this tool or through your own output. +3. Pass the returned fileHandle string as the value of the file field in executeAction. + +${UPLOAD_PREREQUISITE} + +The url accepts one upload: to send different bytes, call this tool again for a fresh destination. The handle expires too, so run the upload and the action without a long pause in between.`, + inputSchema: { + filename: z + .string() + .trim() + .min(1) + .describe('Original file name, e.g. "invoice-2026-01.pdf".'), + mimeType: z.string().describe('Media type of the file, e.g. "application/pdf".'), + sha256: z + .string() + .optional() + .describe( + 'Sha256 digest of the file, hex or base64. Pins the destination to that exact ' + + 'content; compute it and pass it whenever you can.', + ), + }, + }, + async (options: RequestActionFileUploadArgument, extra) => { + if (!fileUploads) { + throw new Error( + 'File uploads are not configured on this server. ' + + 'Ask the administrator to set the fileUploads option to enable action file fields.', + ); + } + + const userId = extra.authInfo?.extra?.userId as number | string | undefined; + + if (userId === undefined || userId === null) { + throw new Error('Cannot request a file upload without an authenticated user'); + } + + // /mcp only requires mcp:read, but this mints a pre-authorized write into the host's + // storage. The route this tool replaced enforced mcp:action, so it is enforced here. + if (!extra.authInfo?.scopes?.includes(REQUIRED_SCOPE)) { + throw new Error(`Requesting a file upload requires the "${REQUIRED_SCOPE}" scope.`); + } + + if (!MIME_TYPE_PATTERN.test(options.mimeType)) { + throw new Error( + `"${options.mimeType}" is not a media type. Expected a type/subtype pair, e.g. application/pdf.`, + ); + } + + const sha256Base64 = normalizeSha256(options.sha256); + const safeName = sanitizeFilename(options.filename); + const key = `${fileUploads.keyPrefix}${crypto.randomUUID()}/${safeName}`; + + const destination = await fileUploads.storage.createUploadUrl({ + key, + mimeType: options.mimeType, + ...(sha256Base64 && { sha256: sha256Base64 }), + expiresInSeconds: fileUploads.uploadUrlTtlSeconds, + }); + + const handle = signUploadHandle( + { + key, + name: safeName, + mimeType: options.mimeType, + userId, + ...(sha256Base64 && { sha256Base64 }), + }, + fileUploads.authSecret, + fileUploads.handleTtlSeconds, + ); + + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify({ + uploadUrl: destination.url, + method: destination.method ?? 'PUT', + headers: destination.headers ?? { 'Content-Type': options.mimeType }, + expiresInSeconds: fileUploads.uploadUrlTtlSeconds, + maxBytes: fileUploads.maxBytes, + fileHandle: `${UPLOADED_FILE_PREFIX}${handle}`, + // Repeated in the response so the model has the diagnosis in context at the moment + // its upload fails, not only when it first read the tool description. + prerequisite: UPLOAD_PREREQUISITE, + }), + }, + ], + }; + }, + logger, + ); +} diff --git a/packages/mcp-server/src/utils/load-file-uploads.ts b/packages/mcp-server/src/utils/load-file-uploads.ts new file mode 100644 index 0000000000..97507f0db6 --- /dev/null +++ b/packages/mcp-server/src/utils/load-file-uploads.ts @@ -0,0 +1,98 @@ +import type { FileUploadsOptions } from '../file-uploads/types'; + +import * as path from 'path'; + +// A module is free to throw a non-Error, and reading .message off it yields undefined. +const describe = (error: unknown) => (error instanceof Error ? error.message : String(error)); + +// Attached rather than passed to the constructor: the error cause option needs a newer lib than +// this package targets, while Node itself carries the property fine. +const withCause = (message: string, cause: unknown) => Object.assign(new Error(message), { cause }); + +/** + * Loads the `fileUploads` options of the standalone server from a module on disk. + * + * A storage backend is an object with methods, so it cannot travel through an environment + * variable like every other standalone option. Without this, file uploads would be reachable + * only by embedding the server in an agent or by writing a custom entry point. + * + * The module default-exports the options, or a function returning them: + * + * module.exports = { storage: myS3Storage, maxBytes: 50 * 1024 * 1024 }; + */ +export default async function loadFileUploads( + modulePath?: string, +): Promise { + if (!modulePath) return undefined; + + const resolved = path.resolve(process.cwd(), modulePath); + let loaded: { default?: unknown } & Record; + + try { + // require rather than import(): this package is CommonJS, and Node handles an ESM module here + // too, whereas a file:// specifier breaks under the test runner. + // eslint-disable-next-line global-require, import/no-dynamic-require, @typescript-eslint/no-var-requires + loaded = require(resolved); + } catch (error) { + // "cannot find it" and "it ran and failed" send the operator to different places, and only the + // first is about the path they configured. + const absent = + (error as NodeJS.ErrnoException)?.code === 'MODULE_NOT_FOUND' && + String((error as Error)?.message).includes(resolved); + + throw withCause( + absent + ? `FOREST_MCP_UPLOAD_STORAGE_MODULE "${modulePath}" was not found (resolved to ${resolved}).` + : `FOREST_MCP_UPLOAD_STORAGE_MODULE "${modulePath}" failed while loading: ${describe( + error, + )}`, + error, + ); + } + + const exported = loaded?.default ?? loaded; + let options: FileUploadsOptions | undefined; + + try { + options = (typeof exported === 'function' ? await exported() : exported) as FileUploadsOptions; + } catch (error) { + // The sync path above is wrapped; a rejecting factory would otherwise surface with no mention + // of the module it came from. + throw withCause( + `FOREST_MCP_UPLOAD_STORAGE_MODULE "${modulePath}" failed while building the options: ${describe( + error, + )}`, + error, + ); + } + + // An object without a storage key is legitimate — it selects the in-memory store. Nothing at all + // is a mistake, most likely a module that forgot to export. + if (!options || typeof options !== 'object') { + throw new Error( + `FOREST_MCP_UPLOAD_STORAGE_MODULE "${modulePath}" must export the fileUploads options, or a ` + + 'function returning them. See the fileUploads section of the mcp-server README.', + ); + } + + // Whoever sets this variable did it *for* the storage: a bad import or a factory that returned + // early leaves `{ storage: undefined }`, which the server would silently replace with the + // in-memory store — putting a deliberately replicated deployment on the one backend that cannot + // serve it. A missing method fails later, on the first user's upload, so it is checked here too. + if ('storage' in options) { + const storage = options.storage as unknown as Record | undefined; + const missing = ['createUploadUrl', 'download', 'getSize'].filter( + method => typeof storage?.[method] !== 'function', + ); + + if (missing.length) { + throw new Error( + `FOREST_MCP_UPLOAD_STORAGE_MODULE "${modulePath}" exports a storage missing ` + + `${missing.join(', ')}. An UploadStorage implements createUploadUrl, download and ` + + 'getSize. Omit the storage entirely to use the in-memory store on purpose.', + ); + } + } + + return options; +} diff --git a/packages/mcp-server/test/file-uploads/ephemeral-storage.test.ts b/packages/mcp-server/test/file-uploads/ephemeral-storage.test.ts new file mode 100644 index 0000000000..15bac672e2 --- /dev/null +++ b/packages/mcp-server/test/file-uploads/ephemeral-storage.test.ts @@ -0,0 +1,332 @@ +import type { Logger } from '../../src/server'; +import type { Express } from 'express'; + +import express from 'express'; +import * as http from 'http'; +import request from 'supertest'; + +import EphemeralStorage from '../../src/file-uploads/ephemeral-storage'; + +describe('EphemeralStorage', () => { + let storage: EphemeralStorage; + let app: Express; + let logger: jest.MockedFunction; + + const configure = (options: Partial[0]> = {}) => { + storage.configure({ + maxBytes: 1024, + maxTotalBytes: 4096, + ttlSeconds: 60, + issuedTtlSeconds: 900, + publicBaseUrl: 'https://agent.example/mcp/uploads', + ...options, + }); + }; + + const put = async (key: string, body: string | Buffer) => { + await storage.createUploadUrl({ key }); + + return request(app) + .put(`/${encodeURIComponent(key)}`) + .send(body); + }; + + beforeEach(() => { + jest.useRealTimers(); + logger = jest.fn(); + storage = new EphemeralStorage(logger); + configure(); + + app = express(); + app.use('/', storage.createRouter()); + }); + + describe('createUploadUrl', () => { + // An issued key outlives the request that asked for one, and nothing obliges the caller to + // upload. Cheap next to a body, but unbounded is unbounded. + it('drops the least recent pending upload past its outstanding cap', async () => { + for (let i = 0; i < 10_000; i += 1) { + // eslint-disable-next-line no-await-in-loop + await storage.createUploadUrl({ key: `k${i}` }); + } + + await storage.createUploadUrl({ key: 'one-too-many' }); + + expect(logger).toHaveBeenCalledWith('Warn', expect.stringContaining('dropped k0')); + await expect(request(app).put('/k0').send('x')).resolves.toMatchObject({ status: 404 }); + await expect(request(app).put('/k1').send('x')).resolves.toMatchObject({ status: 200 }); + }); + + it('points at the configured origin, with the key encoded as one segment', async () => { + const { url, method } = await storage.createUploadUrl({ key: 'mcp-uploads/uuid/a b.pdf' }); + + expect(url).toBe('https://agent.example/mcp/uploads/mcp-uploads%2Fuuid%2Fa%20b.pdf'); + expect(method).toBe('PUT'); + }); + + it('does not double the separator when the base url ends with one', async () => { + configure({ publicBaseUrl: 'https://agent.example/mcp/uploads/' }); + + const { url } = await storage.createUploadUrl({ key: 'k' }); + + expect(url).toBe('https://agent.example/mcp/uploads/k'); + }); + }); + + describe('round trip', () => { + it('returns the exact bytes that were uploaded', async () => { + const body = Buffer.from('CONTENU-BINAIRE-\0ÿ-avec des accents'); + + await expect(put('mcp-uploads/uuid/rapport final;v2.pdf', body)).resolves.toMatchObject({ + status: 200, + }); + + const key = 'mcp-uploads/uuid/rapport final;v2.pdf'; + await expect(storage.getSize(key)).resolves.toBe(body.length); + await expect(storage.download(key)).resolves.toEqual(body); + + // Kept, not consumed: executeAction downloads every reference before it runs, so a later + // failure must not leave a retry with handles whose objects are gone. + await expect(storage.download(key)).resolves.toEqual(body); + }); + + it('accepts an empty upload', async () => { + await put('k', Buffer.alloc(0)); + + await expect(storage.download('k')).resolves.toHaveLength(0); + }); + + it('replaces an object uploaded twice without leaking its size', async () => { + configure({ maxBytes: 1024, maxTotalBytes: 2048 }); + + await put('k', Buffer.alloc(600)); + await put('k', Buffer.alloc(700)); + + await expect(storage.getSize('k')).resolves.toBe(700); + // Counting the replaced 600 too would total 2300 here and refuse this upload. + await expect(put('other', Buffer.alloc(1000))).resolves.toMatchObject({ status: 200 }); + }); + + // The replacement only has to fit next to the *other* objects, not next to the one it evicts. + it('accepts a replacement that alone fills the store', async () => { + configure({ maxBytes: 1024, maxTotalBytes: 1024 }); + await put('k', Buffer.alloc(1000)); + + await expect(put('k', Buffer.alloc(1000))).resolves.toMatchObject({ status: 200 }); + await expect(storage.getSize('k')).resolves.toBe(1000); + }); + }); + + describe('limits', () => { + it('refuses a body over maxBytes', async () => { + const response = await put('k', Buffer.alloc(2000)); + + expect(response.status).toBe(413); + await expect(storage.getSize('k')).resolves.toBeUndefined(); + }); + + it('refuses a body that would take the store over its total, mid-stream', async () => { + await put('a', Buffer.alloc(1000)); + await put('b', Buffer.alloc(1000)); + await put('c', Buffer.alloc(1000)); + await put('d', Buffer.alloc(1000)); + + const response = await put('e', Buffer.alloc(1000)); + + expect(response.status).toBe(413); + await expect(storage.getSize('e')).resolves.toBeUndefined(); + }); + + it('refuses before reading a byte once the store is exactly full', async () => { + configure({ maxBytes: 1024, maxTotalBytes: 2048 }); + await put('a', Buffer.alloc(1024)); + await put('b', Buffer.alloc(1024)); + + const response = await put('c', Buffer.from('x')); + + expect(response.status).toBe(507); + expect(logger).toHaveBeenCalledWith('Warn', expect.stringContaining('store is full')); + }); + + it('answers before reading when the declared size cannot fit', async () => { + await storage.createUploadUrl({ key: 'k' }); + + const response = await request(app) + .put('/k') + .set('content-length', '99999999') + .send(Buffer.alloc(4)); + + expect(response.status).toBe(413); + expect(response.body).toEqual({ error: expect.stringContaining('larger than the 1024') }); + }); + + // Reached when the body fits on its own but the store does not have room: the refusal happens + // on a chunk, and the rest must be dropped rather than accumulated or answered twice. + it('stops accumulating once it has refused mid-stream', async () => { + configure({ maxBytes: 1024, maxTotalBytes: 1200 }); + await put('a', Buffer.alloc(1000)); + + const response = await put('b', Buffer.alloc(1000)); + + expect(response.status).toBe(413); + expect(response.body).toEqual({ error: expect.stringContaining('store is full') }); + await expect(storage.getSize('b')).resolves.toBeUndefined(); + }); + + // A chunked body declares no size, so the pre-check cannot see it and the refusal happens on + // the chunk that goes over. The chunks after it must be dropped, not accumulated. + it('refuses a chunked body over maxBytes, which declares no size', async () => { + configure({ maxBytes: 150 }); + await storage.createUploadUrl({ key: 'k' }); + const server = app.listen(0); + const { port } = server.address() as { port: number }; + + const status = await new Promise(resolve => { + const req = http.request({ port, method: 'PUT', path: '/k' }, res => + resolve(res.statusCode), + ); + + const write = (remaining: number) => { + if (!remaining) return req.end(); + req.write(Buffer.alloc(100)); + // Spaced out so they arrive as separate chunks rather than one coalesced read. + setTimeout(() => write(remaining - 1), 10); + }; + + write(3); + }); + + server.close(); + expect(status).toBe(413); + await expect(storage.getSize('k')).resolves.toBeUndefined(); + }); + + // Substituting the bytes of an upload that already landed is the one thing a leaked url could + // still do, so the authorization is consumed. A retry must say that, not blame a ttl. + it('names the used url rather than blaming a ttl on a second upload', async () => { + await put('k', Buffer.from('first')); + + const response = await request(app).put('/k').send('second'); + + expect(response.status).toBe(409); + expect(response.body).toEqual({ error: expect.stringContaining('already used') }); + await expect(storage.download('k')).resolves.toEqual(Buffer.from('first')); + }); + + // Both would pass a has() check and the later would overwrite the earlier, so the + // authorization is consumed at the start of the request rather than once the body is stored. + it('lets one of two concurrent uploads through, not both', async () => { + await storage.createUploadUrl({ key: 'k' }); + + const [a, b] = await Promise.all([ + request(app).put('/k').send('first'), + request(app).put('/k').send('second'), + ]); + const statuses = [a.status, b.status]; + + expect(statuses.filter(status => status === 200)).toHaveLength(1); + expect(statuses.some(status => status === 404 || status === 409)).toBe(true); + // Whichever won, the object is one whole body rather than two interleaved ones. + await expect(storage.download('k').then(String)).resolves.toMatch(/^(first|second)$/); + }); + + // Consuming at the start means a refused attempt burns the url too. That is the honest reading + // of single-use, and retrying an oversized body against the same url would fail anyway. + it('burns the url even when the upload was refused', async () => { + await put('k', Buffer.alloc(2000)); + + const response = await request(app).put('/k').send('x'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: expect.stringContaining('already used, or expired') }); + }); + + it('stops accepting an upload url that was never used in time', async () => { + configure({ issuedTtlSeconds: 60 }); + await storage.createUploadUrl({ key: 'k' }); + + jest.useFakeTimers().setSystemTime(Date.now() + 61_000); + + const response = await request(app).put('/k').send('x'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: expect.stringContaining('no upload was authorized') }); + }); + + it('serves the next upload once an object expired', async () => { + configure({ ttlSeconds: 60, maxTotalBytes: 1024 }); + await put('a', Buffer.alloc(1000)); + + jest.useFakeTimers().setSystemTime(Date.now() + 61_000); + + await expect(put('b', Buffer.alloc(1000))).resolves.toMatchObject({ status: 200 }); + }); + }); + + describe('expiry', () => { + it('forgets an object past its ttl', async () => { + await put('k', Buffer.from('hi')); + + jest.useFakeTimers().setSystemTime(Date.now() + 61_000); + + await expect(storage.getSize('k')).resolves.toBeUndefined(); + }); + }); + + describe('download of a missing object', () => { + // A refused upload, an expired one and a replica that never saw it are the same absence from + // here, so the message has to name all three rather than pick one and send the reader after it. + it('names every cause an absence can have', async () => { + await expect(storage.download('never-uploaded')).rejects.toThrow( + /the upload never completed .* or it expired after handleTtlSeconds, or it reached another instance/, + ); + }); + }); + + // Allocating the body can fail, and a throw in this listener is an uncaughtException that takes + // the whole agent down rather than this one request. + it('answers 500 when the body cannot be allocated', async () => { + jest.spyOn(Buffer, 'concat').mockImplementationOnce(() => { + throw new Error('Array buffer allocation failed'); + }); + + const response = await put('k', Buffer.from('hi')); + + expect(response.status).toBe(500); + expect(logger).toHaveBeenCalledWith('Error', expect.stringContaining('allocation failed')); + }); + + // The path a real client abort takes, which is the most common failure here. Silence would leave + // an upload that vanished with no trace anywhere. + it('logs an upload the client aborted', async () => { + await storage.createUploadUrl({ key: 'k' }); + const server = app.listen(0); + const { port } = server.address() as { port: number }; + + await new Promise(resolve => { + const req = http.request( + { port, method: 'PUT', path: '/k', headers: { 'content-length': '400' } }, + () => undefined, + ); + + req.on('error', () => resolve()); + req.write(Buffer.alloc(100)); + setTimeout(() => { + req.destroy(new Error('aborted by the client')); + }, 20); + }); + + await new Promise(resolve => { + setTimeout(resolve, 50); + }); + server.close(); + + expect(logger).toHaveBeenCalledWith('Warn', expect.stringContaining('failed after')); + await expect(storage.getSize('k')).resolves.toBeUndefined(); + }); + + it('answers nothing but PUT', async () => { + await expect(request(app).get('/k')).resolves.toMatchObject({ status: 404 }); + await expect(request(app).post('/k').send('x')).resolves.toMatchObject({ status: 404 }); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/file-reference.test.ts b/packages/mcp-server/test/file-uploads/file-reference.test.ts new file mode 100644 index 0000000000..8562bf1136 --- /dev/null +++ b/packages/mcp-server/test/file-uploads/file-reference.test.ts @@ -0,0 +1,23 @@ +import parseFileReference, { UPLOADED_FILE_PREFIX } from '../../src/file-uploads/file-reference'; + +describe('parseFileReference', () => { + it('exposes the sentinel prefix used inside action values', () => { + expect(UPLOADED_FILE_PREFIX).toBe('$uploadedFile:'); + }); + + it('extracts the handle of an upload reference', () => { + expect(parseFileReference('$uploadedFile:a.b.c')).toBe('a.b.c'); + }); + + it.each([ + ['a plain string', 'report.pdf'], + ['a data uri', 'data:application/pdf;base64,JVBERg=='], + ['a prefix appearing mid-string', 'see $uploadedFile:token'], + ['a number', 42], + ['null', null], + ['undefined', undefined], + ['an object', { handle: 'x' }], + ])('returns null for %s', (_, value) => { + expect(parseFileReference(value)).toBeNull(); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/handles.test.ts b/packages/mcp-server/test/file-uploads/handles.test.ts new file mode 100644 index 0000000000..b381b3f8e1 --- /dev/null +++ b/packages/mcp-server/test/file-uploads/handles.test.ts @@ -0,0 +1,83 @@ +import jsonwebtoken from 'jsonwebtoken'; + +import { signUploadHandle, verifyUploadHandle } from '../../src/file-uploads/handles'; + +const AUTH_SECRET = 'test-auth-secret'; + +const claims = { + key: 'mcp-uploads/uuid/report.pdf', + name: 'report.pdf', + mimeType: 'application/pdf', + userId: 42, +}; + +describe('upload handles', () => { + it('round-trips the claims for the same user', () => { + const handle = signUploadHandle(claims, AUTH_SECRET, 60); + + expect(verifyUploadHandle(handle, 42, AUTH_SECRET)).toEqual({ + key: 'mcp-uploads/uuid/report.pdf', + name: 'report.pdf', + mimeType: 'application/pdf', + sha256Base64: undefined, + }); + }); + + it('carries the sha256 pin when provided', () => { + const handle = signUploadHandle({ ...claims, sha256Base64: 'digest==' }, AUTH_SECRET, 60); + + expect(verifyUploadHandle(handle, 42, AUTH_SECRET).sha256Base64).toBe('digest=='); + }); + + it('accepts a user id whose type differs between signing and redemption', () => { + const handle = signUploadHandle(claims, AUTH_SECRET, 60); + + expect(verifyUploadHandle(handle, '42', AUTH_SECRET).key).toBe('mcp-uploads/uuid/report.pdf'); + }); + + it('rejects a handle redeemed by another user', () => { + const handle = signUploadHandle(claims, AUTH_SECRET, 60); + + expect(() => verifyUploadHandle(handle, 43, AUTH_SECRET)).toThrow( + 'Handle was issued to another user', + ); + }); + + it('rejects an expired handle', () => { + const handle = signUploadHandle(claims, AUTH_SECRET, -1); + + expect(() => verifyUploadHandle(handle, 42, AUTH_SECRET)).toThrow('jwt expired'); + }); + + it('rejects a handle signed with another secret', () => { + const handle = signUploadHandle(claims, 'other-secret', 60); + + expect(() => verifyUploadHandle(handle, 42, AUTH_SECRET)).toThrow('invalid signature'); + }); + + it('rejects an access token that was signed with the same secret', () => { + const accessTokenLookalike = jsonwebtoken.sign({ id: 42 }, AUTH_SECRET, { expiresIn: 60 }); + + expect(() => verifyUploadHandle(accessTokenLookalike, 42, AUTH_SECRET)).toThrow( + 'Not an upload handle', + ); + }); + + it.each([ + ['garbage', 'not.a.jwt'], + ['a truncated jwt', 'eyJhbGciOiJIUzI1NiJ9.eyJhIjoxfQ'], + ['an empty string', ''], + ])('rejects %s, which is what a hallucinating model sends', (_, handle) => { + expect(() => verifyUploadHandle(handle, 42, AUTH_SECRET)).toThrow(); + }); + + it('rejects a handle whose key claim is missing, instead of downloading undefined', () => { + const forged = jsonwebtoken.sign( + { type: 'mcp-upload', uploader: '42', name: 'x.pdf', mime: 'application/pdf' }, + AUTH_SECRET, + { expiresIn: 60 }, + ); + + expect(() => verifyUploadHandle(forged, 42, AUTH_SECRET)).toThrow('Malformed upload handle'); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/resolve.test.ts b/packages/mcp-server/test/file-uploads/resolve.test.ts new file mode 100644 index 0000000000..c351eed71f --- /dev/null +++ b/packages/mcp-server/test/file-uploads/resolve.test.ts @@ -0,0 +1,351 @@ +import type { UploadStorage } from '../../src/file-uploads/types'; +import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; + +import * as crypto from 'crypto'; + +import { UPLOADED_FILE_PREFIX } from '../../src/file-uploads/file-reference'; +import { signUploadHandle } from '../../src/file-uploads/handles'; +import resolveUploadedFileValues from '../../src/file-uploads/resolve'; +import { resolveFileUploads } from '../../src/file-uploads/types'; + +const AUTH_SECRET = 'test-auth-secret'; + +const authInfo = { + token: 'token', + clientId: '42', + scopes: ['mcp:action'], + extra: { userId: 42 }, +} as unknown as AuthInfo; + +function makeStorage(overrides: Partial = {}): UploadStorage { + return { + createUploadUrl: jest.fn().mockResolvedValue({ url: 'https://storage.example/put' }), + download: jest.fn().mockResolvedValue(Buffer.from('%PDF-1.4')), + getSize: jest.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +function makeUploads(storage: UploadStorage, options: { maxBytes?: number } = {}) { + return resolveFileUploads({ storage, ...options }, AUTH_SECRET); +} + +function makeHandle(overrides: Partial[0]> = {}): string { + const handle = signUploadHandle( + { + key: 'mcp-uploads/uuid/report.pdf', + name: 'report.pdf', + mimeType: 'application/pdf', + userId: 42, + ...overrides, + }, + AUTH_SECRET, + 60, + ); + + return `${UPLOADED_FILE_PREFIX}${handle}`; +} + +describe('resolveUploadedFileValues', () => { + it('returns the values untouched when none carries a reference', async () => { + const storage = makeStorage(); + const values = { amount: 12, note: 'plain string' }; + + const resolved = await resolveUploadedFileValues(values, authInfo, makeUploads(storage)); + + expect(resolved).toBe(values); + expect(storage.download).not.toHaveBeenCalled(); + }); + + it('throws a configuration error when a reference is present but uploads are disabled', async () => { + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, undefined), + ).rejects.toThrow('File uploads are not configured on this server'); + }); + + it('throws when there is no authenticated user to bind the reference to', async () => { + await expect( + resolveUploadedFileValues({ document: makeHandle() }, undefined, makeUploads(makeStorage())), + ).rejects.toThrow('Cannot resolve uploaded files without an authenticated user'); + }); + + it('replaces a reference with the uploaded file, leaving the name unencoded', async () => { + const storage = makeStorage(); + + const resolved = await resolveUploadedFileValues( + { document: makeHandle({ name: 'rapport final.pdf' }), note: 'untouched' }, + authInfo, + makeUploads(storage), + ); + + expect(storage.download).toHaveBeenCalledWith('mcp-uploads/uuid/report.pdf'); + expect(resolved.document).toEqual({ + buffer: Buffer.from('%PDF-1.4'), + mimeType: 'application/pdf', + name: 'rapport final.pdf', + }); + expect(resolved.note).toBe('untouched'); + }); + + it('resolves references inside array values and leaves other entries alone', async () => { + const resolved = await resolveUploadedFileValues( + { attachments: [makeHandle(), 'existing-value'] }, + authInfo, + makeUploads(makeStorage()), + ); + + expect(resolved.attachments).toEqual([ + expect.objectContaining({ name: 'report.pdf', mimeType: 'application/pdf' }), + 'existing-value', + ]); + }); + + it('downloads a reference used by several fields only once', async () => { + const storage = makeStorage(); + const handle = makeHandle(); + + const resolved = await resolveUploadedFileValues( + { front: handle, back: handle }, + authInfo, + makeUploads(storage), + ); + + expect(storage.download).toHaveBeenCalledTimes(1); + expect(resolved.front).toBe(resolved.back); + }); + + it('rejects a handle issued to another user', async () => { + await expect( + resolveUploadedFileValues( + { document: makeHandle({ userId: 999 }) }, + authInfo, + makeUploads(makeStorage()), + ), + ).rejects.toThrow('Handle was issued to another user'); + }); + + it('rejects an oversized upload without downloading it when the backend reports a size', async () => { + const storage = makeStorage({ getSize: jest.fn().mockResolvedValue(50 * 1024 * 1024) }); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, makeUploads(storage)), + ).rejects.toThrow('above the 20971520 byte limit'); + + expect(storage.download).not.toHaveBeenCalled(); + }); + + it('rejects an oversized upload after download when the backend cannot report a size', async () => { + const storage = makeStorage({ download: jest.fn().mockResolvedValue(Buffer.alloc(11)) }); + + await expect( + resolveUploadedFileValues( + { document: makeHandle() }, + authInfo, + makeUploads(storage, { maxBytes: 10 }), + ), + ).rejects.toThrow('above the 10 byte limit'); + }); + + // A missing object is already rejected by download, so zero bytes only ever means the user + // uploaded an empty file — which maxBytes allows. + it('accepts a legitimately empty file', async () => { + const storage = makeStorage({ download: jest.fn().mockResolvedValue(Buffer.alloc(0)) }); + + const resolved = await resolveUploadedFileValues( + { document: makeHandle() }, + authInfo, + makeUploads(storage), + ); + + expect((resolved.document as { buffer: Buffer }).buffer).toHaveLength(0); + }); + + it('rejects content that does not match the sha256 the handle was pinned to', async () => { + const pinned = crypto.createHash('sha256').update('original content').digest('base64'); + const storage = makeStorage({ + download: jest.fn().mockResolvedValue(Buffer.from('substituted content')), + }); + + await expect( + resolveUploadedFileValues( + { document: makeHandle({ sha256Base64: pinned }) }, + authInfo, + makeUploads(storage), + ), + ).rejects.toThrow('Field "document": uploaded file does not match the sha256 it was pinned to'); + }); + + it('accepts content matching the pinned sha256', async () => { + const content = Buffer.from('original content'); + const pinned = crypto.createHash('sha256').update(new Uint8Array(content)).digest('base64'); + const storage = makeStorage({ download: jest.fn().mockResolvedValue(content) }); + + const resolved = await resolveUploadedFileValues( + { document: makeHandle({ sha256Base64: pinned }) }, + authInfo, + makeUploads(storage), + ); + + expect(resolved.document).toEqual(expect.objectContaining({ buffer: content })); + }); + + it('bounds concurrent downloads to maxConcurrentDownloads', async () => { + let active = 0; + let peak = 0; + + const storage = makeStorage({ + download: jest.fn().mockImplementation(async () => { + active += 1; + peak = Math.max(peak, active); + await new Promise(resolve => { + setTimeout(resolve, 5); + }); + active -= 1; + + return Buffer.from('file content'); + }), + }); + + const uploads = resolveFileUploads({ storage, maxConcurrentDownloads: 2 }, AUTH_SECRET); + + const values = Object.fromEntries( + Array.from({ length: 6 }, (_, i) => [ + `file_${i}`, + makeHandle({ key: `mcp-uploads/uuid/file-${i}.pdf` }), + ]), + ); + + await resolveUploadedFileValues(values, authInfo, uploads); + + expect(storage.download).toHaveBeenCalledTimes(6); + expect(peak).toBeLessThanOrEqual(2); + }); + + it('names the field in the error, so the model knows which file to re-upload', async () => { + const storage = makeStorage({ download: jest.fn().mockRejectedValue(new Error('NoSuchKey')) }); + + await expect( + resolveUploadedFileValues({ invoice: makeHandle() }, authInfo, makeUploads(storage)), + ).rejects.toThrow('Field "invoice"'); + }); + + it('enforces the limit on the downloaded bytes when getSize under-reports', async () => { + const storage = makeStorage({ + getSize: jest.fn().mockResolvedValue(5), + download: jest.fn().mockResolvedValue(Buffer.alloc(11)), + }); + + await expect( + resolveUploadedFileValues( + { document: makeHandle() }, + authInfo, + makeUploads(storage, { maxBytes: 10 }), + ), + ).rejects.toThrow('above the 10 byte limit'); + }); + + it.each([ + ['null', null], + ['NaN', NaN], + ])('still enforces the limit when getSize answers %s', async (_, size) => { + const storage = makeStorage({ + getSize: jest.fn().mockResolvedValue(size), + download: jest.fn().mockResolvedValue(Buffer.alloc(11)), + }); + + await expect( + resolveUploadedFileValues( + { document: makeHandle() }, + authInfo, + makeUploads(storage, { maxBytes: 10 }), + ), + ).rejects.toThrow('above the 10 byte limit'); + }); + + it('accepts a file of exactly maxBytes', async () => { + const storage = makeStorage({ download: jest.fn().mockResolvedValue(Buffer.alloc(10)) }); + + const resolved = await resolveUploadedFileValues( + { document: makeHandle() }, + authInfo, + makeUploads(storage, { maxBytes: 10 }), + ); + + expect((resolved.document as { buffer: Buffer }).buffer).toHaveLength(10); + }); + + it('rejects a forged handle without spending a download slot', async () => { + const storage = makeStorage(); + + await expect( + resolveUploadedFileValues( + { document: makeHandle({ userId: 999 }) }, + authInfo, + makeUploads(storage), + ), + ).rejects.toThrow('Handle was issued to another user'); + + expect(storage.getSize).not.toHaveBeenCalled(); + expect(storage.download).not.toHaveBeenCalled(); + }); + + // A contract-honouring backend rejects a missing object, so this is what "never uploaded" + // actually looks like — the empty-buffer branch only fires on backends that create the object. + it('turns a download rejection into an actionable message, keeping the cause', async () => { + const storage = makeStorage({ + download: jest.fn().mockRejectedValue(new Error('NoSuchKey')), + }); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, makeUploads(storage)), + ).rejects.toThrow( + 'Field "document": could not read the uploaded file. ' + + 'Did the upload to uploadUrl succeed? (NoSuchKey)', + ); + }); + + // Without a bound, a backend that stops answering holds its slot while the calling client's own + // timeout fires, so the failure would be invisible here. + it('gives up on a storage read that never answers', async () => { + const storage = makeStorage({ download: jest.fn().mockReturnValue(new Promise(() => {})) }); + const uploads = resolveFileUploads({ storage, downloadTimeoutSeconds: 1 }, AUTH_SECRET); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, uploads), + ).rejects.toThrow('timed out after 1s'); + }); + + it('gives up on a size probe that never answers', async () => { + const storage = makeStorage({ getSize: jest.fn().mockReturnValue(new Promise(() => {})) }); + const uploads = resolveFileUploads({ storage, downloadTimeoutSeconds: 1 }, AUTH_SECRET); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, uploads), + ).rejects.toThrow('timed out after 1s'); + expect(storage.download).not.toHaveBeenCalled(); + }); + + it('frees the slot after a timeout, so later redemptions still work', async () => { + let answer = false; + const storage = makeStorage({ + download: jest + .fn() + .mockImplementation(() => + answer ? Promise.resolve(Buffer.from('ok')) : new Promise(() => {}), + ), + }); + const uploads = resolveFileUploads( + { storage, maxConcurrentDownloads: 1, downloadTimeoutSeconds: 1 }, + AUTH_SECRET, + ); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, uploads), + ).rejects.toThrow('timed out'); + + answer = true; + const resolved = await resolveUploadedFileValues({ document: makeHandle() }, authInfo, uploads); + + expect((resolved.document as { buffer: Buffer }).buffer.toString()).toBe('ok'); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/semaphore.test.ts b/packages/mcp-server/test/file-uploads/semaphore.test.ts new file mode 100644 index 0000000000..0f515528e7 --- /dev/null +++ b/packages/mcp-server/test/file-uploads/semaphore.test.ts @@ -0,0 +1,93 @@ +import createSemaphore from '../../src/file-uploads/semaphore'; + +const deferred = () => { + let resolve: (value?: unknown) => void; + let reject: (error: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + + return { promise, resolve, reject }; +}; + +describe('createSemaphore', () => { + it('runs a task and returns its value', async () => { + await expect(createSemaphore(1)(async () => 'done')).resolves.toBe('done'); + }); + + it('holds a task until a slot frees up', async () => { + const run = createSemaphore(1); + const first = deferred(); + let secondStarted = false; + + const firstCall = run(() => first.promise); + const secondCall = run(async () => { + secondStarted = true; + }); + + await Promise.resolve(); + expect(secondStarted).toBe(false); + + first.resolve(); + await firstCall; + await secondCall; + expect(secondStarted).toBe(true); + }); + + // Without the finally, a run of failures would exhaust the slots and hang every later task. + it('frees the slot when a task rejects', async () => { + const run = createSemaphore(2); + + await Promise.all( + Array.from({ length: 5 }, () => + expect( + run(async () => { + throw new Error('boom'); + }), + ).rejects.toThrow('boom'), + ), + ); + + await expect(run(async () => 'still works')).resolves.toBe('still works'); + }); + + it('surfaces the task rejection unchanged', async () => { + await expect( + createSemaphore(1)(async () => { + throw new Error('storage is down'); + }), + ).rejects.toThrow('storage is down'); + }); + + it('never exceeds the limit under load', async () => { + let active = 0; + let peak = 0; + const run = createSemaphore(3); + + await Promise.all( + Array.from({ length: 20 }, () => + run(async () => { + active += 1; + peak = Math.max(peak, active); + await new Promise(resolve => { + setTimeout(resolve, 1); + }); + active -= 1; + }), + ), + ); + + expect(peak).toBe(3); + expect(active).toBe(0); + }); + + // One action submits every file reference at once, so the queue must absorb them all. + it('serves a burst larger than the limit instead of refusing it', async () => { + const run = createSemaphore(2); + + const results = await Promise.all(Array.from({ length: 25 }, (_, i) => run(async () => i))); + + expect(results).toHaveLength(25); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/types.test.ts b/packages/mcp-server/test/file-uploads/types.test.ts new file mode 100644 index 0000000000..6f0faadd85 --- /dev/null +++ b/packages/mcp-server/test/file-uploads/types.test.ts @@ -0,0 +1,121 @@ +import type { UploadStorage } from '../../src/file-uploads/types'; +import type { Logger } from '../../src/server'; + +import { resolveFileUploads } from '../../src/file-uploads/types'; + +const AUTH_SECRET = 'test-auth-secret'; + +const storage: UploadStorage = { + createUploadUrl: jest.fn(), + download: jest.fn(), + getSize: jest.fn(), +}; + +describe('resolveFileUploads', () => { + let logger: jest.MockedFunction; + + beforeEach(() => { + jest.clearAllMocks(); + logger = jest.fn(); + }); + + it('returns undefined when the option is absent', () => { + expect(resolveFileUploads(undefined, AUTH_SECRET, logger)).toBeUndefined(); + }); + + it('applies the documented defaults', () => { + const resolved = resolveFileUploads({ storage }, AUTH_SECRET, logger); + + expect(resolved).toMatchObject({ + storage, + keyPrefix: 'mcp-uploads/', + uploadUrlTtlSeconds: 15 * 60, + handleTtlSeconds: 45 * 60, + maxBytes: 20 * 1024 * 1024, + authSecret: AUTH_SECRET, + }); + expect(logger).not.toHaveBeenCalled(); + }); + + it('keeps the values it is given', () => { + const resolved = resolveFileUploads( + { + storage, + keyPrefix: 'uploads/', + uploadUrlTtlSeconds: 60, + handleTtlSeconds: 120, + maxBytes: 1024, + maxConcurrentDownloads: 2, + }, + AUTH_SECRET, + logger, + ); + + expect(resolved).toMatchObject({ + keyPrefix: 'uploads/', + uploadUrlTtlSeconds: 60, + handleTtlSeconds: 120, + maxBytes: 1024, + }); + }); + + it('rejects a missing storage backend at startup', () => { + expect(() => resolveFileUploads({} as never, AUTH_SECRET, logger)).toThrow( + 'fileUploads.storage is required.', + ); + }); + + // A zero or negative limit used to queue every redemption with nothing left to release it, + // hanging forever instead of failing. + it.each([ + ['maxConcurrentDownloads', 0], + ['maxConcurrentDownloads', -1], + ['maxConcurrentDownloads', 1.5], + ['maxBytes', 0], + ['maxBytes', -10], + ['uploadUrlTtlSeconds', 0], + ['handleTtlSeconds', -60], + ['handleTtlSeconds', 60.5], + ])('rejects %s = %p at startup', (field, value) => { + expect(() => resolveFileUploads({ storage, [field]: value }, AUTH_SECRET, logger)).toThrow( + `Invalid fileUploads.${field} "${value}": it must be a positive integer.`, + ); + }); + + describe('when the handle cannot outlive the upload window', () => { + it.each([ + ['shorter', 60], + ['equal', 900], + ])('warns when the handle ttl is %s', (_, handleTtlSeconds) => { + resolveFileUploads( + { storage, uploadUrlTtlSeconds: 900, handleTtlSeconds }, + AUTH_SECRET, + logger, + ); + + expect(logger).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining(`fileUploads.handleTtlSeconds=${handleTtlSeconds}`), + ); + }); + + it('does not warn when the handle outlives it', () => { + resolveFileUploads( + { storage, uploadUrlTtlSeconds: 900, handleTtlSeconds: 901 }, + AUTH_SECRET, + logger, + ); + + expect(logger).not.toHaveBeenCalled(); + }); + + it('tolerates being called without a logger', () => { + expect(() => + resolveFileUploads( + { storage, uploadUrlTtlSeconds: 900, handleTtlSeconds: 60 }, + AUTH_SECRET, + ), + ).not.toThrow(); + }); + }); +}); diff --git a/packages/mcp-server/test/server.test.ts b/packages/mcp-server/test/server.test.ts index 1bd0c74311..60e3e36a59 100644 --- a/packages/mcp-server/test/server.test.ts +++ b/packages/mcp-server/test/server.test.ts @@ -1,3 +1,4 @@ +import type EphemeralStorage from '../src/file-uploads/ephemeral-storage'; import type * as net from 'net'; import * as http from 'http'; @@ -3493,6 +3494,7 @@ describe('enabledTools', () => { 'dissociate', 'getActionForm', 'executeAction', + 'requestActionFileUpload', ], }); @@ -3603,3 +3605,170 @@ describe('Logo URL', () => { expect(response.headers.get('content-type')).toContain('image/png'); }); }); + +describe('file uploads without a storage backend', () => { + const build = (options: Record = {}) => + new ForestMCPServer({ + envSecret: 'test-env-secret', + authSecret: 'test-auth-secret', + forestServerUrl: 'https://test.forestadmin.com', + ...options, + }); + + const buildApp = async (options: Record = {}) => + build(options).buildExpressApp(new URL('https://agent.example')); + + // No option to set: the whole point is that an agent gets this without asking. + it('is enabled without any configuration', async () => { + const response = await request(await buildApp()) + .put('/mcp/uploads/never-issued') + .send('hello'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: expect.stringContaining('no upload was authorized') }); + }); + + // Boot-time would reach every agent, including those whose actions have no file field. + it('stays quiet at startup and announces the single instance on first use', async () => { + const logger = jest.fn(); + const server = build({ logger }); + await server.buildExpressApp(new URL('https://agent.example')); + + expect(logger).not.toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('held in memory, on this instance only'), + ); + + const { ephemeralStorage: storage } = server as unknown as { + ephemeralStorage: EphemeralStorage; + }; + await storage.createUploadUrl({ key: 'k' }); + + expect(logger).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('held in memory, on this instance only'), + ); + }); + + // The tool reports maxBytes to the model verbatim, so this would promise a size the store always + // refuses — with "the store is full", on an empty store. + it('warns when maxBytes exceeds what the in-memory store can ever hold', async () => { + const logger = jest.fn(); + + await buildApp({ logger, fileUploads: { maxBytes: 100 * 1024 * 1024 } }); + + expect(logger).toHaveBeenCalledWith('Warn', expect.stringContaining('always refused')); + }); + + // enabledTools is the off switch, and it has to take the endpoint with it. + it('serves no upload endpoint when the tool is not enabled', async () => { + const app = await buildApp({ enabledTools: ['describeCollection', 'list'] }); + + const response = await request(app).put('/mcp/uploads/anything').send('hello'); + + expect(response.status).toBe(405); + }); + + it('turns the whole feature off on fileUploads: false, without touching enabledTools', async () => { + const server = build({ fileUploads: false }); + const app = await server.buildExpressApp(new URL('https://agent.example')); + + await expect(request(app).put('/mcp/uploads/anything').send('hello')).resolves.toMatchObject({ + status: 405, + }); + expect((server as unknown as { enabledTools: Set }).enabledTools).not.toContain( + 'requestActionFileUpload', + ); + // Every other tool is untouched, which is the point of not going through enabledTools. + expect((server as unknown as { enabledTools: Set }).enabledTools).toContain('list'); + }); + + // Config often merges from two layers; when they contradict each other, false wins — and says so, + // because a tool the caller named vanishing without a log line reads as a bug in the losing layer. + it('lets fileUploads: false override an enabledTools that lists the tool, and warns', async () => { + const logger = jest.fn(); + const server = build({ + logger, + fileUploads: false, + enabledTools: ['describeCollection', 'list', 'requestActionFileUpload'], + }); + await server.buildExpressApp(new URL('https://agent.example')); + + expect((server as unknown as { enabledTools: Set }).enabledTools).not.toContain( + 'requestActionFileUpload', + ); + expect(logger).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('even though enabledTools lists it'), + ); + }); + + // 404 comes from the uploads router itself, for a key it never handed out. A 405 would mean + // allowedMethods(['POST']) claimed the PUT first, and a hang would mean a body parser did. + it('serves the upload endpoint under /mcp/uploads', async () => { + const response = await request(await buildApp()) + .put('/mcp/uploads/mcp-uploads%2Fuuid%2Fa.txt') + .send('hello'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: expect.stringContaining('no upload was authorized') }); + }); + + // The url handed to the client is built from the server's own base url, while the route is + // mounted from the prefix. Uploading to the url it actually returns is the only check that the + // two agree — everything else here asks the route about a path the test itself wrote. + it('accepts an upload at the url it hands out, and serves those exact bytes back', async () => { + const server = build(); + const app = await server.buildExpressApp(new URL('https://agent.example')); + const { ephemeralStorage: storage } = server as unknown as { + ephemeralStorage: EphemeralStorage; + }; + const body = Buffer.from('CONTENU-BINAIRE- ÿ'); + + const { url } = await storage.createUploadUrl({ key: 'mcp-uploads/uuid/rapport final;v2.pdf' }); + const response = await request(app).put(new URL(url).pathname).send(body); + + expect(response.status).toBe(200); + await expect(storage.download('mcp-uploads/uuid/rapport final;v2.pdf')).resolves.toEqual(body); + }); + + it('leaves /mcp itself bearer-protected', async () => { + const response = await request(await buildApp()) + .post('/mcp') + .send({}); + + expect(response.status).toBe(401); + }); +}); + +describe('file uploads tool', () => { + const storage = { + createUploadUrl: jest.fn().mockResolvedValue({ url: 'https://storage.example/put' }), + download: jest.fn(), + getSize: jest.fn(), + }; + + const buildApp = async (fileUploads?: { storage: typeof storage }) => + new ForestMCPServer({ + envSecret: 'test-env-secret', + authSecret: 'test-auth-secret', + forestServerUrl: 'https://test.forestadmin.com', + ...(fileUploads && { fileUploads }), + }).buildExpressApp(new URL('https://agent.example')); + + it.each([[undefined], [{ storage }]])('does not serve /files (fileUploads: %p)', async opts => { + const response = await request(await buildApp(opts)) + .post('/files') + .send({}); + + expect(response.status).toBe(404); + }); + + it('still requires a bearer token on /mcp when uploads are enabled', async () => { + const response = await request(await buildApp({ storage })) + .post('/mcp') + .send({}); + + expect(response.status).toBe(401); + }); +}); diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index 99e8cd2258..9b8bab74d0 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -5,6 +5,9 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; +import { UPLOADED_FILE_PREFIX } from '../../src/file-uploads/file-reference'; +import { signUploadHandle } from '../../src/file-uploads/handles'; +import { resolveFileUploads } from '../../src/file-uploads/types'; import declareExecuteActionTool from '../../src/tools/execute-action'; import { buildClientWithActions } from '../../src/utils/agent-caller'; import withActivityLog from '../../src/utils/with-activity-log'; @@ -552,4 +555,134 @@ describe('declareExecuteActionTool', () => { }); }); }); + + describe('file uploads', () => { + const AUTH_SECRET = 'test-auth-secret'; + + const fileUploads = () => + resolveFileUploads( + { + storage: { + createUploadUrl: jest.fn().mockResolvedValue({ url: 'https://storage.example/put' }), + download: jest.fn().mockResolvedValue(Buffer.from('%PDF-1.4')), + getSize: jest.fn().mockResolvedValue(undefined), + }, + }, + AUTH_SECRET, + ); + + const uploadExtra = { + authInfo: { + token: 'test-token', + extra: { userId: 42, forestServerToken: 'forest-token', renderingId: '123' }, + }, + } as unknown as RequestHandlerExtra; + + const makeHandle = () => + `${UPLOADED_FILE_PREFIX}${signUploadHandle( + { + key: 'mcp-uploads/uuid/report.pdf', + name: 'rapport final.pdf', + mimeType: 'application/pdf', + userId: 42, + }, + AUTH_SECRET, + 60, + )}`; + + const mockAgentAction = () => { + const mockSetFields = jest.fn().mockResolvedValue(undefined); + const mockAction = jest.fn().mockResolvedValue({ + execute: jest.fn().mockResolvedValue({ success: 'Action executed' }), + setFields: mockSetFields, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 42, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + return mockSetFields; + }; + + it('documents the upload workflow in the description when uploads are enabled', () => { + declareExecuteActionTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + fileUploads: fileUploads(), + }); + + expect(registeredToolConfig.description).toContain('requestActionFileUpload'); + expect(registeredToolConfig.description).toContain('never inline base64'); + }); + + it('does not mention uploads in the description when disabled', () => { + declareExecuteActionTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + + expect(registeredToolConfig.description).not.toContain('requestActionFileUpload'); + }); + + it('hands the uploaded file to agent-client, which owns the encoding', async () => { + declareExecuteActionTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + fileUploads: fileUploads(), + }); + const mockSetFields = mockAgentAction(); + + await registeredToolHandler( + { + collectionName: 'users', + actionName: 'attachDocument', + recordIds: [1], + values: { document: makeHandle(), note: 'untouched' }, + }, + uploadExtra, + ); + + expect(mockSetFields).toHaveBeenCalledWith({ + document: { + buffer: Buffer.from('%PDF-1.4'), + mimeType: 'application/pdf', + name: 'rapport final.pdf', + }, + note: 'untouched', + }); + }); + + it('returns a tool error when a handle is sent but uploads are not configured', async () => { + declareExecuteActionTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + }); + const mockSetFields = mockAgentAction(); + + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'attachDocument', + recordIds: [1], + values: { document: makeHandle() }, + }, + uploadExtra, + ); + + expect(result).toEqual({ + content: [ + { + type: 'text', + text: expect.stringContaining('File uploads are not configured on this server'), + }, + ], + isError: true, + }); + expect(mockSetFields).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/mcp-server/test/tools/get-action-form.test.ts b/packages/mcp-server/test/tools/get-action-form.test.ts index a6557a28b4..9102810b26 100644 --- a/packages/mcp-server/test/tools/get-action-form.test.ts +++ b/packages/mcp-server/test/tools/get-action-form.test.ts @@ -277,6 +277,146 @@ describe('declareGetActionFormTool', () => { expect(mockTryToSetFields).toHaveBeenCalledWith(values); }); + // Resolving here would put the bytes back in the model's context, and handing the handle to a + // change hook makes it read `.buffer` off a string and throw a 500. So it is withheld, and the + // other values still reach the hook. + it('withholds an upload handle from the change hooks instead of resolving it', async () => { + const mockTryToSetFields = jest.fn().mockResolvedValue([]); + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([]), + tryToSetFields: mockTryToSetFields, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const values = { + document: '$uploadedFile:some-token', + attachments: ['$uploadedFile:another'], + note: 'hello', + }; + await registeredToolHandler( + { collectionName: 'users', actionName: 'sendEmail', recordIds: [1], values }, + mockExtra, + ); + + expect(mockTryToSetFields).toHaveBeenCalledWith({ note: 'hello' }); + }); + + // Withholding must not make the field unsatisfiable: the agent never sees the handle, so + // counting it as missing would leave canExecute false with nothing the model could send. + it('counts a withheld handle as filling its required field, and echoes it back', async () => { + const mockTryToSetFields = jest.fn().mockResolvedValue([]); + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'Document', + getType: () => 'File', + getTypeName: () => 'File', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({}), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: mockTryToSetFields, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'sendEmail', + recordIds: [1], + values: { Document: '$uploadedFile:some-token' }, + }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(payload.canExecute).toBe(true); + expect(payload.requiredFields).toEqual([]); + expect(payload.fields[0].value).toBe('$uploadedFile:some-token'); + }); + + it('does the same for a FileList: the array of handles fills the field and is echoed back', async () => { + const mockTryToSetFields = jest.fn().mockResolvedValue([]); + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'Attachments', + getType: () => ['File'], + getTypeName: () => 'FileList', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({}), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: mockTryToSetFields, + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'sendEmail', + recordIds: [1], + values: { Attachments: ['$uploadedFile:a', '$uploadedFile:b'] }, + }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(payload.canExecute).toBe(true); + expect(payload.fields[0].value).toEqual(['$uploadedFile:a', '$uploadedFile:b']); + }); + + // `in` would walk the prototype chain: a field literally named toString would read as filled + // by Object.prototype.toString — a function — and canExecute would come back true on an empty + // form. + it('does not count a field named like an Object.prototype member as filled', async () => { + const mockAction = jest.fn().mockResolvedValue({ + getFields: jest.fn().mockReturnValue([ + { + getName: () => 'toString', + getType: () => 'String', + getTypeName: () => 'String', + getValue: () => undefined, + isRequired: () => true, + getPlainField: () => ({}), + getMultipleChoiceField: () => ({ getOptions: () => null }), + }, + ]), + tryToSetFields: jest.fn().mockResolvedValue([]), + }); + mockBuildClientWithActions.mockResolvedValue({ + rpcClient: { collection: jest.fn().mockReturnValue({ action: mockAction }) }, + authData: { userId: 1, renderingId: '123', environmentId: 1, projectId: 1 }, + } as unknown as ReturnType); + + const result = await registeredToolHandler( + { + collectionName: 'users', + actionName: 'sendEmail', + recordIds: [1], + values: { note: 'hello' }, + }, + mockExtra, + ); + const payload = JSON.parse((result as { content: { text: string }[] }).content[0].text); + + expect(payload.canExecute).toBe(false); + expect(payload.requiredFields).toEqual(['toString']); + }); + it('should not call tryToSetFields when values are not provided', async () => { const mockGetFields = jest.fn().mockReturnValue([]); const mockTryToSetFields = jest.fn().mockResolvedValue([]); @@ -303,6 +443,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -311,6 +452,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'message', getType: () => 'String', + getTypeName: () => 'String', getValue: () => 'Default message', isRequired: () => false, getPlainField: () => ({}), @@ -354,6 +496,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => 'Test Subject', isRequired: () => true, getPlainField: () => ({}), @@ -362,6 +505,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'message', getType: () => 'String', + getTypeName: () => 'String', getValue: () => 'Test Message', isRequired: () => true, getPlainField: () => ({}), @@ -396,6 +540,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -404,6 +549,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'message', getType: () => 'String', + getTypeName: () => 'String', getValue: () => 'Test Message', isRequired: () => false, getPlainField: () => ({}), @@ -438,6 +584,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'quantity', getType: () => 'Number', + getTypeName: () => 'Number', getValue: () => 0, isRequired: () => true, getPlainField: () => ({}), @@ -472,6 +619,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'isActive', getType: () => 'Boolean', + getTypeName: () => 'Boolean', getValue: () => false, isRequired: () => true, getPlainField: () => ({}), @@ -506,6 +654,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'notes', getType: () => 'String', + getTypeName: () => 'String', getValue: () => '', isRequired: () => true, getPlainField: () => ({}), @@ -540,6 +689,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => null, isRequired: () => true, getPlainField: () => ({}), @@ -574,6 +724,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'optionalField', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -608,6 +759,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'status', getType: () => 'Enum', + getTypeName: () => 'Enum', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), @@ -616,6 +768,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'message', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -662,6 +815,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'plan', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({ description: 'Subscription plan' }), @@ -675,6 +829,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'priority', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => false, getPlainField: () => ({}), @@ -759,6 +914,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => 'Test Subject', isRequired: () => true, getPlainField: () => ({}), @@ -800,6 +956,7 @@ describe('declareGetActionFormTool', () => { { getName: () => 'subject', getType: () => 'String', + getTypeName: () => 'String', getValue: () => undefined, isRequired: () => true, getPlainField: () => ({}), diff --git a/packages/mcp-server/test/tools/request-action-file-upload.test.ts b/packages/mcp-server/test/tools/request-action-file-upload.test.ts new file mode 100644 index 0000000000..549e1ba0ba --- /dev/null +++ b/packages/mcp-server/test/tools/request-action-file-upload.test.ts @@ -0,0 +1,314 @@ +import type { UploadStorage } from '../../src/file-uploads/types'; +import type { ForestServerClient } from '../../src/http-client'; +import type { Logger } from '../../src/server'; +import type { RegisteredToolConfig } from '../helpers/registered-tool-config'; +import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import type { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +import { verifyUploadHandle } from '../../src/file-uploads/handles'; +import { resolveFileUploads } from '../../src/file-uploads/types'; +import declareRequestActionFileUploadTool from '../../src/tools/request-action-file-upload'; + +const AUTH_SECRET = 'test-auth-secret'; +const mockLogger: Logger = jest.fn(); +const mockForestServerClient = { + forestServerUrl: 'https://api.forestadmin.com', + fetchSchema: jest.fn(), + createActivityLog: jest.fn(), + createMcpActivityLog: jest.fn(), + updateActivityLogStatus: jest.fn(), +} as unknown as ForestServerClient; + +const authenticatedExtra = { + authInfo: { token: 'test-token', scopes: ['mcp:read', 'mcp:action'], extra: { userId: 42 } }, +} as unknown as RequestHandlerExtra; + +describe('declareRequestActionFileUploadTool', () => { + let mcpServer: McpServer; + let handler: (options: unknown, extra: unknown) => Promise<{ content: { text: string }[] }>; + let config: RegisteredToolConfig; + let storage: UploadStorage; + + const setup = (uploadsEnabled = true) => { + storage = { + createUploadUrl: jest.fn().mockResolvedValue({ + url: 'https://storage.example/put?signed=1', + headers: { 'Content-Type': 'application/pdf' }, + }), + download: jest.fn(), + getSize: jest.fn(), + }; + + declareRequestActionFileUploadTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + ...(uploadsEnabled && { fileUploads: resolveFileUploads({ storage }, AUTH_SECRET) }), + }); + }; + + const call = async (args: Record, extra = authenticatedExtra) => { + const result = await handler(args, extra); + + return JSON.parse(result.content[0].text); + }; + + // registerToolWithLogging reports execution errors as an isError result, not a rejection. + const callExpectingError = async (args: Record, extra = authenticatedExtra) => + (await handler(args, extra)) as unknown as { isError: boolean; content: { text: string }[] }; + + const claimsOf = (fileHandle: string) => + verifyUploadHandle(fileHandle.slice('$uploadedFile:'.length), 42, AUTH_SECRET); + + beforeEach(() => { + jest.clearAllMocks(); + mcpServer = { + registerTool: jest.fn((name, toolConfig, toolHandler) => { + config = toolConfig; + handler = toolHandler; + }), + } as unknown as McpServer; + }); + + describe('registration', () => { + it('tells the model never to inline base64', () => { + setup(); + + expect(config.description).toContain('Never inline base64 file content'); + }); + + it('documents the outbound request prerequisite without promising where to configure it', () => { + setup(); + + expect(config.description).toContain('allowed for outbound traffic in that environment'); + }); + }); + + describe('handler', () => { + it('returns an upload destination and a handle redeemable by the caller', async () => { + setup(); + + const response = await call({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response).toMatchObject({ + uploadUrl: 'https://storage.example/put?signed=1', + method: 'PUT', + headers: { 'Content-Type': 'application/pdf' }, + expiresInSeconds: 15 * 60, + maxBytes: 20 * 1024 * 1024, + }); + expect(response.fileHandle).toMatch(/^\$uploadedFile:/); + expect(claimsOf(response.fileHandle)).toMatchObject({ + name: 'report.pdf', + mimeType: 'application/pdf', + }); + }); + + it('repeats the prerequisite in the response, where a failing upload will read it', async () => { + setup(); + + const response = await call({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.prerequisite).toContain('allowed for outbound traffic in that environment'); + }); + + it('asks the storage for a key under the configured prefix', async () => { + setup(); + + await call({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(storage.createUploadUrl).toHaveBeenCalledWith({ + key: expect.stringMatching(/^mcp-uploads\/[0-9a-f-]{36}\/report\.pdf$/), + mimeType: 'application/pdf', + expiresInSeconds: 15 * 60, + }); + }); + + it('normalizes a hex digest to base64 and pins both destination and handle', async () => { + setup(); + const hex = 'a'.repeat(64); + const expected = Buffer.from(hex, 'hex').toString('base64'); + + const response = await call({ + filename: 'report.pdf', + mimeType: 'application/pdf', + sha256: hex, + }); + + expect(storage.createUploadUrl).toHaveBeenCalledWith( + expect.objectContaining({ sha256: expected }), + ); + expect(claimsOf(response.fileHandle).sha256Base64).toBe(expected); + }); + + it('falls back to PUT and a Content-Type header when the storage provides neither', async () => { + declareRequestActionFileUploadTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + fileUploads: resolveFileUploads( + { + storage: { + createUploadUrl: jest.fn().mockResolvedValue({ url: 'https://storage.example/put' }), + download: jest.fn(), + getSize: jest.fn(), + }, + }, + AUTH_SECRET, + ), + }); + + const response = await call({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.method).toBe('PUT'); + expect(response.headers).toEqual({ 'Content-Type': 'application/pdf' }); + }); + + it('uses the method and headers the storage returns', async () => { + declareRequestActionFileUploadTool(mcpServer, { + forestServerClient: mockForestServerClient, + logger: mockLogger, + collectionNames: [], + fileUploads: resolveFileUploads( + { + storage: { + createUploadUrl: jest.fn().mockResolvedValue({ + url: 'https://storage.example/post', + method: 'POST', + headers: { 'x-amz-checksum-sha256': 'abc' }, + }), + download: jest.fn(), + getSize: jest.fn(), + }, + }, + AUTH_SECRET, + ), + }); + + const response = await call({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.method).toBe('POST'); + expect(response.headers).toEqual({ 'x-amz-checksum-sha256': 'abc' }); + }); + + it('accepts a digest already in base64', async () => { + setup(); + const base64 = Buffer.alloc(32, 1).toString('base64'); + + await call({ filename: 'report.pdf', mimeType: 'application/pdf', sha256: base64 }); + + expect(storage.createUploadUrl).toHaveBeenCalledWith( + expect.objectContaining({ sha256: base64 }), + ); + }); + + it.each([ + ['../etc/passwd.pdf', '.._etc_passwd.pdf'], + ['..', 'file'], + ['.', 'file'], + ['a/b/c.pdf', 'a_b_c.pdf'], + ])('sanitizes %p to %p so it cannot escape the storage key', async (filename, expected) => { + setup(); + + const response = await call({ filename, mimeType: 'text/plain' }); + + expect(claimsOf(response.fileHandle).name).toBe(expected); + }); + + it('keeps a filename the agent can round-trip, spaces and all', async () => { + setup(); + + const response = await call({ + filename: 'rapport final (v2).pdf', + mimeType: 'application/pdf', + }); + + expect(claimsOf(response.fileHandle).name).toBe('rapport final (v2).pdf'); + }); + + it('truncates a long filename from the front so the extension survives', async () => { + setup(); + + const response = await call({ + filename: `${'a'.repeat(200)}.pdf`, + mimeType: 'application/pdf', + }); + + const { name } = claimsOf(response.fileHandle); + expect(name).toHaveLength(128); + expect(name.endsWith('.pdf')).toBe(true); + }); + + it.each([ + ['a malformed digest', { sha256: 'nope' }, 'sha256 must be the file digest'], + ['a mime type that is not a pair', { mimeType: 'not a mime type' }, 'is not a media type'], + ])('rejects %s', async (_, override, message) => { + setup(); + + const result = await callExpectingError({ + filename: 'report.pdf', + mimeType: 'application/pdf', + ...override, + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain(message); + expect(storage.createUploadUrl).not.toHaveBeenCalled(); + }); + + // /mcp only gates on mcp:read, so a read-only token would otherwise mint a pre-authorized + // write into the host's storage. + it('rejects a token that lacks the mcp:action scope', async () => { + setup(); + + const result = await callExpectingError({ filename: 'r.pdf', mimeType: 'application/pdf' }, { + authInfo: { token: 't', scopes: ['mcp:read'], extra: { userId: 42 } }, + } as unknown as typeof authenticatedExtra); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('requires the "mcp:action" scope'); + expect(storage.createUploadUrl).not.toHaveBeenCalled(); + }); + + // The zod schema trims and requires a non-empty name, but the fallback is what keeps a name + // that sanitizes down to nothing from becoming a key ending in '/' and an empty File name. + it.each([ + [' ', 'file'], + ['%%%', '___'], + ])('names an object %p as %p rather than leaving it empty', async (filename, expected) => { + setup(); + + const response = await call({ filename, mimeType: 'text/plain' }); + + expect(claimsOf(response.fileHandle).name).toBe(expected); + }); + + it('rejects a call with no authenticated user', async () => { + setup(); + + const result = await callExpectingError( + { + filename: 'report.pdf', + mimeType: 'application/pdf', + }, + { authInfo: { token: 't', extra: {} } } as unknown as typeof authenticatedExtra, + ); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('without an authenticated user'); + }); + + it('rejects when no storage backend is configured', async () => { + setup(false); + + const result = await callExpectingError({ + filename: 'report.pdf', + mimeType: 'application/pdf', + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('File uploads are not configured on this server'); + }); + }); +}); diff --git a/packages/mcp-server/test/utils/load-file-uploads.test.ts b/packages/mcp-server/test/utils/load-file-uploads.test.ts new file mode 100644 index 0000000000..59930c0503 --- /dev/null +++ b/packages/mcp-server/test/utils/load-file-uploads.test.ts @@ -0,0 +1,119 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import loadFileUploads from '../../src/utils/load-file-uploads'; + +describe('loadFileUploads', () => { + const written: string[] = []; + + const writeModule = (body: string) => { + const file = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-uploads-')), 'storage.js'); + fs.writeFileSync(file, body); + written.push(file); + + return file; + }; + + const storageSource = `{ + createUploadUrl: async () => ({ url: 'https://storage.example/put' }), + download: async () => Buffer.from('x'), + getSize: async () => undefined, + }`; + + afterAll(() => written.forEach(file => fs.rmSync(path.dirname(file), { recursive: true }))); + + it('returns undefined when no module is configured', async () => { + await expect(loadFileUploads(undefined)).resolves.toBeUndefined(); + await expect(loadFileUploads('')).resolves.toBeUndefined(); + }); + + it('loads options exported directly', async () => { + const file = writeModule(`module.exports = { storage: ${storageSource}, maxBytes: 4242 };`); + + const options = await loadFileUploads(file); + + expect(options?.maxBytes).toBe(4242); + expect(typeof options?.storage.download).toBe('function'); + }); + + it('loads options returned by a function', async () => { + const file = writeModule(`module.exports = () => ({ storage: ${storageSource} });`); + + const options = await loadFileUploads(file); + + expect(typeof options?.storage.createUploadUrl).toBe('function'); + }); + + it('awaits a function returning a promise', async () => { + const file = writeModule(`module.exports = async () => ({ storage: ${storageSource} });`); + + await expect(loadFileUploads(file)).resolves.toMatchObject({ + storage: expect.any(Object), + }); + }); + + it('names the module and the resolved path when it cannot be found', async () => { + await expect(loadFileUploads('./does-not-exist.js')).rejects.toThrow( + /"\.\/does-not-exist\.js" was not found \(resolved to .+\)/, + ); + }); + + // "your path is wrong" and "your module rejected its own config" send the operator elsewhere. + it('separates a module that ran and failed from one that is absent', async () => { + const file = writeModule(`throw new Error('FOREST_S3_BUCKET is not set');`); + + const error = (await loadFileUploads(file).catch((e: Error) => e)) as Error & { + cause?: unknown; + }; + + expect(error.message).toContain('failed while loading: FOREST_S3_BUCKET is not set'); + expect(error.message).not.toContain('was not found'); + expect(error.cause).toBeInstanceOf(Error); + }); + + it('names the module when its factory rejects', async () => { + const file = writeModule(`module.exports = async () => { throw new Error('no bucket'); };`); + + await expect(loadFileUploads(file)).rejects.toThrow( + /failed while building the options: no bucket/, + ); + }); + + it('describes a module that throws something that is not an Error', async () => { + const file = writeModule(`throw 'nope';`); + + await expect(loadFileUploads(file)).rejects.toThrow(/failed while loading: nope/); + }); + + // The variable exists for the storage, so a storage that is present but broken must not fall + // back to the very backend the operator was configuring their way out of. + it.each([ + ['undefined', 'module.exports = { storage: undefined };'], + ['not an object', 'module.exports = { storage: "s3" };'], + [ + 'missing getSize', + 'module.exports = { storage: { createUploadUrl: () => {}, download: () => {} } };', + ], + ])('rejects a storage that is %s instead of falling back to memory', async (_, body) => { + const file = writeModule(body); + + await expect(loadFileUploads(file)).rejects.toThrow(/exports a storage missing/); + }); + + // Omitting the storage selects the in-memory store, so it must not be an error. + it('accepts options without a storage', async () => { + const file = writeModule(`module.exports = { maxBytes: 10 };`); + + await expect(loadFileUploads(file)).resolves.toEqual({ maxBytes: 10 }); + }); + + it.each([ + ['a function returning nothing', 'module.exports = () => undefined;'], + ['a module exporting null', 'module.exports = null;'], + ])('rejects %s, which is a module that forgot to export', async (_, body) => { + const file = writeModule(body); + + await expect(loadFileUploads(file)).rejects.toThrow('must export the fileUploads options'); + }); +}); diff --git a/packages/plugin-aws-s3/src/types.ts b/packages/plugin-aws-s3/src/types.ts index e229ab0257..09064ee743 100644 --- a/packages/plugin-aws-s3/src/types.ts +++ b/packages/plugin-aws-s3/src/types.ts @@ -10,12 +10,7 @@ import type { import type CollectionCustomizationContext from '@forestadmin/datasource-customizer/dist/context/collection-context'; import type WriteCustomizationContext from '@forestadmin/datasource-customizer/dist/decorators/write/write-replace/context'; -export type File = { - name: string; - buffer: Buffer; - mimeType: string; - charset?: string; -}; +export type { File } from '@forestadmin/datasource-toolkit'; /** * Configuration for the AWS S3 addon of Forest Admin. diff --git a/packages/plugin-aws-s3/src/utils/data-uri.ts b/packages/plugin-aws-s3/src/utils/data-uri.ts index 42893cb009..3f0c7e4a25 100644 --- a/packages/plugin-aws-s3/src/utils/data-uri.ts +++ b/packages/plugin-aws-s3/src/utils/data-uri.ts @@ -1,37 +1 @@ -import type { File } from '../types'; - -export function parseDataUri(dataUri: string): File { - if (!dataUri) return null; - - const [header, data] = dataUri.substring(5).split(','); - const [mimeType, ...mediaTypes] = header.split(';'); - const result = { mimeType, buffer: Buffer.from(data, 'base64') }; - - for (const mediaType of mediaTypes) { - const index = mediaType.indexOf('='); - - if (index !== -1) { - result[mediaType.substring(0, index)] = decodeURIComponent(mediaType.substring(index + 1)); - } - } - - return result as unknown as File; -} - -export function encodeDataUri(data: File): string { - // prefix - let uri = `data:${data.mimeType}`; - - // media types - const mediaTypes = Object.entries(data) - .filter(([mediaType, value]) => value && mediaType !== 'mimeType' && mediaType !== 'buffer') - .map(([mediaType, value]) => `${mediaType}=${encodeURIComponent(value as string)}`) - .join(';'); - - if (mediaTypes.length) uri += `;${mediaTypes}`; - - // data - uri += `;base64,${data.buffer.toString('base64')}`; - - return uri; -} +export { parseDataUri, makeDataUri as encodeDataUri } from '@forestadmin/datasource-toolkit'; diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index 1245532a93..085e938984 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -61,7 +61,8 @@ export type GetActionFormQuery = { // get-action-form tool's field shape. export type ActionFormField = { name: string; - type: string; + /** Verbatim from the agent, so a list type is `['String']` rather than `'StringList'`. */ + type: string | [string]; value?: unknown; isRequired: boolean; enumValues?: string[];