Merge branch 'fix-memory-leaks' into bbc-main-autonext

Author: PeterC89

meteor/server/api/blueprints/__tests__/api.test.ts

@@ -1,17 +1,21 @@
 import _ from 'underscore'
+import path from 'path'
+import os from 'os'
+import { promises as fsp } from 'fs'
 import { setupDefaultStudioEnvironment, packageBlueprint } from '../../../../__mocks__/helpers/database'
 import { literal, getRandomId } from '@sofie-automation/corelib/dist/lib'
 import { protectString } from '@sofie-automation/corelib/dist/protectedString'
 import { Blueprint } from '@sofie-automation/corelib/dist/dataModel/Blueprint'
 import { BlueprintManifestType } from '@sofie-automation/blueprints-integration'
 import { SYSTEM_ID, ICoreSystem } from '@sofie-automation/meteor-lib/dist/collections/CoreSystem'
-import { insertBlueprint, uploadBlueprint } from '../api'
+import { insertBlueprint, uploadBlueprint, uploadBlueprintAsset } from '../api'
 import { MeteorCall } from '../../methods'
 import '../../../../__mocks__/_extendJest'
 import { Blueprints, CoreSystem } from '../../../collections'
 import { SupressLogMessages } from '../../../../__mocks__/suppressLogging'
 import { JSONBlobStringify } from '@sofie-automation/shared-lib/dist/lib/JSONBlob'
 import { Meteor } from 'meteor/meteor'
+import * as CoreSystemAPI from '../../../coreSystem'
 
 // we don't want the deviceTriggers observer to start up at this time
 jest.mock('../../deviceTriggers/observer')
@@ -549,4 +553,34 @@ describe('Test blueprint management api', () => {
 			)
 		})
 	})
+	describe('uploadBlueprintAsset', () => {
+		let storePath: string
+
+		beforeEach(async () => {
+			storePath = await fsp.mkdtemp(path.join(os.tmpdir(), 'sofie-blueprint-assets-'))
+			jest.spyOn(CoreSystemAPI, 'getSystemStorePath').mockReturnValue(storePath)
+		})
+
+		afterEach(async () => {
+			jest.restoreAllMocks()
+			await fsp.rm(storePath, { recursive: true, force: true })
+		})
+
+		test('writes decoded base64 payload to file', async () => {
+			const payload = Buffer.from('some fake binary data \u0000\u0001', 'utf8')
+			const fileId = 'myBlueprint/logo.bin'
+
+			await uploadBlueprintAsset(DEFAULT_CONNECTION, fileId, payload.toString('base64'))
+
+			const expectedFilePath = path.join(storePath, 'assets', fileId)
+			const writtenBuffer = await fsp.readFile(expectedFilePath)
+			expect(writtenBuffer.equals(payload)).toBeTruthy()
+		})
+
+		test('rejects path traversal attempts', async () => {
+			await expect(
+				uploadBlueprintAsset(DEFAULT_CONNECTION, '../outside.bin', Buffer.from('x').toString('base64'))
+			).rejects.toThrow('Asset name outside of asset storage path')
+		})
+	})
 })

meteor/server/api/blueprints/__tests__/http.test.ts

@@ -1,5 +1,6 @@
 import _ from 'underscore'
 import { Meteor } from 'meteor/meteor'
+import { PassThrough } from 'stream'
 import { SupressLogMessages } from '../../../../__mocks__/suppressLogging'
 import { callKoaRoute } from '../../../../__mocks__/koa-util'
 import { blueprintsRouter } from '../http'
@@ -357,4 +358,247 @@ describe('Test blueprint http api', () => {
 			}
 		})
 	})
