diff --git a/app/js/CompileController.js b/app/js/CompileController.js index c76d0d5..9c18830 100644 --- a/app/js/CompileController.js +++ b/app/js/CompileController.js @@ -218,6 +218,15 @@ module.exports = CompileController = { const { project_id } = req.params const { user_id } = req.params const { image } = req.query + if ( + image && + Settings.clsi && + Settings.clsi.docker && + Settings.clsi.docker.allowedImages && + !Settings.clsi.docker.allowedImages.includes(image) + ) { + return res.status(400).send('invalid image') + } logger.log({ image, file, project_id }, 'word count request') return CompileManager.wordcount(project_id, user_id, file, image, function( diff --git a/app/js/DockerRunner.js b/app/js/DockerRunner.js index 5874a51..49c7f40 100644 --- a/app/js/DockerRunner.js +++ b/app/js/DockerRunner.js @@ -86,6 +86,13 @@ module.exports = DockerRunner = { ;({ image } = Settings.clsi.docker) } + if ( + Settings.clsi.docker.allowedImages && + !Settings.clsi.docker.allowedImages.includes(image) + ) { + return callback(new Error('image not allowed')) + } + if (Settings.texliveImageNameOveride != null) { const img = image.split('/') image = `${Settings.texliveImageNameOveride}/${img[2]}` diff --git a/app/js/RequestParser.js b/app/js/RequestParser.js index f2be556..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' } + { + 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 3e115e8..823f1f7 100644 --- a/config/settings.defaults.js +++ b/config/settings.defaults.js @@ -129,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 392f8fe..afe56bb 100644 --- a/docker-compose-config.yml +++ b/docker-compose-config.yml @@ -3,6 +3,7 @@ version: "2.3" services: dev: environment: + 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 @@ -18,6 +19,7 @@ services: ci: environment: + 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/acceptance/js/AllowedImageNames.js b/test/acceptance/js/AllowedImageNames.js new file mode 100644 index 0000000..a9b3996 --- /dev/null +++ b/test/acceptance/js/AllowedImageNames.js @@ -0,0 +1,102 @@ +const Client = require('./helpers/Client') +const ClsiApp = require('./helpers/ClsiApp') +const { expect } = require('chai') + +describe('AllowedImageNames', function() { + beforeEach(function(done) { + this.project_id = Client.randomId() + this.request = { + options: { + imageName: undefined + }, + resources: [ + { + path: 'main.tex', + content: `\ +\\documentclass{article} +\\begin{document} +Hello world +\\end{document}\ +` + } + ] + } + ClsiApp.ensureRunning(done) + }) + + describe('with a valid name', function() { + beforeEach(function(done) { + this.request.options.imageName = process.env.TEXLIVE_IMAGE + + Client.compile(this.project_id, this.request, (error, res, body) => { + this.error = error + this.res = res + this.body = body + done(error) + }) + }) + it('should return success', function() { + expect(this.res.statusCode).to.equal(200) + }) + + it('should return a PDF', function() { + let pdf + try { + pdf = Client.getOutputFile(this.body, 'pdf') + } catch (e) {} + expect(pdf).to.exist + }) + }) + + describe('with an invalid name', function() { + beforeEach(function(done) { + this.request.options.imageName = 'something/evil:1337' + Client.compile(this.project_id, this.request, (error, res, body) => { + this.error = error + this.res = res + this.body = body + done(error) + }) + }) + it('should return non success', function() { + expect(this.res.statusCode).to.not.equal(200) + }) + + it('should not return a PDF', function() { + let pdf + try { + pdf = Client.getOutputFile(this.body, 'pdf') + } catch (e) {} + expect(pdf).to.not.exist + }) + }) + + describe('wordcount', function() { + beforeEach(function(done) { + Client.compile(this.project_id, this.request, done) + }) + it('should error out with an invalid imageName', function() { + Client.wordcountWithImage( + this.project_id, + 'main.tex', + 'something/evil:1337', + (error, result) => { + expect(String(error)).to.include('statusCode=400') + } + ) + }) + + it('should produce a texcout a valid imageName', function() { + Client.wordcountWithImage( + this.project_id, + 'main.tex', + process.env.TEXLIVE_IMAGE, + (error, result) => { + expect(error).to.not.exist + expect(result).to.exist + expect(result.texcount).to.exist + } + ) + }) + }) +}) diff --git a/test/acceptance/js/helpers/Client.js b/test/acceptance/js/helpers/Client.js index d659416..c940e30 100644 --- a/test/acceptance/js/helpers/Client.js +++ b/test/acceptance/js/helpers/Client.js @@ -189,6 +189,11 @@ module.exports = Client = { }, wordcount(project_id, file, callback) { + const image = undefined + Client.wordcountWithImage(project_id, file, image, callback) + }, + + wordcountWithImage(project_id, file, image, callback) { if (callback == null) { callback = function(error, pdfPositions) {} } @@ -196,6 +201,7 @@ module.exports = Client = { { url: `${this.host}/project/${project_id}/wordcount`, qs: { + image, file } }, @@ -203,6 +209,9 @@ module.exports = Client = { if (error != null) { return callback(error) } + if (response.statusCode !== 200) { + return callback(new Error(`statusCode=${response.statusCode}`)) + } return callback(null, JSON.parse(body)) } ) diff --git a/test/unit/js/CompileControllerTests.js b/test/unit/js/CompileControllerTests.js index 4480c88..8bb83e6 100644 --- a/test/unit/js/CompileControllerTests.js +++ b/test/unit/js/CompileControllerTests.js @@ -12,6 +12,7 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') require('chai').should() +const { expect } = require('chai') const modulePath = require('path').join( __dirname, '../../../app/js/CompileController' @@ -287,21 +288,60 @@ describe('CompileController', function() { this.CompileManager.wordcount = sinon .stub() .callsArgWith(4, null, (this.texcount = ['mock-texcount'])) - return this.CompileController.wordcount(this.req, this.res, this.next) }) it('should return the word count of a file', function() { + this.CompileController.wordcount(this.req, this.res, this.next) return this.CompileManager.wordcount .calledWith(this.project_id, undefined, this.file, this.image) .should.equal(true) }) - return it('should return the texcount info', function() { + it('should return the texcount info', function() { + this.CompileController.wordcount(this.req, this.res, this.next) return this.res.json .calledWith({ texcount: this.texcount }) .should.equal(true) }) + + describe('when allowedImages is set', function() { + beforeEach(function() { + this.Settings.clsi = { docker: {} } + this.Settings.clsi.docker.allowedImages = [ + 'repo/image:tag1', + 'repo/image:tag2' + ] + this.res.send = sinon.stub() + this.res.status = sinon.stub().returns({ send: this.res.send }) + }) + + describe('with an invalid image', function() { + beforeEach(function() { + this.req.query.image = 'something/evil:1337' + this.CompileController.wordcount(this.req, this.res, this.next) + }) + it('should return a 400', function() { + expect(this.res.status.calledWith(400)).to.equal(true) + }) + it('should not run the query', function() { + expect(this.CompileManager.wordcount.called).to.equal(false) + }) + }) + + describe('with a valid image', function() { + beforeEach(function() { + this.req.query.image = 'repo/image:tag1' + this.CompileController.wordcount(this.req, this.res, this.next) + }) + it('should not return a 400', function() { + expect(this.res.status.calledWith(400)).to.equal(false) + }) + it('should run the query', function() { + expect(this.CompileManager.wordcount.called).to.equal(true) + }) + }) + }) }) }) 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 e2d8b02..25b6b29 100644 --- a/test/unit/js/RequestParserTests.js +++ b/test/unit/js/RequestParserTests.js @@ -114,6 +114,48 @@ describe('RequestParser', function() { }) }) + describe('when image restrictions are present', function() { + beforeEach(function() { + this.settings.clsi = { docker: {} } + this.settings.clsi.docker.allowedImages = [ + 'repo/name:tag1', + 'repo/name:tag2' + ] + }) + + describe('with imageName set to something invalid', function() { + beforeEach(function() { + const request = this.validRequest + request.compile.options.imageName = 'something/different:latest' + this.RequestParser.parse(request, (error, data) => { + this.error = error + this.data = data + }) + }) + + it('should throw an error for imageName', function() { + expect(String(this.error)).to.include( + 'imageName attribute should be one of' + ) + }) + }) + + describe('with imageName set to something valid', function() { + beforeEach(function() { + const request = this.validRequest + request.compile.options.imageName = 'repo/name:tag1' + this.RequestParser.parse(request, (error, data) => { + this.error = error + this.data = data + }) + }) + + it('should set the imageName', function() { + this.data.imageName.should.equal('repo/name:tag1') + }) + }) + }) + describe('with flags set', function() { beforeEach(function() { this.validRequest.compile.options.flags = ['-file-line-error']