Merge branch 'fix/reject-invalid-paths2'

apps/http-server/packages/generic/src/storage/fileStorage.ts

@@ -4,7 +4,7 @@ import { promisify } from 'util'
 import mime from 'mime-types'
 import prettyBytes from 'pretty-bytes'
 import { asyncPipe, CTXPost } from '../lib'
-import { HTTPServerConfig, LoggerInstance } from '@sofie-package-manager/api'
+import { betterPathResolve, HTTPServerConfig, LoggerInstance } from '@sofie-package-manager/api'
 import { BadResponse, PackageInfo, ResponseMeta, Storage } from './storage'
 import { Readable } from 'stream'
 
@@ -31,7 +31,7 @@ export class FileStorage extends Storage {
 	private logger: LoggerInstance
 	constructor(logger: LoggerInstance, private config: HTTPServerConfig) {
 		super()
-		this._basePath = path.resolve(this.config.httpServer.basePath)
+		this._basePath = betterPathResolve(this.config.httpServer.basePath)
 		this.logger = logger.category('FileStorage')
 
 		// Run this on startup, so that if there are any critical errors we'll see them right away:

shared/packages/api/src/__tests__/lib.spec.ts

@@ -1,4 +1,15 @@
-import { deferGets, diff, promiseTimeout, stringMaxLength, stringifyError, waitTime } from '../lib'
+import * as path from 'path'
+import {
+	betterPathIsAbsolute,
+	betterPathJoin,
+	betterPathResolve,
+	deferGets,
+	diff,
+	promiseTimeout,
+	stringMaxLength,
+	stringifyError,
+	waitTime,
+} from '../lib'
 
 describe('lib', () => {
 	test('diff', () => {
@@ -226,5 +237,38 @@ describe('lib', () => {
 		expect(stringMaxLength('0123456789abcdefg', 9)).toBe('012...efg')
 		expect(stringMaxLength('0123456789abcdefg', 8)).toBe('01...efg')
 	})
+	test('betterPathJoin', () => {
+		const fixSep = (str: string) => str.replace(/\//g, path.sep)
+
+		expect(betterPathJoin('\\\\a\\b/c')).toBe(fixSep('//a/b/c'))
+		expect(betterPathJoin('a/b', 'c')).toBe(fixSep('a/b/c'))
+		expect(betterPathJoin('a/b', '../c')).toBe(fixSep('a/c'))
+		expect(betterPathJoin('a/b', '/c')).toBe(fixSep('a/b/c'))
+		expect(betterPathJoin('C:\\a\\b', 'c\\d')).toBe(fixSep('C:/a/b/c/d'))
+		expect(betterPathJoin('C:\\a\\b', '../c')).toBe(fixSep('C:/a/c'))
+
+		expect(betterPathJoin('\\\\a\\b', '../c')).toBe(fixSep('//a/c')) // This is where path.join fails
+		expect(betterPathJoin('//a/b', '../c')).toBe(fixSep('//a/c')) // This is where path.join fails
+	})
+	test('betterPathResolve', () => {
+		const fixSep = (str: string) => str.replace(/\//g, path.sep)
+
+		expect(betterPathResolve('a/b/c')).toBe(path.resolve(fixSep('a/b/c')))
+		expect(betterPathResolve('a/b/c')).toBe(path.resolve(fixSep('a/b/c')))
+
+		expect(betterPathResolve('\\\\a\\b')).toBe(fixSep('//a/b'))
+		expect(betterPathResolve('\\\\a\\b\\c')).toBe(fixSep('//a/b/c'))
+	})
+	test('betterPathIsAbsolute', () => {
+		expect(betterPathIsAbsolute('a/b/c')).toBe(false)
+		expect(betterPathIsAbsolute('./a/b/c')).toBe(false)
+		expect(betterPathIsAbsolute('../a/b/c')).toBe(false)
+
+		expect(betterPathIsAbsolute('/a')).toBe(true)
+		expect(betterPathIsAbsolute('C:\\a\\b\\c')).toBe(true)
+		expect(betterPathIsAbsolute('\\\\a\\b\\c')).toBe(true)
+		expect(betterPathIsAbsolute('\\a')).toBe(true)
+		expect(betterPathIsAbsolute('//a')).toBe(true)
+	})
 })
 export {}

shared/packages/api/src/lib.ts

@@ -1,4 +1,5 @@
 import crypto from 'crypto'
+import path from 'path'
 import { compact } from 'underscore'
 import { AnyProtectedString } from './ProtectedString'
 
@@ -382,6 +383,54 @@ export function mapToObject<T>(map: Map<any, T>): { [key: string]: T } {
 	})
 	return o
 }
+/**
+ * Like path.join(),
+ * but this fixes an issue in path.join where it doesn't handle paths double slashes together with relative paths correctly
+ * And it also returns predictable results on both Windows and Linux
+ */
+export function betterPathJoin(...paths: string[]): string {
+	// Replace slashes with the correct path separator, because "C:\\a\\b" is not interpreted correctly on Linux (but C:/a/b is)
+	paths = paths.map((p) => p.replace(/[\\/]/g, path.sep))
+
+	let firstPath = paths[0]
+	const restPaths = paths.slice(1)
+
+	let prefix = ''
+	if (firstPath.startsWith('//') || firstPath.startsWith('\\\\')) {
+		// Preserve the prefix, as path.join will remove it:
+		prefix = path.sep + path.sep
+		firstPath = firstPath.slice(2)
+	}
+
+	return prefix + path.join(firstPath, ...restPaths)
+}
+/**
+ * Like path.resolve(),
+ * but it returns predictable results on both Windows and Linux
+ */
+export function betterPathResolve(p: string): string {
+	p = p.replace(/[\\/]/g, path.sep)
+
+	// let prefix = ''
+	if (p.startsWith('//') || p.startsWith('\\\\')) {
+		return path.sep + path.sep + path.normalize(p.slice(2))
+	} else {
+		return path.resolve(p)
+	}
+}
+/**
+ * Like path.isAbsolute(),
+ * but it returns same results on both Windows and Linux
+ */
+export function betterPathIsAbsolute(p: string): boolean {
+	return (
+		path.isAbsolute(p) ||
+		Boolean(p.match(/^\w:/)) || // C:\, path.isAbsolute() doesn't handle this on Linux
+		p.startsWith('\\\\') || // \\server\path, path.isAbsolute() doesn't handle this on Linux
+		p.startsWith('\\') // \server\path, path.isAbsolute() doesn't handle this on Linux
+	)
+}
+
 /** Returns true if we're running tests (in Jest) */
 export function isRunningInTest(): boolean {
 	// Note: JEST_WORKER_ID is set when running in unit tests

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationState.ts

@@ -24,7 +24,6 @@ export async function evaluateExpectationState(
 	const tracker: ExpectationTracker = runner.tracker
 
 	const timeSinceLastEvaluation = Date.now() - trackedExp.lastEvaluationTime
-	if (!trackedExp.session) trackedExp.session = {}
 	if (trackedExp.session.hadError) return // There was an error during the session.
 
 	if (trackedExp.session.expectationCanBeRemoved) return // The expectation has been removed

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/aborted.ts

@@ -14,7 +14,6 @@ export async function evaluateExpectationStateAborted({
 }: EvaluateContext): Promise<void> {
 	assertState(trackedExp, ExpectedPackageStatusAPI.WorkStatusState.ABORTED)
 
-	if (!trackedExp.session) trackedExp.session = {}
 	await manager.workerAgents.assignWorkerToSession(trackedExp)
 	if (trackedExp.session.assignedWorker) {
 		// Start by removing the expectation

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/fulfilled.ts

@@ -20,7 +20,6 @@ export async function evaluateExpectationStateFulfilled({
 	// TODO: Some monitor that is able to invalidate if it isn't fulfilled anymore?
 
 	if (timeSinceLastEvaluation > tracker.getFulfilledWaitTime()) {
-		if (!trackedExp.session) trackedExp.session = {}
 		await manager.workerAgents.assignWorkerToSession(trackedExp)
 		if (trackedExp.session.assignedWorker) {
 			try {

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ready.ts

@@ -17,7 +17,6 @@ export async function evaluateExpectationStateReady({
 }: EvaluateContext): Promise<void> {
 	assertState(trackedExp, ExpectedPackageStatusAPI.WorkStatusState.READY)
 	// Start working on it:
-	if (!trackedExp.session) trackedExp.session = {}
 	await manager.workerAgents.assignWorkerToSession(trackedExp)
 
 	if (

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/removed.ts

@@ -17,7 +17,6 @@ export async function evaluateExpectationStateRemoved({
 	/** When true, the expectation can be removed */
 	let removeTheExpectation = false
 
-	if (!trackedExp.session) trackedExp.session = {}
 	await manager.workerAgents.assignWorkerToSession(trackedExp)
 	if (trackedExp.session.assignedWorker) {
 		const removed = await trackedExp.session.assignedWorker.worker.removeExpectation(trackedExp.exp)

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

@@ -13,7 +13,6 @@ export async function evaluateExpectationStateRestarted({
 }: EvaluateContext): Promise<void> {
 	assertState(trackedExp, ExpectedPackageStatusAPI.WorkStatusState.RESTARTED)
 
-	if (!trackedExp.session) trackedExp.session = {}
 	await manager.workerAgents.assignWorkerToSession(trackedExp)
 	if (trackedExp.session.assignedWorker) {
 		// Start by removing the expectation

shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/waiting.ts

@@ -19,7 +19,6 @@ export async function evaluateExpectationStateWaiting({
 	assertState(trackedExp, ExpectedPackageStatusAPI.WorkStatusState.WAITING)
 
 	// Check if the expectation is ready to start:
-	if (!trackedExp.session) trackedExp.session = {}
 	await manager.workerAgents.assignWorkerToSession(trackedExp)
 
 	if (trackedExp.session.assignedWorker) {

shared/packages/expectationManager/src/evaluationRunner/evaluationRunner.ts

@@ -278,7 +278,7 @@ export class EvaluationRunner {
 
 		// Step 0: Reset the session:
 		for (const trackedExp of tracked) {
-			trackedExp.session = null
+			trackedExp.session = {}
 		}
 
 		const postProcessSession = (trackedExp: TrackedExpectation) => {
@@ -343,7 +343,7 @@ export class EvaluationRunner {
 		// Step 1.5: Reset the session:
 		// Because during the next iteration, the worker-assignment need to be done in series
 		for (const trackedExp of tracked) {
-			trackedExp.session = null
+			trackedExp.session = {}
 		}
 
 		this.logger.debug(`Handle other states..`)

shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts

@@ -170,7 +170,6 @@ export class TrackedExpectationAPI {
 	 * Handle an Expectation that had no worker assigned
 	 */
 	public noWorkerAssigned(trackedExp: TrackedExpectation): void {
-		if (!trackedExp.session) throw new Error('Internal error: noWorkerAssigned: session not set')
 		if (trackedExp.session.assignedWorker)
 			throw new Error('Internal error: noWorkerAssigned can only be called when assignedWorker is falsy')
 

shared/packages/expectationManager/src/lib/trackedExpectation.ts

@@ -54,8 +54,8 @@ export interface TrackedExpectation {
 		targetCanBeUsedWhileTransferring?: boolean
 		sourceIsPlaceholder?: boolean
 	}
-	/** A storage which is persistant only for a short while, during an evaluation of the Expectation. */
-	session: ExpectationStateHandlerSession | null
+	/** A storage which is persistent only for a short while, during an evaluation of the Expectation. */
+	session: ExpectationStateHandlerSession
 }
 
 export function expLabel(exp: TrackedExpectation): string {
@@ -120,6 +120,6 @@ export function getDefaultTrackedExpectation(
 		},
 		prevStatusReasons: existingtrackedExp?.prevStatusReasons || {},
 		status: {},
-		session: null,
+		session: {},
 	}
 }

shared/packages/worker/src/worker/accessorHandlers/__tests__/fileShare.spec.ts

@@ -0,0 +1,82 @@
+import {
+	AccessorOnPackage,
+	protectString,
+	setupLogger,
+	initializeLogger,
+	ProcessConfig,
+	Expectation,
+	Accessor,
+} from '@sofie-package-manager/api'
+import { Content, FileShareAccessorHandle } from '../fileShare'
+import { PassiveTestWorker } from './lib'
+
+const processConfig: ProcessConfig = {
+	logPath: undefined,
+	logLevel: undefined,
+	unsafeSSL: false,
+	certificates: [],
+}
+initializeLogger({ process: processConfig })
+test('checkHandleBasic', () => {
+	const logger = setupLogger(
+		{
+			process: processConfig,
+		},
+		''
+	)
+	const worker = new PassiveTestWorker(logger)
+
+	function getFileShareAccessor(
+		accessor: AccessorOnPackage.FileShare,
+		content: Content,
+		workOptions: Expectation.WorkOptions.Base &
+			Expectation.WorkOptions.RemoveDelay &
+			Expectation.WorkOptions.UseTemporaryFilePath = {}
+	) {
+		accessor.type = Accessor.AccessType.FILE_SHARE
+		return new FileShareAccessorHandle(worker, protectString('share0'), accessor, content, workOptions)
+	}
+
+	expect(() => getFileShareAccessor({}, {}).checkHandleBasic()).toThrowError('Bad input data')
+
+	// missing accessor.folderPath:
+	expect(getFileShareAccessor({}, { filePath: 'amb.amp4' }).checkHandleBasic()).toMatchObject({
+		success: false,
+		reason: { tech: 'Folder path not set' },
+	})
+
+	// All OK:
+	expect(
+		getFileShareAccessor({ folderPath: '\\\\nas01\\media' }, { filePath: 'amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: true,
+	})
+
+	// Absolute file path:
+	expect(
+		getFileShareAccessor({ folderPath: '\\\\nas01\\media' }, { filePath: '//secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+	expect(
+		getFileShareAccessor(
+			{ folderPath: '\\\\nas01\\media' },
+			{ filePath: 'C:\\secret\\amb.amp4' }
+		).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: { tech: expect.stringMatching(/File path.*absolute path/) },
+	})
+
+	// File path outside of folder path:
+	expect(
+		getFileShareAccessor({ folderPath: '//nas01/media' }, { filePath: '../secret/amb.amp4' }).checkHandleBasic()
+	).toMatchObject({
+		success: false,
+		reason: {
+			user: `File path is outside of folder path`,
+			tech: expect.stringMatching(/Full path.*does not start with/),
+		},
+	})
+})

shared/packages/worker/src/worker/accessorHandlers/__tests__/lib.ts

@@ -0,0 +1,65 @@
+import { protectString, LoggerInstance } from '@sofie-package-manager/api'
+import { BaseWorker } from '../../worker'
+
+export class PassiveTestWorker extends BaseWorker {
+	constructor(logger: LoggerInstance) {
+		super(
+			logger,
+			{
+				config: {
+					workerId: protectString('test'),
+					sourcePackageStabilityThreshold: 0,
+					windowsDriveLetters: ['X', 'Y', 'Z'],
+				},
+				location: {
+					// localComputerId?: string
+					localNetworkIds: [],
+				},
+				workerStorageWrite: () => {
+					throw new Error('Method not implemented.')
+				},
+				workerStorageRead: () => {
+					throw new Error('Method not implemented.')
+				},
+			},
+			async () => {
+				throw new Error('Method not implemented.')
+			},
+			'passive-test-worker'
+		)
+	}
+
+	async init() {
+		throw new Error('Method not implemented.')
+	}
+	terminate() {
+		throw new Error('Method not implemented.')
+	}
+	async doYouSupportExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async getCostFortExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async isExpectationReadyToStartWorkingOn(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async isExpectationFulfilled(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async workOnExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async removeExpectation(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async doYouSupportPackageContainer(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async runPackageContainerCronJob(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+	async setupPackageContainerMonitors(): Promise<any> {
+		throw new Error('Method not implemented.')
+	}
+}