+
+	describe('router upload assets', () => {
+		describe('POST /assets', () => {
+			async function callRoute(body: any) {
+				const ctx = await callKoaRoute(blueprintsRouter, {
+					method: 'POST',
+					url: '/assets',
+
+					requestBody: body,
+				})
+
+				expect(ctx.response.type).toBe('text/plain')
+				return ctx
+			}
+
+			function resetUploadAssetMock() {
+				const uploadBlueprintAsset = api.uploadBlueprintAsset as any as jest.MockInstance<any, any>
+				uploadBlueprintAsset.mockClear()
+				return uploadBlueprintAsset
+			}
+
+			beforeEach(() => {
+				resetUploadAssetMock()
+			})
+
+			test('missing body', async () => {
+				SupressLogMessages.suppressLogMessage(/Invalid request body/i)
+				const res = await callRoute(undefined)
+				expect(res.response.status).toEqual(500)
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(0)
+			})
+
+			test('empty body', async () => {
+				SupressLogMessages.suppressLogMessage(/Missing request body/i)
+				const res = await callRoute('')
+				expect(res.response.status).toEqual(500)
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(0)
+			})
+
+			test('non-object body', async () => {
+				SupressLogMessages.suppressLogMessage(/Invalid request body/i)
+				const res = await callRoute(99)
+				expect(res.response.status).toEqual(500)
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(0)
+			})
+
+			test('empty object body', async () => {
+				SupressLogMessages.suppressLogMessage(/Invalid request body/i)
+				const res = await callRoute({})
+				expect(res.response.status).toEqual(500)
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(0)
+			})
+
+			test('with json body', async () => {
+				const fileId = 'folder/asset.png'
+				const payload = {
+					[fileId]: 'Ym9keQ==',
+				}
+
+				const res = await callRoute(payload)
+				expect(res.response.status).toEqual(200)
+				expect(res.body).toEqual('')
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(1)
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, fileId, payload[fileId])
+			})
+
+			test('with json body - multiple', async () => {
+				const count = 10
+				const payload: Record<string, string> = {}
+				for (let i = 0; i < count; i++) {
+					payload[`id${i}.png`] = `body${i}`
+				}
+
+				const res = await callRoute(payload)
+				expect(res.response.status).toEqual(200)
+				expect(res.body).toEqual('')
+
+				expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(count)
+				for (let i = 0; i < count; i++) {
+					expect(api.uploadBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, `id${i}.png`, `body${i}`)
+				}
+			})
+
+			test('with errors', async () => {
+				const count = 10
+				const payload: Record<string, string> = {}
+				for (let i = 0; i < count; i++) {
+					payload[`id${i}.png`] = `body${i}`
+				}
+
+				const uploadBlueprintAsset = resetUploadAssetMock()
+				let called = 0
+				uploadBlueprintAsset.mockImplementation(() => {
+					called++
+					if (called === 3 || called === 7) {
+						throw new Meteor.Error(505, 'Some thrown error')
+					}
+				})
+
+				try {
+					SupressLogMessages.suppressLogMessage(/Some thrown error/i)
+					SupressLogMessages.suppressLogMessage(/Some thrown error/i)
+					const res = await callRoute(payload)
+					expect(res.response.status).toEqual(500)
+					expect(res.body).toEqual(
+						'Errors were encountered: \n[505] Some thrown error\n[505] Some thrown error\n'
+					)
+
+					expect(api.uploadBlueprintAsset).toHaveBeenCalledTimes(count)
+					for (let i = 0; i < count; i++) {
+						expect(api.uploadBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, `id${i}.png`, `body${i}`)
+					}
+				} finally {
+					uploadBlueprintAsset.mockRestore()
+				}
+			})
+		})
+
+		describe('GET /assets/*fileId', () => {
+			function createDataStream() {
+				const stream = new PassThrough()
+				stream.end('asset')
+				return stream
+			}
+
+			async function callRoute(fileId: string) {
+				const ctx = await callKoaRoute(blueprintsRouter, {
+					method: 'GET',
+					url: `/assets/${fileId}`,
+				})
+
+				return ctx
+			}
+
+			function resetRetrieveAssetMock() {
+				const retrieveBlueprintAsset = api.retrieveBlueprintAsset as any as jest.MockInstance<any, any>
+				retrieveBlueprintAsset.mockClear()
+				return retrieveBlueprintAsset
+			}
+
+			beforeEach(() => {
+				resetRetrieveAssetMock()
+			})
+
+			test('png asset', async () => {
+				const fileId = 'folder/file.png'
+				const dataStream = createDataStream()
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockReturnValue(dataStream)
+
+				const res = await callRoute(fileId)
+
+				expect(res.statusCode).toEqual(200)
+				expect(res.response.type).toEqual('image/png')
+				expect(res.body).toBe(dataStream)
+				expect(res.response.get('Cache-Control')).toEqual('public, max-age=1296000, immutable')
+
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledTimes(1)
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, fileId)
+			})
+
+			test('svg asset', async () => {
+				const fileId = 'folder/file.svg'
+				const dataStream = createDataStream()
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockReturnValue(dataStream)
+
+				const res = await callRoute(fileId)
+
+				expect(res.statusCode).toEqual(200)
+				expect(res.response.type).toEqual('image/svg+xml')
+				expect(res.body).toBe(dataStream)
+
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledTimes(1)
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, fileId)
+			})
+
+			test('gif asset', async () => {
+				const fileId = 'folder/file.gif'
+				const dataStream = createDataStream()
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockReturnValue(dataStream)
+
+				const res = await callRoute(fileId)
+
+				expect(res.statusCode).toEqual(200)
+				expect(res.response.type).toEqual('image/gif')
+				expect(res.body).toBe(dataStream)
+
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledTimes(1)
+				expect(api.retrieveBlueprintAsset).toHaveBeenCalledWith(DEFAULT_CONTEXT, fileId)
+			})
+
+			test('not found', async () => {
+				const fileId = 'folder/missing.png'
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockImplementation(() => {
+					const err = new Error('No such file') as Error & { code?: string }
+					err.code = 'ENOENT'
+					throw err
+				})
+
+				SupressLogMessages.suppressLogMessage(/Blueprint asset not found/i)
+				const res = await callRoute(fileId)
+				expect(res.statusCode).toEqual(404)
+			})
+
+			test('path traversal attempt', async () => {
+				const fileId = 'folder/../escape.png'
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockImplementation(() => {
+					throw new Error('Requested asset outside of asset storage path')
+				})
+
+				SupressLogMessages.suppressLogMessage(/Blueprint asset path traversal attempt/i)
+				const res = await callRoute(fileId)
+				expect(res.statusCode).toEqual(400)
+			})
+
+			test('internal error', async () => {
+				const fileId = 'folder/file.png'
+
+				const retrieveBlueprintAsset = resetRetrieveAssetMock()
+				retrieveBlueprintAsset.mockImplementation(() => {
+					throw new Error('Some thrown error')
+				})
+
+				SupressLogMessages.suppressLogMessage(/Blueprint asset retrieval failed/i)
+				const res = await callRoute(fileId)
+				expect(res.statusCode).toEqual(500)
+			})
+		})
+	})
 })

