From 4e15c1f493c3e5cad7db278071af3be87cd34675 Mon Sep 17 00:00:00 2001 From: Stefano Amorelli Date: Sun, 9 Aug 2026 14:32:42 +0300 Subject: [PATCH] feat(mcp-server): enable action file fields via an upload side-channel The agent expects action file fields as base64 data URIs, which cannot pass through an MCP client. The payload would transit the model's context window and exceed most clients' body limits, so actions with File fields could not run over MCP at all. Add an opt-in fileUploads option backed by a pluggable UploadStorage interface. POST /files (bearer-protected) returns a pre-authorized upload URL plus a signed handle bound to the requesting user. executeAction swaps "$uploadedFile:" values for the data URI before calling the agent, so the model only ever exchanges the small handle and the bytes bypass both the server and the model on upload. The handle is a JWT signed with authSecret, so the server stays stateless and horizontally scalable. Redemption enforces the size cap (a pre-authorized upload URL cannot always cap the object size), re-verifies an optional sha256 pin against the downloaded bytes so substituted content cannot be redeemed, and runs under a per-process concurrency bound so worst-case memory stays at maxBytes times the slot count. getActionForm leaves handles unresolved on purpose because it echoes values back into the model's context. The route matcher only claims /files when the feature is enabled, so a host app's own /files keeps working otherwise. Signed-off-by: Stefano Amorelli --- packages/mcp-server/CLAUDE.md | 1 + packages/mcp-server/README.md | 88 +++++++ .../mcp-server/src/file-uploads/handles.ts | 64 ++++++ .../mcp-server/src/file-uploads/resolve.ts | 122 ++++++++++ .../mcp-server/src/file-uploads/routes.ts | 117 ++++++++++ .../mcp-server/src/file-uploads/semaphore.ts | 39 ++++ packages/mcp-server/src/file-uploads/types.ts | 91 ++++++++ packages/mcp-server/src/index.ts | 1 + packages/mcp-server/src/mcp-paths.ts | 20 +- packages/mcp-server/src/server.ts | 44 +++- packages/mcp-server/src/tool-context.ts | 2 + .../mcp-server/src/tools/execute-action.ts | 19 +- .../test/file-uploads/handles.test.ts | 69 ++++++ .../test/file-uploads/resolve.test.ts | 217 ++++++++++++++++++ .../test/file-uploads/routes.test.ts | 183 +++++++++++++++ packages/mcp-server/test/mcp-paths.test.ts | 22 ++ .../test/tools/execute-action.test.ts | 129 +++++++++++ 17 files changed, 1216 insertions(+), 12 deletions(-) create mode 100644 packages/mcp-server/src/file-uploads/handles.ts create mode 100644 packages/mcp-server/src/file-uploads/resolve.ts create mode 100644 packages/mcp-server/src/file-uploads/routes.ts create mode 100644 packages/mcp-server/src/file-uploads/semaphore.ts create mode 100644 packages/mcp-server/src/file-uploads/types.ts create mode 100644 packages/mcp-server/test/file-uploads/handles.test.ts create mode 100644 packages/mcp-server/test/file-uploads/resolve.test.ts create mode 100644 packages/mcp-server/test/file-uploads/routes.test.ts diff --git a/packages/mcp-server/CLAUDE.md b/packages/mcp-server/CLAUDE.md index 23cadc4e31..1b59793edf 100644 --- a/packages/mcp-server/CLAUDE.md +++ b/packages/mcp-server/CLAUDE.md @@ -17,6 +17,7 @@ Key flows that only make sense across files: - **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"). - **`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()`. +- **Action file uploads are a side-channel** (`src/file-uploads/`), enabled by the `fileUploads` option with a host-provided `UploadStorage` backend. `POST /files` (bearer-protected, only mounted when enabled) returns a pre-authorized upload URL plus a user-bound JWT handle signed with `authSecret`. `executeAction` swaps `"$uploadedFile:"` values for the base64 data URI the agent expects (`resolve.ts`) and enforces `maxBytes`, the optional sha256 pin, and a per-process download concurrency bound at redemption. `getActionForm` leaves handles unresolved on purpose because it echoes values back into the model's context. When enabled, `makeIsMcpRoute(prefix, { fileUploads: true })` also claims `/files`. ## Commands diff --git a/packages/mcp-server/README.md b/packages/mcp-server/README.md index 8757681d97..0785b0f4d4 100644 --- a/packages/mcp-server/README.md +++ b/packages/mcp-server/README.md @@ -164,6 +164,93 @@ 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 + +Actions with **File fields** cannot normally run over MCP. The agent expects file values as base64 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 `POST`s `/files` (same Bearer token as `/mcp`) 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 redeems it by downloading the object, re-encoding it as the data URI the agent expects, and forwarding it. The model only ever exchanges the small handle. + +```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: POST /files {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 as a data URI + Agent-->>Server: action result + Server-->>Client: result (the model only saw the handle) +``` + +The storage backend is pluggable. The server itself has no storage dependency. You provide an implementation of the `UploadStorage` interface, and any backend that can pre-authorize an upload and read the object back works, such as S3 presigned URLs (shown below), GCS signed URLs, Azure SAS, or a local endpoint you serve yourself. + +```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, + ...(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), and `maxConcurrentDownloads` (default 5). + +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`. +- 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. +- A pre-authorized upload URL cannot always cap the object size, so `maxBytes` is enforced at redemption (before download when the backend implements `getSize`). Each redemption holds up to the file plus its base64 copy in memory, and `maxConcurrentDownloads` bounds the process's worst case to roughly `maxBytes × 2.3 × maxConcurrentDownloads`. +- The server never deletes objects. Configure a lifecycle rule on the storage backend, for example deleting objects under `keyPrefix` after one day. Handles cannot be revoked before they expire, so keep `handleTtlSeconds` short. + +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. + ## API Endpoints Once running, the MCP server exposes the following endpoints: @@ -171,6 +258,7 @@ Once running, the MCP server exposes the following endpoints: | Method | Path | Description | |--------|------|-------------| | POST | `/mcp` | Main MCP protocol endpoint (requires Bearer token) | +| POST | `/files` | Upload side-channel for action file fields (only with `fileUploads`; requires Bearer token) | | POST | `/oauth/authorize` | OAuth 2.0 authorization | | POST | `/oauth/token` | OAuth 2.0 token exchange | | GET | `/.well-known/oauth-protected-resource/mcp` | OAuth metadata discovery | 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..a4ba40d225 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/handles.ts @@ -0,0 +1,64 @@ +import jsonwebtoken from 'jsonwebtoken'; + +/** + * Sentinel prefix used inside action form values, e.g. + * { "document": "$uploadedFile:" } + * A string (not an object) so it passes the agent-client's field validation and stays + * cheap when echoed back by getActionForm. + */ +export const UPLOADED_FILE_PREFIX = '$uploadedFile:'; + +const HANDLE_TYPE = 'mcp-upload'; + +export interface UploadHandleClaims { + key: string; + name: string; + mimeType: string; + /** Base64 sha256 the upload was pinned to, when the client provided one. */ + sha256?: 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.sha256 && { sha256: claims.sha256 }), + }, + authSecret, + { expiresIn: ttlSeconds }, + ); +} + +/** Throws on tampered, expired, or cross-user handles. */ +export function verifyUploadHandle( + handle: string, + userId: number | string, + authSecret: string, +): UploadHandleClaims { + const decoded = jsonwebtoken.verify(handle, authSecret) as { + type?: string; + key: string; + name: string; + mime: string; + uploader?: string; + sha256?: string; + }; + + 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'); + + return { + key: decoded.key, + name: decoded.name, + mimeType: decoded.mime, + sha256: 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..63793e25d7 --- /dev/null +++ b/packages/mcp-server/src/file-uploads/resolve.ts @@ -0,0 +1,122 @@ +import type { ResolvedFileUploads } from './types'; +import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; + +import * as crypto from 'crypto'; + +import { UPLOADED_FILE_PREFIX, verifyUploadHandle } from './handles'; + +function isHandle(value: unknown): value is string { + return typeof value === 'string' && value.startsWith(UPLOADED_FILE_PREFIX); +} + +function collectHandles(values: Record): Set { + const handles = new Set(); + + for (const value of Object.values(values)) { + if (isHandle(value)) handles.add(value); + else if (Array.isArray(value)) value.filter(isHandle).forEach(v => handles.add(v as string)); + } + + return handles; +} + +async function loadAsDataUri( + handle: string, + userId: number | string, + uploads: ResolvedFileUploads, +): Promise { + const claims = verifyUploadHandle( + handle.slice(UPLOADED_FILE_PREFIX.length), + userId, + uploads.authSecret, + ); + + // A pre-authorized upload URL cannot always cap the object size, so the limit is + // enforced here, before the bytes are read when the backend can report a size. + const size = await uploads.storage.getSize?.(claims.key); + + if (size !== undefined && size > uploads.maxBytes) { + throw new Error(`Uploaded file is ${size} bytes, above the ${uploads.maxBytes} byte limit`); + } + + const buffer = await uploads.storage.download(claims.key); + + if (buffer.length === 0) { + throw new Error('Uploaded file is empty. Did the upload to uploadUrl succeed?'); + } + + if (buffer.length > uploads.maxBytes) { + throw new Error( + `Uploaded file is ${buffer.length} bytes, above the ${uploads.maxBytes} byte limit`, + ); + } + + // When the handle carries a sha256, re-verify the digest on the downloaded bytes. Even + // if the upload URL leaked and someone overwrote the object, substituted content cannot + // be redeemed. + if (claims.sha256) { + const digest = crypto.createHash('sha256').update(new Uint8Array(buffer)).digest('base64'); + + if (digest !== claims.sha256) { + throw new Error('Uploaded file does not match the sha256 it was pinned to'); + } + } + + return `data:${claims.mimeType};name=${claims.name};base64,${buffer.toString('base64')}`; +} + +/** + * Replaces "$uploadedFile:" values in action form values with the uploaded + * object re-encoded as the data URI the agent expects for File fields. The model only + * ever exchanges the small handle. The base64 payload exists in memory here and in the + * outbound call to the agent. + * + * Handles are resolved concurrently and deduplicated, so a handle referenced by several + * fields is downloaded once. + * + * Only executeAction resolves handles. getActionForm echoes field values back to + * the model, and a resolved data URI there would put the file content back into + * the model's context. + */ +export default async function resolveUploadedFileValues( + values: Record, + authInfo: AuthInfo | undefined, + uploads: ResolvedFileUploads | undefined, +): Promise> { + const handles = collectHandles(values); + + if (handles.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'); + } + + const dataUris = new Map( + await Promise.all( + [...handles].map( + async (handle): Promise<[string, string]> => [ + handle, + await uploads.limitDownload(() => loadAsDataUri(handle, userId, uploads)), + ], + ), + ), + ); + + const substitute = (value: unknown) => (isHandle(value) ? dataUris.get(value) : 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/routes.ts b/packages/mcp-server/src/file-uploads/routes.ts new file mode 100644 index 0000000000..f4ab69d0ae --- /dev/null +++ b/packages/mcp-server/src/file-uploads/routes.ts @@ -0,0 +1,117 @@ +import type { ResolvedFileUploads } from './types'; +import type { Logger } from '../server'; +import type { Request, Response, Router } from 'express'; + +import * as crypto from 'crypto'; +import express from 'express'; + +import { UPLOADED_FILE_PREFIX, signUploadHandle } from './handles'; + +const MIME_TYPE_PATTERN = /^[\w.+-]+\/[\w.+-]+$/; +const SHA256_HEX_PATTERN = /^[0-9a-f]{64}$/i; +const SHA256_BASE64_PATTERN = /^[A-Za-z0-9+/]{43}=$/; + +// The data URI format delimits the filename with ';' and ',', and the storage key with +// '/'. Keep a conservative charset so neither parser can be confused. +function sanitizeFilename(filename: string): string { + return filename + .trim() + .slice(-128) + .replace(/[^\w.\- ()]/g, '_'); +} + +// Accepts the digest as hex (shasum -a 256 output) or base64. Returns base64, +// null when the digest is absent, or false when it is malformed. +function normalizeSha256(sha256: unknown): string | null | false { + if (sha256 === undefined || sha256 === null || sha256 === '') return null; + if (typeof sha256 !== 'string') return false; + if (SHA256_BASE64_PATTERN.test(sha256)) return sha256; + if (SHA256_HEX_PATTERN.test(sha256)) return Buffer.from(sha256, 'hex').toString('base64'); + + return false; +} + +/** + * Router for POST /files, the upload half of the action file side-channel. + * + * Must be mounted behind requireBearerAuth so req.auth carries the caller's identity. + * The returned handle is bound to that user and can only be redeemed by them. + */ +export default function createFilesRouter(uploads: ResolvedFileUploads, logger: Logger): Router { + const router = express.Router(); + + router.post('/', async (req: Request, res: Response) => { + const userId = req.auth?.extra?.userId; + + if (userId === undefined || userId === null) { + res.status(401).json({ error: 'Missing or invalid access token.' }); + + return; + } + + const { filename, mimeType, sha256 } = (req.body ?? {}) as Record; + + if (typeof filename !== 'string' || !filename.trim()) { + res.status(400).json({ error: 'filename is required.' }); + + return; + } + + if (typeof mimeType !== 'string' || !MIME_TYPE_PATTERN.test(mimeType)) { + res.status(400).json({ error: 'mimeType is required, e.g. application/pdf.' }); + + return; + } + + const sha256Base64 = normalizeSha256(sha256); + + if (sha256Base64 === false) { + res.status(400).json({ error: 'sha256 must be the file digest as hex or base64.' }); + + return; + } + + const safeName = sanitizeFilename(filename); + const key = `${uploads.keyPrefix}${crypto.randomUUID()}/${safeName}`; + + const destination = await uploads.storage.createUploadUrl({ + key, + mimeType, + ...(sha256Base64 && { sha256: sha256Base64 }), + expiresInSeconds: uploads.uploadUrlTtlSeconds, + }); + + const handle = signUploadHandle( + { + key, + name: safeName, + mimeType, + userId: userId as number | string, + ...(sha256Base64 && { sha256: sha256Base64 }), + }, + uploads.authSecret, + uploads.handleTtlSeconds, + ); + + res.json({ + uploadUrl: destination.url, + method: destination.method ?? 'PUT', + headers: destination.headers ?? { 'Content-Type': mimeType }, + expiresInSeconds: uploads.uploadUrlTtlSeconds, + maxBytes: uploads.maxBytes, + fileHandle: `${UPLOADED_FILE_PREFIX}${handle}`, + usage: + 'Upload the raw file bytes to uploadUrl with the given method and headers, ' + + 'then pass fileHandle as the value of the action file field in executeAction. ' + + 'Provide sha256 in the request to pin the upload to that exact content.', + }); + }); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars -- error handlers need arity 4 + router.use((error: Error, req: Request, res: Response, next: express.NextFunction) => { + logger('Error', `/files error: ${error.message}`); + res.status(500).json({ error: 'Failed to create upload URL.' }); + }); + + return router; +} 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..8f0c6a28de --- /dev/null +++ b/packages/mcp-server/src/file-uploads/semaphore.ts @@ -0,0 +1,39 @@ +/** + * Minimal counting semaphore, used to bound concurrent handle redemptions. Each + * redemption can hold up to maxBytes plus its base64 copy in memory, so the process's + * worst case stays bounded by maxBytes * limit instead of by whatever load arrives. + */ +export type RunExclusive = (task: () => Promise) => Promise; + +export default function createSemaphore(limit: number): RunExclusive { + let active = 0; + const queue: Array<() => void> = []; + + const acquire = () => + new Promise(resolve => { + if (active < limit) { + active += 1; + resolve(); + } else { + queue.push(resolve); + } + }); + + const release = () => { + const next = queue.shift(); + + // When a task is waiting, the slot transfers to it and 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..aa914d750a --- /dev/null +++ b/packages/mcp-server/src/file-uploads/types.ts @@ -0,0 +1,91 @@ +import type { RunExclusive } from './semaphore'; + +import createSemaphore from './semaphore'; + +/** + * Storage backend for the action file upload side-channel. + * + * Implementations are provided by the host application (S3, GCS, local disk...). + * The MCP server never sees the file bytes during upload. Clients PUT them straight + * to the URL returned by createUploadUrl, and the server only reads them back when + * an executeAction call redeems the handle. + */ +export interface UploadStorage { + /** Return a pre-authorized URL the client can upload a single object to. */ + createUploadUrl(params: { + /** Storage key the object must land under. */ + key: string; + mimeType: string; + /** Base64 sha256 digest the upload must match, when the client pinned one. */ + sha256?: string; + expiresInSeconds: number; + }): Promise<{ url: string; method?: string; headers?: Record }>; + + /** Read the uploaded object back. Must reject when the object does not exist. */ + download(key: string): Promise; + + /** + * Optional size probe used to reject oversized uploads without downloading them. + * Return undefined when the size cannot be known cheaply. + */ + getSize?(key: string): Promise; +} + +/** Options for the `fileUploads` server option. */ +export interface FileUploadsOptions { + storage: UploadStorage; + /** Key prefix for uploaded objects. Defaults to 'mcp-uploads/'. */ + keyPrefix?: string; + /** Lifetime of the upload URL. Defaults to 15 minutes. */ + uploadUrlTtlSeconds?: number; + /** + * Lifetime of the file handle. Defaults to 45 minutes, longer than the upload URL, + * so a slow upload still leaves time to run the action. + */ + handleTtlSeconds?: number; + /** Maximum uploaded file size, enforced when the handle is redeemed. Defaults to 20 MiB. */ + maxBytes?: number; + /** + * Maximum handle redemptions running at once per process. Each redemption may hold up + * to maxBytes plus its base64 copy in memory, so this bounds the worst case to + * maxBytes * maxConcurrentDownloads. Defaults to 5. + */ + maxConcurrentDownloads?: number; +} + +/** FileUploadsOptions with defaults applied and the auth secret attached. */ +export interface ResolvedFileUploads { + storage: UploadStorage; + keyPrefix: string; + uploadUrlTtlSeconds: number; + handleTtlSeconds: number; + maxBytes: number; + authSecret: string; + /** Runs a redemption inside the process-wide concurrency bound. */ + 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; + +export function resolveFileUploads( + options: FileUploadsOptions | undefined, + authSecret: string, +): ResolvedFileUploads | undefined { + if (!options) return undefined; + + return { + storage: options.storage, + keyPrefix: options.keyPrefix ?? DEFAULT_KEY_PREFIX, + uploadUrlTtlSeconds: options.uploadUrlTtlSeconds ?? DEFAULT_UPLOAD_URL_TTL_SECONDS, + handleTtlSeconds: options.handleTtlSeconds ?? DEFAULT_HANDLE_TTL_SECONDS, + maxBytes: options.maxBytes ?? DEFAULT_MAX_BYTES, + authSecret, + limitDownload: createSemaphore( + 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/mcp-paths.ts b/packages/mcp-server/src/mcp-paths.ts index d1eaf3aeb0..61a8fd3cfb 100644 --- a/packages/mcp-server/src/mcp-paths.ts +++ b/packages/mcp-server/src/mcp-paths.ts @@ -22,11 +22,16 @@ export function normalizeMountPath(input?: string): string { return collapsed; } +export interface McpRouteOptions { + /** Claim the /files upload route too. Only set when the fileUploads option is enabled. */ + fileUploads?: boolean; +} + /** * Well-known paths stay anchored at the origin root (per RFC 8414/9728) but carry the prefix * as a suffix, so a host's own root OAuth metadata is not claimed. */ -export function buildMcpPaths(prefix = ''): string[] { +export function buildMcpPaths(prefix = '', options: McpRouteOptions = {}): string[] { const normalized = normalizeMountPath(prefix); const wellKnown = normalized @@ -36,11 +41,18 @@ export function buildMcpPaths(prefix = ''): string[] { ] : ['/.well-known/']; - return [...wellKnown, `${normalized}/oauth/`, `${normalized}/mcp`]; + return [ + ...wellKnown, + `${normalized}/oauth/`, + `${normalized}/mcp`, + // /files is only claimed when uploads are enabled, so a host app's own /files + // route keeps working otherwise. + ...(options.fileUploads ? [`${normalized}/files`] : []), + ]; } -export function makeIsMcpRoute(prefix = ''): McpRouteMatcher { - const paths = buildMcpPaths(prefix); +export function makeIsMcpRoute(prefix = '', options: McpRouteOptions = {}): McpRouteMatcher { + const paths = buildMcpPaths(prefix, options); // Match on the pathname (req.url carries the query string) and on a segment boundary, so // '/mcp?x=1' still matches and '/ai/mcp' does not shadow '/ai/mcp-dashboard'. diff --git a/packages/mcp-server/src/server.ts b/packages/mcp-server/src/server.ts index dab57f6252..b2738ead0c 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 createFilesRouter from './file-uploads/routes'; +import { resolveFileUploads } from './file-uploads/types'; import ForestOAuthProvider from './forest-oauth-provider'; import { createForestServerClient } from './http-client'; import { makeIsMcpRoute, normalizeMountPath } from './mcp-paths'; @@ -154,6 +157,16 @@ export interface ForestMCPServerOptions { * Omit to accept any dynamically registered client. */ allowedOAuthClients?: string[]; + /** + * Enables file fields in action forms via an upload side-channel. Without it, action file + * fields are unusable over MCP, because the agent expects them as base64 data URIs, which + * would transit the model's context window and exceed most clients' payload limits. When + * set, POST /files returns a pre-authorized upload URL plus a signed handle, and + * executeAction swaps "$uploadedFile:" values for the data URI before calling the + * agent, so the model only ever exchanges the small handle. Requires a storage backend + * implementation. + */ + fileUploads?: FileUploadsOptions; } /** @@ -180,6 +193,8 @@ export default class ForestMCPServer { private agentDispatcher?: InProcessAgentDispatcher; private tokenTtl?: TokenTtlOptions; private allowedOAuthClients?: string[]; + private fileUploadsOptions?: FileUploadsOptions; + private fileUploads?: ResolvedFileUploads; constructor(options?: ForestMCPServerOptions) { this.forestServerUrl = options?.forestServerUrl || 'https://api.forestadmin.com'; @@ -194,6 +209,8 @@ export default class ForestMCPServer { this.tokenTtl = normalizeTokenTtl(options?.tokenTtl, this.logger); this.allowedOAuthClients = normalizeDomainList(options?.allowedOAuthClients); + // Resolution waits for buildExpressApp, where the auth secret is known to be set. + this.fileUploadsOptions = options?.fileUploads; // Use injected forestServerClient or create default this.forestServerClient = options?.forestServerClient ?? this.createDefaultForestServerClient(); @@ -230,6 +247,7 @@ export default class ForestMCPServer { logger: this.logger, collectionNames: this.collectionNames, agentDispatcher: this.agentDispatcher, + fileUploads: this.fileUploads, }; const allTools: Array<{ name: ToolName; register: () => string }> = [ @@ -428,6 +446,8 @@ export default class ForestMCPServer { async buildExpressApp(baseUrl?: URL): Promise { const { envSecret, authSecret } = this.ensureSecretsAreSet(); + this.fileUploads = resolveFileUploads(this.fileUploadsOptions, authSecret); + await this.fetchCollectionNames(); const app = express(); @@ -559,15 +579,29 @@ export default class ForestMCPServer { app.use(allowedMethods(['POST'])); + const resourceMetadataUrl = new URL( + `/.well-known/oauth-protected-resource${mcpResourceUrl.pathname}`, + effectiveBaseUrl, + ).href; + + if (this.fileUploads) { + app.use( + `${prefix}/files`, + requireBearerAuth({ + verifier: oauthProvider, + requiredScopes: ['mcp:action'], + resourceMetadataUrl, + }), + createFilesRouter(this.fileUploads, this.logger), + ); + } + 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 => { @@ -636,7 +670,7 @@ export default class ForestMCPServer { */ async getHttpCallback(baseUrl?: URL): Promise { const app = await this.buildExpressApp(baseUrl); - const isMcpRoute = makeIsMcpRoute(this.basePath); + const isMcpRoute = makeIsMcpRoute(this.basePath, { fileUploads: Boolean(this.fileUploads) }); return (req, res, next) => { const url = req.url || '/'; 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..5cbc93e0d8 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. Request an upload destination via POST /files (same Bearer token, JSON body with "filename", "mimeType" and optionally "sha256"), upload the raw bytes to the returned uploadUrl, and pass the returned fileHandle string as the field value.` + : '' + }`, inputSchema: argumentShape, }, async (options: ExecuteActionArgument, extra) => { @@ -54,6 +61,12 @@ If you call executeAction with missing required fields, it will return an error // Cast to satisfy the type system - the API accepts both string[] and number[] const recordIds = (options.recordIds ?? []) as string[] | number[]; + // Swap "$uploadedFile:" values for the data URIs the agent expects, + // before the values reach the form. This is a no-op when no value carries a handle. + const values = options.values + ? await resolveUploadedFileValues(options.values, extra.authInfo, ctx.fileUploads) + : undefined; + return withActivityLog({ forestServerClient, request: extra, @@ -69,8 +82,8 @@ If you call executeAction with missing required fields, it will return an error .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/test/file-uploads/handles.test.ts b/packages/mcp-server/test/file-uploads/handles.test.ts new file mode 100644 index 0000000000..18c3d17c4c --- /dev/null +++ b/packages/mcp-server/test/file-uploads/handles.test.ts @@ -0,0 +1,69 @@ +import jsonwebtoken from 'jsonwebtoken'; + +import { + UPLOADED_FILE_PREFIX, + 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('exports the sentinel prefix used inside action values', () => { + expect(UPLOADED_FILE_PREFIX).toBe('$uploadedFile:'); + }); + + describe('signUploadHandle / verifyUploadHandle', () => { + 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', + sha256: undefined, + }); + }); + + it('carries the sha256 pin when provided', () => { + const handle = signUploadHandle({ ...claims, sha256: 'digest==' }, AUTH_SECRET, 60); + + expect(verifyUploadHandle(handle, 42, AUTH_SECRET).sha256).toBe('digest=='); + }); + + 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 token signed with another secret', () => { + const handle = signUploadHandle(claims, 'other-secret', 60); + + expect(() => verifyUploadHandle(handle, 42, AUTH_SECRET)).toThrow('invalid signature'); + }); + + it('rejects a JWT that is not an upload handle', () => { + const accessTokenLookalike = jsonwebtoken.sign({ id: 42 }, AUTH_SECRET, { expiresIn: 60 }); + + expect(() => verifyUploadHandle(accessTokenLookalike, 42, AUTH_SECRET)).toThrow( + 'Not an 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..6242a74d6e --- /dev/null +++ b/packages/mcp-server/test/file-uploads/resolve.test.ts @@ -0,0 +1,217 @@ +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, 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('file content')), + ...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 values untouched when no value carries a handle', 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 handle 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 handle to', async () => { + await expect( + resolveUploadedFileValues({ document: makeHandle() }, undefined, makeUploads(makeStorage())), + ).rejects.toThrow('Cannot resolve uploaded files without an authenticated user'); + }); + + it('replaces a handle with the data URI of the uploaded object', async () => { + const storage = makeStorage({ download: jest.fn().mockResolvedValue(Buffer.from('%PDF-1.4')) }); + + const resolved = await resolveUploadedFileValues( + { document: makeHandle(), note: 'untouched' }, + authInfo, + makeUploads(storage), + ); + + expect(storage.download).toHaveBeenCalledWith('mcp-uploads/uuid/report.pdf'); + expect(resolved.document).toBe( + `data:application/pdf;name=report.pdf;base64,${Buffer.from('%PDF-1.4').toString('base64')}`, + ); + expect(resolved.note).toBe('untouched'); + }); + + it('resolves handles inside array values and leaves other entries alone', async () => { + const storage = makeStorage(); + + const resolved = await resolveUploadedFileValues( + { attachments: [makeHandle(), 'existing-value'] }, + authInfo, + makeUploads(storage), + ); + + expect(resolved.attachments).toEqual([ + expect.stringMatching(/^data:application\/pdf;name=report\.pdf;base64,/), + 'existing-value', + ]); + }); + + it('downloads a handle referenced 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'); + }); + + it('rejects an empty upload with a hint about the PUT step', async () => { + const storage = makeStorage({ download: jest.fn().mockResolvedValue(Buffer.alloc(0)) }); + + await expect( + resolveUploadedFileValues({ document: makeHandle() }, authInfo, makeUploads(storage)), + ).rejects.toThrow('Uploaded file is empty'); + }); + + 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({ sha256: pinned }) }, + authInfo, + makeUploads(storage), + ), + ).rejects.toThrow('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({ sha256: pinned }) }, + authInfo, + makeUploads(storage), + ); + + expect(resolved.document).toMatch(/^data:application\/pdf/); + }); + + 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); + }); +}); diff --git a/packages/mcp-server/test/file-uploads/routes.test.ts b/packages/mcp-server/test/file-uploads/routes.test.ts new file mode 100644 index 0000000000..c4d489409b --- /dev/null +++ b/packages/mcp-server/test/file-uploads/routes.test.ts @@ -0,0 +1,183 @@ +import type { UploadStorage } from '../../src/file-uploads/types'; +import type { Logger } from '../../src/server'; +import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; +import type { Express } from 'express'; + +import express from 'express'; +import request from 'supertest'; + +import { verifyUploadHandle } from '../../src/file-uploads/handles'; +import createFilesRouter from '../../src/file-uploads/routes'; +import { resolveFileUploads } from '../../src/file-uploads/types'; + +const AUTH_SECRET = 'test-auth-secret'; +const mockLogger: Logger = jest.fn(); + +function makeApp(options: { + storage?: UploadStorage; + auth?: AuthInfo | undefined; + keyPrefix?: string; +}): { app: Express; storage: UploadStorage } { + const storage: UploadStorage = options.storage ?? { + createUploadUrl: jest.fn().mockResolvedValue({ + url: 'https://storage.example/put?signed=1', + headers: { 'Content-Type': 'application/pdf' }, + }), + download: jest.fn(), + }; + + const uploads = resolveFileUploads( + { storage, ...(options.keyPrefix ? { keyPrefix: options.keyPrefix } : {}) }, + AUTH_SECRET, + ); + + const app = express(); + app.use(express.json()); + + // Stands in for requireBearerAuth, which attaches the verified AuthInfo to req.auth. + app.use((req, res, next) => { + req.auth = options.auth; + next(); + }); + + app.use('/files', createFilesRouter(uploads, mockLogger)); + + return { app, storage }; +} + +const authenticatedUser = { + token: 'token', + clientId: '42', + scopes: ['mcp:action'], + extra: { userId: 42 }, +} as unknown as AuthInfo; + +describe('POST /files', () => { + it('returns 401 when no authenticated user is attached to the request', async () => { + const { app } = makeApp({ auth: undefined }); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.status).toBe(401); + expect(response.body).toEqual({ error: 'Missing or invalid access token.' }); + }); + + it('returns 400 when filename is missing or blank', async () => { + const { app } = makeApp({ auth: authenticatedUser }); + + const response = await request(app) + .post('/files') + .send({ filename: ' ', mimeType: 'application/pdf' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'filename is required.' }); + }); + + it('returns 400 when mimeType is not a type/subtype pair', async () => { + const { app } = makeApp({ auth: authenticatedUser }); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'not a mime type' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'mimeType is required, e.g. application/pdf.' }); + }); + + it('returns 400 when sha256 is neither hex nor base64', async () => { + const { app } = makeApp({ auth: authenticatedUser }); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'application/pdf', sha256: 'nope' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'sha256 must be the file digest as hex or base64.' }); + }); + + it('returns an upload destination and a redeemable handle', async () => { + const { app, storage } = makeApp({ auth: authenticatedUser }); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + uploadUrl: 'https://storage.example/put?signed=1', + method: 'PUT', + headers: { 'Content-Type': 'application/pdf' }, + expiresInSeconds: 15 * 60, + maxBytes: 20 * 1024 * 1024, + }); + expect(response.body.fileHandle).toMatch(/^\$uploadedFile:/); + expect(response.body.usage).toContain('executeAction'); + + expect(storage.createUploadUrl).toHaveBeenCalledWith({ + key: expect.stringMatching(/^mcp-uploads\/[0-9a-f-]{36}\/report\.pdf$/), + mimeType: 'application/pdf', + expiresInSeconds: 15 * 60, + }); + + const claims = verifyUploadHandle( + (response.body.fileHandle as string).slice('$uploadedFile:'.length), + 42, + AUTH_SECRET, + ); + expect(claims).toMatchObject({ name: 'report.pdf', mimeType: 'application/pdf' }); + }); + + it('normalizes a hex sha256 to base64 and pins both the destination and the handle', async () => { + const { app, storage } = makeApp({ auth: authenticatedUser }); + const hex = 'a'.repeat(64); + const expectedBase64 = Buffer.from(hex, 'hex').toString('base64'); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'application/pdf', sha256: hex }); + + expect(response.status).toBe(200); + expect(storage.createUploadUrl).toHaveBeenCalledWith( + expect.objectContaining({ sha256: expectedBase64 }), + ); + + const claims = verifyUploadHandle( + (response.body.fileHandle as string).slice('$uploadedFile:'.length), + 42, + AUTH_SECRET, + ); + expect(claims.sha256).toBe(expectedBase64); + }); + + it('sanitizes filenames so they cannot confuse the data URI or the storage key', async () => { + const { app, storage } = makeApp({ auth: authenticatedUser }); + + const response = await request(app) + .post('/files') + .send({ filename: '../etc;name=x,y/passwd.pdf', mimeType: 'application/pdf' }); + + expect(response.status).toBe(200); + + const { key } = (storage.createUploadUrl as jest.Mock).mock.calls[0][0]; + const filenamePart = key.split('/').pop(); + expect(filenamePart).not.toMatch(/[;,/]/); + }); + + it('returns 500 without leaking details when the storage backend fails', async () => { + const storage: UploadStorage = { + createUploadUrl: jest.fn().mockRejectedValue(new Error('bucket is on fire')), + download: jest.fn(), + }; + const { app } = makeApp({ auth: authenticatedUser, storage }); + + const response = await request(app) + .post('/files') + .send({ filename: 'report.pdf', mimeType: 'application/pdf' }); + + expect(response.status).toBe(500); + expect(response.body).toEqual({ error: 'Failed to create upload URL.' }); + expect(mockLogger).toHaveBeenCalledWith('Error', expect.stringContaining('bucket is on fire')); + }); +}); diff --git a/packages/mcp-server/test/mcp-paths.test.ts b/packages/mcp-server/test/mcp-paths.test.ts index 398c00bf21..7ceda5ac56 100644 --- a/packages/mcp-server/test/mcp-paths.test.ts +++ b/packages/mcp-server/test/mcp-paths.test.ts @@ -66,6 +66,12 @@ describe('mcp-paths', () => { it('normalizes a raw (un-normalized) prefix on entry', () => { expect(buildMcpPaths('mcp/')).toEqual(buildMcpPaths('/mcp')); }); + + it('claims /files only when file uploads are enabled', () => { + expect(buildMcpPaths('')).not.toContain('/files'); + expect(buildMcpPaths('', { fileUploads: true })).toContain('/files'); + expect(buildMcpPaths('/ai', { fileUploads: true })).toContain('/ai/files'); + }); }); describe('default exports (root)', () => { @@ -111,4 +117,20 @@ describe('mcp-paths', () => { expect(matches(url)).toBe(false); }); }); + + describe('makeIsMcpRoute with file uploads enabled', () => { + const matches = makeIsMcpRoute('', { fileUploads: true }); + + it.each(['/files', '/files?x=1'])('claims %p', url => { + expect(matches(url)).toBe(true); + }); + + it('does not claim /files when uploads are disabled', () => { + expect(makeIsMcpRoute('')('/files')).toBe(false); + }); + + it('does not shadow sibling routes like /files-admin', () => { + expect(matches('/files-admin')).toBe(false); + }); + }); }); diff --git a/packages/mcp-server/test/tools/execute-action.test.ts b/packages/mcp-server/test/tools/execute-action.test.ts index 99e8cd2258..e8f033bb05 100644 --- a/packages/mcp-server/test/tools/execute-action.test.ts +++ b/packages/mcp-server/test/tools/execute-action.test.ts @@ -5,6 +5,8 @@ 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, 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 +554,131 @@ 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')), + }, + }, + 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: 'report.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('POST /files'); + 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('POST /files'); + }); + + it('swaps a file handle for the uploaded data URI before setting the form fields', 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: `data:application/pdf;name=report.pdf;base64,${Buffer.from('%PDF-1.4').toString( + 'base64', + )}`, + 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(); + }); + }); });