diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/marxan-directory.service.spec.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/marxan-directory.service.spec.ts index 07b59e1f37..e9285f4b3d 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/marxan-directory.service.spec.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/marxan-directory.service.spec.ts @@ -24,8 +24,11 @@ beforeEach(async () => { }).compile(); fileReader = sandbox.get(FileReader); sut = sandbox.get(MarxanDirectory); - workspace = new Workspace(cwd as WorkingDirectory, bin, () => - Promise.resolve(), + workspace = new Workspace( + cwd as WorkingDirectory, + bin, + () => Promise.resolve(), + () => Promise.resolve(), ); }); diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/ports/temporary-directory.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/ports/temporary-directory.ts index 9524ac7cb9..a951a51e3b 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/ports/temporary-directory.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/ports/temporary-directory.ts @@ -6,5 +6,7 @@ import { WorkingDirectory } from '../../../ports/working-directory'; export abstract class TemporaryDirectory { abstract get(): Promise; + abstract createOutputDirectory(inside: WorkingDirectory): Promise; + abstract cleanup(directory: string): Promise; } diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.spec.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.spec.ts new file mode 100644 index 0000000000..b3d8615b6d --- /dev/null +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.spec.ts @@ -0,0 +1,41 @@ +import { SharedStorage, SharedStoragePath } from './shared-storage'; +import { Test } from '@nestjs/testing'; +import { MarxanDirectory } from '../marxan-directory.service'; +import { v4 } from 'uuid'; + +let sut: SharedStorage; +const randomSubdirectory = v4(); + +beforeEach(async () => { + const sandbox = await Test.createTestingModule({ + providers: [ + SharedStorage, + { + provide: MarxanDirectory, + useValue: {}, + }, + { + provide: SharedStoragePath, + useValue: `/tmp/${randomSubdirectory}`, + }, + ], + }).compile(); + + sut = sandbox.get(SharedStorage); +}); + +describe(`when trying to reach disallowed directory`, () => { + it(`should throw`, async () => { + await expect(sut.cleanup('../../etc/passwd')).rejects.toMatchInlineSnapshot( + `[Error: Directory traversal is not allowed.]`, + ); + }); +}); + +describe(`when contains null byte`, () => { + it(`should throw`, async () => { + await expect( + sut.cleanup('random-subdir/\0cat /etc/passwd'), + ).rejects.toMatchInlineSnapshot(`[Error: Hacking is not allowed.]`); + }); +}); diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.ts index 9a7a09c6e8..39880bc112 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/shared-storage.ts @@ -1,28 +1,30 @@ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { v4 } from 'uuid'; import { promises } from 'fs'; import { resolve } from 'path'; -import { AppConfig } from '@marxan-geoprocessing/utils/config.utils'; -import { assertDefined } from '@marxan/utils'; - import { TemporaryDirectory } from './ports/temporary-directory'; import { WorkingDirectory } from '../../ports/working-directory'; +import { MarxanDirectory } from '../marxan-directory.service'; + +export const SharedStoragePath = Symbol('shared storage temporary directory'); @Injectable() export class SharedStorage implements TemporaryDirectory { - readonly #tempDirectory: string; - - constructor() { - const storagePath = AppConfig.get( - 'storage.sharedFileStorage.localPath', - ); - assertDefined(storagePath); - this.#tempDirectory = storagePath; - } + constructor( + @Inject(SharedStoragePath) private readonly tempDirectory: string, + private readonly marxanDirectory: MarxanDirectory, + ) {} async cleanup(directory: string): Promise { - // TODO check if starts with tempDirectory + if (this.#hasPoisonNullByte(directory)) { + throw new Error(`Hacking is not allowed.`); + } + + if (this.#isDirectoryTraversal(directory, this.tempDirectory)) { + throw new Error(`Directory traversal is not allowed.`); + } + await promises.rm(directory, { recursive: true, force: true, @@ -32,10 +34,20 @@ export class SharedStorage implements TemporaryDirectory { async get(): Promise { const directory = v4(); - const fullPath = resolve(this.#tempDirectory, directory); - await promises.mkdir(resolve(this.#tempDirectory, directory)); - // TODO replace output with the name from params - await promises.mkdir(resolve(this.#tempDirectory, directory, 'output')); + const fullPath = resolve(this.tempDirectory, directory) as WorkingDirectory; + + await promises.mkdir(fullPath); + return fullPath as WorkingDirectory; } + + #hasPoisonNullByte = (path: string): boolean => path.indexOf('\0') !== -1; + + #isDirectoryTraversal = (fullPath: string, rootDirectory: string) => + !fullPath.startsWith(rootDirectory); + + async createOutputDirectory(inside: WorkingDirectory): Promise { + const outputPath = this.marxanDirectory.get('OUTPUTDIR', inside); + await promises.mkdir(outputPath.fullPath); + } } diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.module.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.module.ts index 196c456eac..2ab2d86736 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.module.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.module.ts @@ -4,10 +4,13 @@ import { WorkspaceBuilder } from '../../ports/workspace-builder'; import { LinkMarxan } from './ports/link-marxan'; import { TemporaryDirectory } from './ports/temporary-directory'; import { SymlinkBinary } from './symlink-binary'; -import { SharedStorage } from './shared-storage'; +import { SharedStorage, SharedStoragePath } from './shared-storage'; import { WorkspaceService } from './workspace.service'; import { MarxanConfig } from '../../marxan-config'; +import { MarxanDirectory } from '../marxan-directory.service'; +import { FileReader } from '../../adapters/file-reader'; +import { AppConfig } from '@marxan-geoprocessing/utils/config.utils'; @Module({ providers: [ @@ -24,7 +27,15 @@ import { MarxanConfig } from '../../marxan-config'; provide: WorkspaceBuilder, useClass: WorkspaceService, }, + { + provide: SharedStoragePath, + useFactory: () => { + return AppConfig.get('storage.sharedFileStorage.localPath'); + }, + }, WorkspaceService, + FileReader, + MarxanDirectory, ], exports: [WorkspaceBuilder], }) diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.service.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.service.ts index 2c9cb12c70..379cf03a69 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.service.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/adapters/workspace/workspace.service.ts @@ -21,8 +21,12 @@ export class WorkspaceService implements WorkspaceBuilder { await this.binaryLinker.link(this.marxanConfig.binPath, marxanBinaryPath); console.log(`doing things in..`, workingDirectory); - return new Workspace(workingDirectory, marxanBinaryPath, async () => - this.workingDirectoryService.cleanup(workingDirectory), + return new Workspace( + workingDirectory, + marxanBinaryPath, + async () => this.workingDirectoryService.cleanup(workingDirectory), + async () => + this.workingDirectoryService.createOutputDirectory(workingDirectory), ); } } diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/marxan-sandbox-runner.service.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/marxan-sandbox-runner.service.ts index bd04dc3dec..aa3744cf1d 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/marxan-sandbox-runner.service.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/marxan-sandbox-runner.service.ts @@ -56,6 +56,7 @@ export class MarxanSandboxRunnerService { await interruptIfKilled(); await inputFiles.include(workspace, assets); + await workspace.arrangeOutputSpace(); return new Promise(async (resolve, reject) => { marxanRun.on('error', async (result) => { diff --git a/api/apps/geoprocessing/src/marxan-sandboxed-runner/ports/workspace.ts b/api/apps/geoprocessing/src/marxan-sandboxed-runner/ports/workspace.ts index a20be69c8d..b88fc4ebbd 100644 --- a/api/apps/geoprocessing/src/marxan-sandboxed-runner/ports/workspace.ts +++ b/api/apps/geoprocessing/src/marxan-sandboxed-runner/ports/workspace.ts @@ -6,9 +6,14 @@ export class Workspace implements Cancellable { public readonly workingDirectory: WorkingDirectory, public readonly marxanBinaryPath: string, public readonly cleanup: () => Promise, + public readonly assembleOutputDirectory: () => Promise, ) {} cancel(): Promise { - return Promise.resolve(undefined); + return this.cleanup(); + } + + arrangeOutputSpace() { + return this.assembleOutputDirectory(); } }