diff --git a/app/js/CompileController.js b/app/js/CompileController.js index 857d050..9c18830 100644 --- a/app/js/CompileController.js +++ b/app/js/CompileController.js @@ -220,8 +220,10 @@ module.exports = CompileController = { const { image } = req.query if ( image && - Settings.allowedImageNamesFlat && - Settings.allowedImageNamesFlat.indexOf(image) === -1 + Settings.clsi && + Settings.clsi.docker && + Settings.clsi.docker.allowedImages && + !Settings.clsi.docker.allowedImages.includes(image) ) { return res.status(400).send('invalid image') } diff --git a/app/js/DockerRunner.js b/app/js/DockerRunner.js index 5874a51..5f04fe0 100644 --- a/app/js/DockerRunner.js +++ b/app/js/DockerRunner.js @@ -91,6 +91,13 @@ module.exports = DockerRunner = { image = `${Settings.texliveImageNameOveride}/${img[2]}` } + if ( + Settings.clsi.docker.allowedImages && + !Settings.clsi.docker.allowedImages.includes(image) + ) { + return callback(new Error('image not allowed')) + } + const options = DockerRunner._getContainerOptions( command, image, diff --git a/app/js/RequestParser.js b/app/js/RequestParser.js index ecc92c0..342dbc8 100644 --- a/app/js/RequestParser.js +++ b/app/js/RequestParser.js @@ -61,7 +61,13 @@ module.exports = RequestParser = { response.imageName = this._parseAttribute( 'imageName', compile.options.imageName, - { type: 'string', validValues: settings.allowedImageNamesFlat } + { + type: 'string', + validValues: + settings.clsi && + settings.clsi.docker && + settings.clsi.docker.allowedImages + } ) response.draft = this._parseAttribute('draft', compile.options.draft, { default: false, diff --git a/config/settings.defaults.js b/config/settings.defaults.js index fd2c28a..823f1f7 100644 --- a/config/settings.defaults.js +++ b/config/settings.defaults.js @@ -73,16 +73,6 @@ if (process.env.ALLOWED_COMPILE_GROUPS) { process.exit(1) } } -if (process.env.ALLOWED_IMAGE_NAMES_FLAT) { - try { - module.exports.allowedImageNamesFlat = process.env.ALLOWED_IMAGE_NAMES_FLAT.split( - ' ' - ) - } catch (error) { - console.error(error, 'could not apply allowed image names setting') - process.exit(1) - } -} if (process.env.DOCKER_RUNNER) { let seccompProfilePath @@ -139,6 +129,17 @@ if (process.env.DOCKER_RUNNER) { process.exit(1) } + if (process.env.ALLOWED_IMAGES) { + try { + module.exports.clsi.docker.allowedImages = process.env.ALLOWED_IMAGES.split( + ' ' + ) + } catch (error) { + console.error(error, 'could not apply allowed images setting') + process.exit(1) + } + } + module.exports.path.synctexBaseDir = () => '/compile' module.exports.path.sandboxedCompilesHostDir = process.env.COMPILES_HOST_DIR diff --git a/docker-compose-config.yml b/docker-compose-config.yml index d1b72ee..afe56bb 100644 --- a/docker-compose-config.yml +++ b/docker-compose-config.yml @@ -3,7 +3,7 @@ version: "2.3" services: dev: environment: - ALLOWED_IMAGE_NAMES_FLAT: "quay.io/sharelatex/texlive-full:2017.1" + ALLOWED_IMAGES: "quay.io/sharelatex/texlive-full:2017.1" TEXLIVE_IMAGE: quay.io/sharelatex/texlive-full:2017.1 TEXLIVE_IMAGE_USER: "tex" SHARELATEX_CONFIG: /app/config/settings.defaults.coffee @@ -19,7 +19,7 @@ services: ci: environment: - ALLOWED_IMAGE_NAMES_FLAT: ${TEXLIVE_IMAGE} + ALLOWED_IMAGES: ${TEXLIVE_IMAGE} TEXLIVE_IMAGE: quay.io/sharelatex/texlive-full:2017.1 TEXLIVE_IMAGE_USER: "tex" SHARELATEX_CONFIG: /app/config/settings.defaults.coffee diff --git a/test/unit/js/CompileControllerTests.js b/test/unit/js/CompileControllerTests.js index f3f3fa4..8bb83e6 100644 --- a/test/unit/js/CompileControllerTests.js +++ b/test/unit/js/CompileControllerTests.js @@ -306,9 +306,10 @@ describe('CompileController', function() { .should.equal(true) }) - describe('when allowedImageNamesFlat is set', function() { + describe('when allowedImages is set', function() { beforeEach(function() { - this.Settings.allowedImageNamesFlat = [ + this.Settings.clsi = { docker: {} } + this.Settings.clsi.docker.allowedImages = [ 'repo/image:tag1', 'repo/image:tag2' ] diff --git a/test/unit/js/DockerRunnerTests.js b/test/unit/js/DockerRunnerTests.js index 16ecbbc..9c1731a 100644 --- a/test/unit/js/DockerRunnerTests.js +++ b/test/unit/js/DockerRunnerTests.js @@ -273,7 +273,7 @@ describe('DockerRunner', function() { }) }) - return describe('with image override', function() { + describe('with image override', function() { beforeEach(function() { this.Settings.texliveImageNameOveride = 'overrideimage.com/something' this.DockerRunner._runAndWaitForContainer = sinon @@ -296,6 +296,62 @@ describe('DockerRunner', function() { return image.should.equal('overrideimage.com/something/image:2016.2') }) }) + + describe('with image restriction', function() { + beforeEach(function() { + this.Settings.clsi.docker.allowedImages = [ + 'repo/image:tag1', + 'repo/image:tag2' + ] + this.DockerRunner._runAndWaitForContainer = sinon + .stub() + .callsArgWith(3, null, (this.output = 'mock-output')) + }) + + describe('with a valid image', function() { + beforeEach(function() { + this.DockerRunner.run( + this.project_id, + this.command, + this.directory, + 'repo/image:tag1', + this.timeout, + this.env, + this.compileGroup, + this.callback + ) + }) + + it('should setup the container', function() { + this.DockerRunner._getContainerOptions.called.should.equal(true) + }) + }) + + describe('with a invalid image', function() { + beforeEach(function() { + this.DockerRunner.run( + this.project_id, + this.command, + this.directory, + 'something/different:evil', + this.timeout, + this.env, + this.compileGroup, + this.callback + ) + }) + + it('should call the callback with an error', function() { + const err = new Error('image not allowed') + this.callback.called.should.equal(true) + this.callback.args[0][0].message.should.equal(err.message) + }) + + it('should not setup the container', function() { + this.DockerRunner._getContainerOptions.called.should.equal(false) + }) + }) + }) }) describe('run with _getOptions', function() { diff --git a/test/unit/js/RequestParserTests.js b/test/unit/js/RequestParserTests.js index 16955bb..25b6b29 100644 --- a/test/unit/js/RequestParserTests.js +++ b/test/unit/js/RequestParserTests.js @@ -116,7 +116,11 @@ describe('RequestParser', function() { describe('when image restrictions are present', function() { beforeEach(function() { - this.settings.allowedImageNamesFlat = ['repo/name:tag1', 'repo/name:tag2'] + this.settings.clsi = { docker: {} } + this.settings.clsi.docker.allowedImages = [ + 'repo/name:tag1', + 'repo/name:tag2' + ] }) describe('with imageName set to something invalid', function() {