From fb510d11a2f985785bd86b575b2ce97bbd55536d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 1 Aug 2026 12:09:59 -0700 Subject: [PATCH] fix(ssh/sftp): cap remote file reads on received bytes, not stat() size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SFTP/SSH download routes buffered a remote file into memory with the only size guard being sftp.stat().size — a value the caller-supplied SSH server controls. A server that reports a tiny size and then streams endlessly drove unbounded heap growth until OOM. Read through the existing readNodeStreamToBufferWithLimit limiter, which enforces the cap on bytes actually received and destroys the stream on breach, via a small readSftpFileCapped wrapper in sftp/utils. All four SFTP-reading tool routes now share it — download, download-file, read-file-content, and the append path of write-file-content, which had no cap at all. The wrapper keeps a no-op error listener on the stream for its whole life: ssh2 rejects still-pending SFTP requests when the channel closes, which arrives as a late error event after the limiter has detached its own handlers, and an error event with no listener would take the process down. Too-large responses now return 413 to match how the rest of the routes surface PayloadSizeLimitError, and responses report the actual byte count rather than the server-reported stat size. --- apps/sim/app/api/tools/sftp/download/route.ts | 40 ++++++----- apps/sim/app/api/tools/sftp/utils.test.ts | 72 +++++++++++++++++++ apps/sim/app/api/tools/sftp/utils.ts | 31 ++++++++ .../app/api/tools/ssh/download-file/route.ts | 39 +++++----- .../api/tools/ssh/read-file-content/route.ts | 40 ++++------- .../api/tools/ssh/write-file-content/route.ts | 36 +++++----- 6 files changed, 178 insertions(+), 80 deletions(-) create mode 100644 apps/sim/app/api/tools/sftp/utils.test.ts diff --git a/apps/sim/app/api/tools/sftp/download/route.ts b/apps/sim/app/api/tools/sftp/download/route.ts index 5ba713c1065..8b1bd70680f 100644 --- a/apps/sim/app/api/tools/sftp/download/route.ts +++ b/apps/sim/app/api/tools/sftp/download/route.ts @@ -6,9 +6,17 @@ import { sftpDownloadContract } from '@/lib/api/contracts/storage-transfer' import { parseRequest } from '@/lib/api/server' import { checkInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' -import { createSftpConnection, getSftp, isPathSafe, sanitizePath } from '@/app/api/tools/sftp/utils' +import { + createSftpConnection, + getSftp, + isPathSafe, + MAX_SFTP_READ_BYTES, + readSftpFileCapped, + sanitizePath, +} from '@/app/api/tools/sftp/utils' export const dynamic = 'force-dynamic' @@ -73,30 +81,22 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }) }) - const maxSize = 50 * 1024 * 1024 - if (stats.size > maxSize) { + if (stats.size > MAX_SFTP_READ_BYTES) { const sizeMB = (stats.size / (1024 * 1024)).toFixed(2) return NextResponse.json( { success: false, error: `File size (${sizeMB}MB) exceeds download limit of 50MB` }, - { status: 400 } + { status: 413 } ) } logger.info(`[${requestId}] Downloading file ${remotePath} (${stats.size} bytes)`) - const chunks: Buffer[] = [] - await new Promise((resolve, reject) => { - const readStream = sftp.createReadStream(remotePath) - - readStream.on('data', (chunk: Buffer) => { - chunks.push(chunk) - }) - - readStream.on('end', () => resolve()) - readStream.on('error', reject) - }) - - const buffer = Buffer.concat(chunks) + const buffer = await readSftpFileCapped( + sftp, + remotePath, + MAX_SFTP_READ_BYTES, + 'SFTP download' + ) const fileName = path.basename(remotePath) const extension = getFileExtension(fileName) const mimeType = getMimeTypeFromExtension(extension) @@ -129,6 +129,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } catch (error) { const errorMessage = getErrorMessage(error, 'Unknown error occurred') + + if (isPayloadSizeLimitError(error)) { + logger.warn(`[${requestId}] SFTP download aborted: ${errorMessage}`) + return NextResponse.json({ success: false, error: errorMessage }, { status: 413 }) + } + logger.error(`[${requestId}] SFTP download failed:`, error) return NextResponse.json({ error: `SFTP download failed: ${errorMessage}` }, { status: 500 }) diff --git a/apps/sim/app/api/tools/sftp/utils.test.ts b/apps/sim/app/api/tools/sftp/utils.test.ts new file mode 100644 index 00000000000..a9e3121717e --- /dev/null +++ b/apps/sim/app/api/tools/sftp/utils.test.ts @@ -0,0 +1,72 @@ +/** + * @vitest-environment node + */ +import { Readable } from 'stream' +import type { SFTPWrapper } from 'ssh2' +import { describe, expect, it, vi } from 'vitest' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' +import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils' + +/** + * Builds a fake SFTP wrapper whose read stream emits `chunkCount` chunks of + * `chunkSize` bytes — the shape of a malicious server that understates the + * file size in its stat reply and then streams unbounded data. + */ +function fakeSftp(chunkSize: number, chunkCount: number) { + let emitted = 0 + const stream = new Readable({ + read() { + if (emitted >= chunkCount) { + this.push(null) + return + } + emitted++ + this.push(Buffer.alloc(chunkSize, 0x41)) + }, + }) + const createReadStream = vi.fn(() => stream) + return { sftp: { createReadStream } as unknown as SFTPWrapper, stream, createReadStream } +} + +describe('readSftpFileCapped', () => { + it('resolves with the full contents when under the cap', async () => { + const { sftp, createReadStream } = fakeSftp(4, 3) + + const buffer = await readSftpFileCapped(sftp, '/file', 1024, 'file') + + expect(buffer.toString()).toBe('A'.repeat(12)) + expect(createReadStream).toHaveBeenCalledWith('/file') + }) + + it('rejects and destroys the stream once received bytes exceed the cap', async () => { + const { sftp, stream } = fakeSftp(8, 1_000_000) + + await expect(readSftpFileCapped(sftp, '/bomb', 16, 'file')).rejects.toSatisfy( + isPayloadSizeLimitError + ) + expect(stream.destroyed).toBe(true) + }) + + it('enforces the cap on actual bytes even when the file was reported as tiny', async () => { + const { sftp, stream } = fakeSftp(1024, 1_000_000) + + await expect(readSftpFileCapped(sftp, '/bomb', 4096, 'file')).rejects.toThrow( + /exceeds maximum size of 4096 bytes/ + ) + expect(stream.destroyed).toBe(true) + }) + + it('survives the late stream error ssh2 emits when the channel closes after an abort', async () => { + const { sftp, stream } = fakeSftp(8, 1_000_000) + + await expect(readSftpFileCapped(sftp, '/bomb', 16, 'file')).rejects.toSatisfy( + isPayloadSizeLimitError + ) + + expect(() => stream.emit('error', new Error('No response from server'))).not.toThrow() + }) + + it('caps remote reads at 50MB', () => { + expect(MAX_SFTP_READ_BYTES).toBe(50 * 1024 * 1024) + }) +}) diff --git a/apps/sim/app/api/tools/sftp/utils.ts b/apps/sim/app/api/tools/sftp/utils.ts index 639672ae438..ea81b52793c 100644 --- a/apps/sim/app/api/tools/sftp/utils.ts +++ b/apps/sim/app/api/tools/sftp/utils.ts @@ -1,6 +1,7 @@ import { toError } from '@sim/utils/errors' import { type Attributes, Client, type ConnectConfig, type SFTPWrapper } from 'ssh2' import { validateDatabaseHost } from '@/lib/core/security/input-validation.server' +import { readNodeStreamToBufferWithLimit } from '@/lib/core/utils/stream-limits' const S_IFMT = 0o170000 const S_IFDIR = 0o040000 @@ -172,6 +173,36 @@ export function getSftp(client: Client): Promise { }) } +/** Maximum bytes a route will buffer from a remote SFTP file. */ +export const MAX_SFTP_READ_BYTES = 50 * 1024 * 1024 + +/** + * Reads a remote file into memory, enforcing the cap on the bytes actually + * received rather than on the `stat()` size the remote server reports. + * A caller-supplied SSH server can understate the size in its `SSH_FXP_STAT` + * reply and then stream unbounded data, so the stream is destroyed as soon as + * the running total exceeds `maxBytes`. Rejects with a `PayloadSizeLimitError`. + */ +export function readSftpFileCapped( + sftp: SFTPWrapper, + remotePath: string, + maxBytes: number, + label: string +): Promise { + const stream = sftp.createReadStream(remotePath) + + /** + * Closing the SSH client rejects every still-pending SFTP request with + * "No response from server", which lands as a late `error` on a stream the + * limiter has already detached from once it destroyed it. An `error` event + * with no listener is an uncaught exception, so keep one attached for the + * stream's whole life; the limiter's own handler still settles the promise. + */ + stream.on('error', () => {}) + + return readNodeStreamToBufferWithLimit(stream, { maxBytes, label }) +} + /** * Sanitizes a remote path to prevent path traversal attacks. * Removes null bytes, normalizes path separators, and collapses traversal sequences. diff --git a/apps/sim/app/api/tools/ssh/download-file/route.ts b/apps/sim/app/api/tools/ssh/download-file/route.ts index 8bd41d9f253..9c0af06f10e 100644 --- a/apps/sim/app/api/tools/ssh/download-file/route.ts +++ b/apps/sim/app/api/tools/ssh/download-file/route.ts @@ -7,8 +7,10 @@ import type { Client, SFTPWrapper } from 'ssh2' import { sshDownloadFileContract } from '@/lib/api/contracts/storage-transfer' import { parseRequest } from '@/lib/api/server' import { checkInternalAuth } from '@/lib/auth/hybrid' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils' +import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils' import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils' const logger = createLogger('SSHDownloadFileAPI') @@ -67,31 +69,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }) }) - // Check file size limit (50MB to prevent memory exhaustion) - const maxSize = 50 * 1024 * 1024 - if (stats.size > maxSize) { + if (stats.size > MAX_SFTP_READ_BYTES) { const sizeMB = (stats.size / (1024 * 1024)).toFixed(2) return NextResponse.json( { error: `File size (${sizeMB}MB) exceeds download limit of 50MB` }, - { status: 400 } + { status: 413 } ) } - // Read file content - const content = await new Promise((resolve, reject) => { - const chunks: Buffer[] = [] - const readStream = sftp.createReadStream(remotePath) - - readStream.on('data', (chunk: Buffer) => { - chunks.push(chunk) - }) - - readStream.on('end', () => { - resolve(Buffer.concat(chunks)) - }) - - readStream.on('error', reject) - }) + const content = await readSftpFileCapped( + sftp, + remotePath, + MAX_SFTP_READ_BYTES, + 'SSH file download' + ) const fileName = path.basename(remotePath) const extension = getFileExtension(fileName) @@ -108,12 +99,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { name: fileName, mimeType, data: base64Content, - size: stats.size, + size: content.length, }, content: base64Content, fileName: fileName, remotePath: remotePath, - size: stats.size, + size: content.length, message: `File downloaded successfully from ${remotePath}`, }) } finally { @@ -121,6 +112,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } catch (error) { const errorMessage = getErrorMessage(error, 'Unknown error occurred') + + if (isPayloadSizeLimitError(error)) { + logger.warn(`[${requestId}] SSH file download aborted: ${errorMessage}`) + return NextResponse.json({ error: errorMessage }, { status: 413 }) + } + logger.error(`[${requestId}] SSH file download failed:`, error) return NextResponse.json( diff --git a/apps/sim/app/api/tools/ssh/read-file-content/route.ts b/apps/sim/app/api/tools/ssh/read-file-content/route.ts index 2558f1fcf62..7594803fe53 100644 --- a/apps/sim/app/api/tools/ssh/read-file-content/route.ts +++ b/apps/sim/app/api/tools/ssh/read-file-content/route.ts @@ -6,7 +6,9 @@ import type { Client, SFTPWrapper } from 'ssh2' import { sshReadFileContentContract } from '@/lib/api/contracts/storage-transfer' import { parseRequest } from '@/lib/api/server' import { checkInternalAuth } from '@/lib/auth/hybrid' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { readSftpFileCapped } from '@/app/api/tools/sftp/utils' import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils' const logger = createLogger('SSHReadFileContentAPI') @@ -68,51 +70,37 @@ export const POST = withRouteHandler(async (request: NextRequest) => { if (stats.size > maxBytes) { return NextResponse.json( { error: `File size (${stats.size} bytes) exceeds maximum allowed (${maxBytes} bytes)` }, - { status: 400 } + { status: 413 } ) } - const content = await new Promise((resolve, reject) => { - const chunks: Buffer[] = [] - let totalBytes = 0 - const readStream = sftp.createReadStream(filePath) - - readStream.on('data', (chunk: Buffer) => { - totalBytes += chunk.length - if (totalBytes > maxBytes) { - readStream.destroy() - reject(new Error(`File exceeds maximum allowed size of ${params.maxSize}MB`)) - return - } - chunks.push(chunk) - }) - - readStream.on('end', () => { - const buffer = Buffer.concat(chunks) - resolve(buffer.toString(params.encoding as BufferEncoding)) - }) - - readStream.on('error', reject) - }) + const buffer = await readSftpFileCapped(sftp, filePath, maxBytes, `File '${filePath}'`) + const content = buffer.toString(params.encoding as BufferEncoding) const lines = content.split('\n').length logger.info( - `[${requestId}] File content read successfully: ${stats.size} bytes, ${lines} lines` + `[${requestId}] File content read successfully: ${buffer.length} bytes, ${lines} lines` ) return NextResponse.json({ content, - size: stats.size, + size: buffer.length, lines, path: filePath, - message: `File read successfully: ${stats.size} bytes, ${lines} lines`, + message: `File read successfully: ${buffer.length} bytes, ${lines} lines`, }) } finally { client.end() } } catch (error) { const errorMessage = getErrorMessage(error, 'Unknown error occurred') + + if (isPayloadSizeLimitError(error)) { + logger.warn(`[${requestId}] SSH read file content aborted: ${errorMessage}`) + return NextResponse.json({ error: errorMessage }, { status: 413 }) + } + logger.error(`[${requestId}] SSH read file content failed:`, error) return NextResponse.json( diff --git a/apps/sim/app/api/tools/ssh/write-file-content/route.ts b/apps/sim/app/api/tools/ssh/write-file-content/route.ts index 9ce1f92b5e2..ce742d0e62a 100644 --- a/apps/sim/app/api/tools/ssh/write-file-content/route.ts +++ b/apps/sim/app/api/tools/ssh/write-file-content/route.ts @@ -6,7 +6,9 @@ import type { Client, SFTPWrapper } from 'ssh2' import { sshWriteFileContentContract } from '@/lib/api/contracts/storage-transfer' import { parseRequest } from '@/lib/api/server' import { checkInternalAuth } from '@/lib/auth/hybrid' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils' import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils' const logger = createLogger('SSHWriteFileContentAPI') @@ -73,22 +75,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { // Handle append mode by reading existing content first let finalContent = params.content if (params.mode === 'append') { - const existingContent = await new Promise((resolve) => { - const chunks: Buffer[] = [] - const readStream = sftp.createReadStream(filePath) - - readStream.on('data', (chunk: Buffer) => { - chunks.push(chunk) - }) - - readStream.on('end', () => { - resolve(Buffer.concat(chunks).toString('utf-8')) - }) - - readStream.on('error', () => { - resolve('') - }) - }) + let existingContent = '' + try { + const existing = await readSftpFileCapped( + sftp, + filePath, + MAX_SFTP_READ_BYTES, + `Existing file '${filePath}'` + ) + existingContent = existing.toString('utf-8') + } catch (error) { + if (isPayloadSizeLimitError(error)) throw error + } finalContent = existingContent + params.content } @@ -124,6 +122,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } } catch (error) { const errorMessage = getErrorMessage(error, 'Unknown error occurred') + + if (isPayloadSizeLimitError(error)) { + logger.warn(`[${requestId}] SSH write file content aborted: ${errorMessage}`) + return NextResponse.json({ error: errorMessage }, { status: 413 }) + } + logger.error(`[${requestId}] SSH write file content failed:`, error) return NextResponse.json(