Allow convert command to be prefixed by security commands

services/filestore/app/coffee/FileConverter.coffee

@@ -3,6 +3,7 @@ metrics = require("metrics-sharelatex")
 logger = require("logger-sharelatex")
 safe_exec = require("./SafeExec")
 approvedFormats = ["png"]
+Settings = require "settings-sharelatex"
 
 fourtySeconds = 40 * 1000
 
@@ -22,8 +23,9 @@ module.exports =
 			err = new Error("invalid format requested")
 			return callback err
 		width = "600x"
-		args = "nice convert -define pdf:fit-page=#{width} -flatten -density 300 #{sourcePath} #{destPath}"
-		safe_exec args, childProcessOpts, (err, stdout, stderr)->
+		command = ["convert", "-define", "pdf:fit-page=#{width}", "-flatten", "-density", "300", sourcePath, destPath]
+		command = Settings.commands.convertCommandPrefix.concat(command)
+		safe_exec command, childProcessOpts, (err, stdout, stderr)->
 			timer.done()
 			if err?
 				logger.err err:err, stderr:stderr, sourcePath:sourcePath, requestedFormat:requestedFormat, destPath:destPath,  "something went wrong converting file"
@@ -36,8 +38,9 @@ module.exports =
 		destPath = "#{sourcePath}.png"
 		sourcePath = "#{sourcePath}[0]"
 		width = "260x"
-		args = "nice convert -flatten -background white -density 300 -define pdf:fit-page=#{width} #{sourcePath} -resize #{width} #{destPath}"
-		safe_exec args, childProcessOpts, (err, stdout, stderr)->
+		command = ["convert", "-flatten", "-background", "white", "-density", "300", "-define", "pdf:fit-page=#{width}", sourcePath, "-resize", width, destPath]
+		command = Settings.commands.convertCommandPrefix.concat(command)
+		safe_exec command, childProcessOpts, (err, stdout, stderr)->
 			if err?
 				logger.err err:err, stderr:stderr, sourcePath:sourcePath, "something went wrong converting file to thumbnail"
 			else
@@ -49,8 +52,9 @@ module.exports =
 		destPath = "#{sourcePath}.png"
 		sourcePath = "#{sourcePath}[0]"
 		width = "548x"
-		args = "nice convert -flatten -background white -density 300 -define pdf:fit-page=#{width} #{sourcePath} -resize #{width} #{destPath}"
-		safe_exec args, childProcessOpts, (err, stdout, stderr)->
+		command = ["convert", "-flatten", "-background", "white", "-density", "300", "-define", "pdf:fit-page=#{width}", sourcePath, "-resize", width, destPath]
+		command = Settings.commands.convertCommandPrefix.concat(command)
+		safe_exec command, childProcessOpts, (err, stdout, stderr)->
 			if err?
 				logger.err err:err, stderr:stderr, sourcePath:sourcePath, destPath:destPath, "something went wrong converting file to preview"
 			else

services/filestore/app/coffee/SafeExec.coffee

@@ -10,7 +10,7 @@ child_process = require('child_process')
 
 module.exports = (command, options, callback = (err, stdout, stderr) ->) ->
 	# options are {timeout:  number-of-milliseconds, killSignal: signal-name}
-	[cmd, args...] = command.split(' ')
+	[cmd, args...] = command
 
 	child = child_process.spawn cmd, args, {detached:true}
 	stdout = ""

services/filestore/config/settings.defaults.coffee

@@ -30,6 +30,10 @@ module.exports =
 
 	path:
 		uploadFolder: Path.resolve(__dirname + "/../uploads")
+		
+	commands:
+		# Any commands to wrap the convert utility in, for example ["nice"], or ["firejail", "--profile=/etc/firejail/convert.profile"]
+		convertCommandPrefix: []
 
 	# Filestore health check
 	# ----------------------

services/filestore/test/unit/coffee/FileConverterTests.coffee

@@ -16,6 +16,9 @@ describe "FileConverter", ->
 			"logger-sharelatex":
 				log:->
 				err:->
+			"settings-sharelatex": @Settings =
+				commands:
+					convertCommandPrefix: []
 
 		@sourcePath = "/this/path/here.eps"
 		@format = "png"
@@ -27,8 +30,8 @@ describe "FileConverter", ->
 			@safe_exec.callsArgWith(2)
 			@converter.convert @sourcePath, @format, (err)=>
 				args = @safe_exec.args[0][0]
-				args.indexOf(@sourcePath).should.not.equal -1 
-				args.indexOf(@format).should.not.equal -1 
+				args.indexOf("#{@sourcePath}[0]").should.not.equal -1 
+				args.indexOf("#{@sourcePath}.#{@format}").should.not.equal -1 
 				done()
 
 		it "should return the dest path", (done)->
@@ -48,13 +51,21 @@ describe "FileConverter", ->
 			@converter.convert @sourcePath, "ahhhhh", (err)=>
 				expect(err).to.exist
 				done()
+		
+		it "should prefix the command with Settings.commands.convertCommandPrefix", (done) ->
+			@safe_exec.callsArgWith(2)
+			@Settings.commands.convertCommandPrefix = ["nice"]
+			@converter.convert @sourcePath, @format, (err)=>
+				command = @safe_exec.args[0][0]
+				command[0].should.equal "nice"
+				done()
 
 	describe "thumbnail", ->
 		it "should call converter resize with args", (done)->
 			@safe_exec.callsArgWith(2)
 			@converter.thumbnail @sourcePath, (err)=>
 				args = @safe_exec.args[0][0]
-				args.indexOf(@sourcePath).should.not.equal -1 
+				args.indexOf("#{@sourcePath}[0]").should.not.equal -1 
 				done()
 
 	describe "preview", ->
@@ -62,5 +73,5 @@ describe "FileConverter", ->
 			@safe_exec.callsArgWith(2)
 			@converter.preview @sourcePath, (err)=>
 				args = @safe_exec.args[0][0]
-				args.indexOf(@sourcePath).should.not.equal -1 
+				args.indexOf("#{@sourcePath}[0]").should.not.equal -1 
 				done()

services/filestore/test/unit/coffee/SafeExec.coffee

@@ -19,24 +19,24 @@ describe "SafeExec", ->
 	describe "safe_exec", ->
 
 		it "should execute a valid command", (done) ->
-			@safe_exec "/bin/echo hello", @options, (err, stdout, stderr) =>
+			@safe_exec ["/bin/echo", "hello"], @options, (err, stdout, stderr) =>
 				stdout.should.equal "hello\n"
 				should.not.exist(err)
 				done()
 
 		it "should execute a command with non-zero exit status", (done) ->
-			@safe_exec "/usr/bin/env false", @options, (err, stdout, stderr) =>
+			@safe_exec ["/usr/bin/env", "false"], @options, (err, stdout, stderr) =>
 				stdout.should.equal ""
 				stderr.should.equal ""
 				err.message.should.equal "exit status 1"
 				done()
 
 		it "should handle an invalid command", (done) ->
-			@safe_exec "/bin/foobar", @options, (err, stdout, stderr) =>
+			@safe_exec ["/bin/foobar"], @options, (err, stdout, stderr) =>
 				err.code.should.equal "ENOENT"
 				done()
 
 		it "should handle a command that runs too long", (done) ->
-			@safe_exec "/bin/sleep 10", {timeout: 500, killSignal: "SIGTERM"}, (err, stdout, stderr) =>
+			@safe_exec ["/bin/sleep", "10"], {timeout: 500, killSignal: "SIGTERM"}, (err, stdout, stderr) =>
 				err.should.equal "SIGTERM"
 				done()