meteor/server/api/blueprints/api.ts

@@ -112,23 +112,36 @@ export async function uploadBlueprintAsset(cred: RequestCredentials, fileId: str
 	assertConnectionHasOneOfPermissions(cred, ...PERMISSIONS_FOR_MANAGE_BLUEPRINTS)
 
 	const storePath = getSystemStorePath()
+	const assetsDir = path.resolve(storePath, 'assets') + path.sep
+	const assetPath = path.resolve(path.join(assetsDir, fileId))
+	if (!assetPath.startsWith(assetsDir)) {
+		throw new Error('Asset name outside of asset storage path')
+	}
 
 	// TODO: add access control here
 	const data = Buffer.from(body, 'base64')
-	const parsedPath = path.parse(fileId)
-	logger.info(
-		`Write ${data.length} bytes to ${path.join(storePath, fileId)} (storePath: ${storePath}, fileId: ${fileId})`
-	)
+	logger.info(`Write ${data.length} bytes to ${assetPath} (storePath: ${storePath}, fileId: ${fileId})`)
+
+	const assetDirPath = path.dirname(assetPath)
 
-	await fsp.mkdir(path.join(storePath, parsedPath.dir), { recursive: true })
-	await fsp.writeFile(path.join(storePath, fileId), data)
+	await fsp.mkdir(assetDirPath, { recursive: true })
+	await fsp.writeFile(assetPath, data)
 }
-export function retrieveBlueprintAsset(_cred: RequestCredentials, fileId: string): ReadStream {
+export async function retrieveBlueprintAsset(_cred: RequestCredentials, fileId: string): Promise<ReadStream> {
 	check(fileId, String)
 
 	const storePath = getSystemStorePath()
+	const assetsDir = path.resolve(storePath, 'assets') + path.sep
+	const assetPath = path.resolve(path.join(assetsDir, fileId))
+	if (!assetPath.startsWith(assetsDir)) {
+		throw new Error('Requested asset outside of asset storage path')
+	}
 
-	return createReadStream(path.join(storePath, fileId))
+	const stream = createReadStream(assetPath)
+	return new Promise((resolve, reject) => {
+		stream.on('open', () => resolve(stream))
+		stream.on('error', (err) => reject(err))
+	})
 }
 /** Only to be called from internal functions */
 export async function internalUploadBlueprint(

meteor/server/api/blueprints/http.ts

@@ -179,15 +179,15 @@ blueprintsRouter.post(
 	}
 )
 
-blueprintsRouter.get('/assets/*splat', async (ctx) => {
+blueprintsRouter.get('/assets/*fileId', async (ctx) => {
 	logger.debug(`Blueprint Asset: ${ctx.socket.remoteAddress} GET "${ctx.url}"`)
 	// TODO - some sort of user verification
 	// for now just check it's a png to prevent snapshots being downloaded
 
-	const filePath = ctx.params[0]
+	const filePath = ctx.params.fileId
 	if (filePath.match(/\.(png|svg|gif)?$/)) {
 		try {
-			const dataStream = retrieveBlueprintAsset(ctx, filePath)
+			const dataStream = await retrieveBlueprintAsset(ctx, filePath)
 			const extension = path.extname(filePath)
 			if (extension === '.svg') {
 				ctx.response.type = 'image/svg+xml'
@@ -200,8 +200,17 @@ blueprintsRouter.get('/assets/*splat', async (ctx) => {
 			ctx.set('Cache-Control', `public, max-age=${BLUEPRINT_ASSET_MAX_AGE}, immutable`)
 			ctx.statusCode = 200
 			ctx.body = dataStream
-		} catch {
-			ctx.statusCode = 404 // Probably
+		} catch (e) {
+			if (e instanceof Error && 'code' in e && e.code === 'ENOENT') {
+				logger.warn('Blueprint asset not found: ' + e)
+				ctx.statusCode = 404
+			} else if (e instanceof Error && e.message.includes('outside of asset storage path')) {
+				logger.warn('Blueprint asset path traversal attempt: ' + e)
+				ctx.statusCode = 400
+			} else {
+				logger.warn('Blueprint asset retrieval failed: ' + e)
+				ctx.statusCode = 500
+			}
 		}
 	} else {
 		ctx.statusCode = 403