Răsfoiți Sursa

[misc] RequestParser: restrict imageName to an allow list and add tests

Jakob Ackermann 6 ani în urmă
părinte
comite
8846efe7ce

+ 1 - 1
services/clsi/app/js/RequestParser.js

@@ -61,7 +61,7 @@ module.exports = RequestParser = {
       response.imageName = this._parseAttribute(
         'imageName',
         compile.options.imageName,
-        { type: 'string' }
+        { type: 'string', validValues: settings.allowedImageNamesFlat }
       )
       response.draft = this._parseAttribute('draft', compile.options.draft, {
         default: false,

+ 10 - 0
services/clsi/config/settings.defaults.js

@@ -73,6 +73,16 @@ 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

+ 2 - 0
services/clsi/docker-compose-config.yml

@@ -3,6 +3,7 @@ version: "2.3"
 services:
   dev:
     environment:
+      ALLOWED_IMAGE_NAMES_FLAT: "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_IMAGE_NAMES_FLAT: ${TEXLIVE_IMAGE}
       TEXLIVE_IMAGE: quay.io/sharelatex/texlive-full:2017.1
       TEXLIVE_IMAGE_USER: "tex"
       SHARELATEX_CONFIG: /app/config/settings.defaults.coffee

+ 73 - 0
services/clsi/test/acceptance/js/AllowedImageNames.js

@@ -0,0 +1,73 @@
+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
+    })
+  })
+})

+ 38 - 0
services/clsi/test/unit/js/RequestParserTests.js

@@ -114,6 +114,44 @@ describe('RequestParser', function() {
     })
   })
 
+  describe('when image restrictions are present', function() {
+    beforeEach(function() {
+      this.settings.allowedImageNamesFlat = ['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']