From 22c11a6e23c4b1e11cc9ca3f9959cae620a3cac8 Mon Sep 17 00:00:00 2001 From: Jan Starzak Date: Thu, 2 Apr 2026 16:54:14 +0200 Subject: [PATCH 1/4] fix: remove some unusual padding from PreviewPopUp --large variant --- packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scss b/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scss index 31ca95133d8..99c409ad219 100644 --- a/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scss +++ b/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scss @@ -35,7 +35,6 @@ &--large { width: 482px; - padding-bottom: 10px; --preview-max-dimension: 480; } @@ -433,4 +432,8 @@ font-weight: 300; } } + + > * + :last-child:not(.preview-popUp__box-layout, .preview-popUp__iframe, .preview-popUp__video) { + margin-bottom: 10px; + } } From 9508fd6a3e216d8c01f58110f660ad2954718b76 Mon Sep 17 00:00:00 2001 From: Jan Starzak Date: Tue, 31 Mar 2026 16:33:14 +0200 Subject: [PATCH 2/4] fix: improve Blueprint asset handling --- meteor/server/api/blueprints/api.ts | 23 +++++++++++++------ meteor/server/api/blueprints/http.ts | 17 ++++++++++---- .../lib/Components/BlueprintAssetIcon.tsx | 2 +- .../ui/PreviewPopUp/PreviewPopUpContext.tsx | 6 ++--- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/meteor/server/api/blueprints/api.ts b/meteor/server/api/blueprints/api.ts index 4a152ef66e9..80d0dcf54d8 100644 --- a/meteor/server/api/blueprints/api.ts +++ b/meteor/server/api/blueprints/api.ts @@ -112,23 +112,32 @@ 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})`) - await fsp.mkdir(path.join(storePath, parsedPath.dir), { recursive: true }) - await fsp.writeFile(path.join(storePath, fileId), data) + const assetDirPath = path.dirname(assetPath) + + await fsp.mkdir(assetDirPath, { recursive: true }) + await fsp.writeFile(assetPath, data) } export function retrieveBlueprintAsset(_cred: RequestCredentials, fileId: string): 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)) + return createReadStream(assetPath) } /** Only to be called from internal functions */ export async function internalUploadBlueprint( diff --git a/meteor/server/api/blueprints/http.ts b/meteor/server/api/blueprints/http.ts index 29b1a6bc719..b4a524548d6 100644 --- a/meteor/server/api/blueprints/http.ts +++ b/meteor/server/api/blueprints/http.ts @@ -179,12 +179,12 @@ 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) @@ -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 diff --git a/packages/webui/src/client/lib/Components/BlueprintAssetIcon.tsx b/packages/webui/src/client/lib/Components/BlueprintAssetIcon.tsx index 6d18d80b098..909420a11d9 100644 --- a/packages/webui/src/client/lib/Components/BlueprintAssetIcon.tsx +++ b/packages/webui/src/client/lib/Components/BlueprintAssetIcon.tsx @@ -6,7 +6,7 @@ const GLOBAL_BLUEPRINT_ASSET_CACHE: Record = {} export function BlueprintAssetIcon({ src, className }: { src: string; className?: string }): JSX.Element | null { const url = useMemo(() => { if (src.startsWith('data:')) return new URL(src) - return new URL(createPrivateApiPath('/blueprints/assets/' + src), location.href) + return new URL(createPrivateApiPath('blueprints/assets/' + src), location.href) }, [src]) const [svgAsset, setSvgAsset] = useState(GLOBAL_BLUEPRINT_ASSET_CACHE[url.href] ?? null) diff --git a/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx b/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx index 0e43ca735e0..ae407efc246 100644 --- a/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx +++ b/packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx @@ -50,7 +50,7 @@ export function convertSourceLayerItemToPreview( case PreviewType.BlueprintImage: contents.push({ type: 'image', - src: createPrivateApiPath('/blueprints/assets/' + popupPreview.preview.image), + src: createPrivateApiPath('blueprints/assets/' + popupPreview.preview.image), }) break case PreviewType.HTML: @@ -82,7 +82,7 @@ export function convertSourceLayerItemToPreview( contents.push({ type: 'boxLayout', boxSourceConfiguration: popupPreview.preview.boxes, - backgroundArtSrc: createPrivateApiPath('/blueprints/assets/' + popupPreview.preview.background), + backgroundArtSrc: createPrivateApiPath('blueprints/assets/' + popupPreview.preview.background), }) break case PreviewType.Table: @@ -288,7 +288,7 @@ export function convertSourceLayerItemToPreview( const content = item.content as TransitionContent if (content.preview) return { - contents: [{ type: 'image', src: createPrivateApiPath('/blueprints/assets/' + content.preview) }], + contents: [{ type: 'image', src: createPrivateApiPath('blueprints/assets/' + content.preview) }], options: {}, } } From e9300e5af4fdb54cd2c261630256a29f5f4cc3e4 Mon Sep 17 00:00:00 2001 From: Jan Starzak Date: Tue, 14 Apr 2026 13:21:06 +0200 Subject: [PATCH 3/4] test: add tests for http blueprint asset upload and download --- .../api/blueprints/__tests__/api.test.ts | 36 ++- .../api/blueprints/__tests__/http.test.ts | 244 ++++++++++++++++++ 2 files changed, 279 insertions(+), 1 deletion(-) diff --git a/meteor/server/api/blueprints/__tests__/api.test.ts b/meteor/server/api/blueprints/__tests__/api.test.ts index 1bd945c2fd2..2c92ed6f4f8 100644 --- a/meteor/server/api/blueprints/__tests__/api.test.ts +++ b/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') + }) + }) }) diff --git a/meteor/server/api/blueprints/__tests__/http.test.ts b/meteor/server/api/blueprints/__tests__/http.test.ts index 6673f4eb6a9..6755071de6a 100644 --- a/meteor/server/api/blueprints/__tests__/http.test.ts +++ b/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 + 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 = {} + 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 = {} + 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 + 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) + }) + }) + }) }) From 6760f8b0210832ae93621250d3221cf67a7d9b55 Mon Sep 17 00:00:00 2001 From: Jan Starzak Date: Thu, 30 Apr 2026 11:51:59 +0200 Subject: [PATCH 4/4] fix: update retrieveBlueprintAsset to return a Promise and await its result in the assets route --- meteor/server/api/blueprints/api.ts | 8 ++++++-- meteor/server/api/blueprints/http.ts | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/meteor/server/api/blueprints/api.ts b/meteor/server/api/blueprints/api.ts index 80d0dcf54d8..a02c027868b 100644 --- a/meteor/server/api/blueprints/api.ts +++ b/meteor/server/api/blueprints/api.ts @@ -127,7 +127,7 @@ export async function uploadBlueprintAsset(cred: RequestCredentials, fileId: str 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 { check(fileId, String) const storePath = getSystemStorePath() @@ -137,7 +137,11 @@ export function retrieveBlueprintAsset(_cred: RequestCredentials, fileId: string throw new Error('Requested asset outside of asset storage path') } - return createReadStream(assetPath) + 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( diff --git a/meteor/server/api/blueprints/http.ts b/meteor/server/api/blueprints/http.ts index b4a524548d6..bf9666dd303 100644 --- a/meteor/server/api/blueprints/http.ts +++ b/meteor/server/api/blueprints/http.ts @@ -187,7 +187,7 @@ blueprintsRouter.get('/assets/*fileId', async (ctx) => { 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'