Merge pull request #1005 from sharelatex/spd-no-more-asterisks

Sanitize paths in all relevant ProjectEntityHandler methods

services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee

@@ -129,6 +129,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 		Project.update {_id:project_id}, {$unset: {rootDoc_id: true}}, {}, callback
 
 	addDoc: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=>
+		if not SafePath.isCleanFilename docName
+			return callback new Errors.InvalidNameError("invalid element name")
 		self.addDocWithoutUpdatingHistory.withoutLock project_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path, project) ->
 			return callback(error) if error?
 			projectHistoryId = project.overleaf?.history?.id
@@ -166,6 +168,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 	addFile: wrapWithLock
 		beforeLock: (next) ->
 			(project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) ->
+				if not SafePath.isCleanFilename fileName
+					return callback new Errors.InvalidNameError("invalid element name")
 				ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) ->
 					return callback(error) if error?
 					next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback)
@@ -241,6 +245,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 		# the history unless you are making sure it is updated in some other way.
 		beforeLock: (next) ->
 			(project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) ->
+				if not SafePath.isCleanFilename fileName
+					return callback(new Errors.InvalidNameError("invalid element name"))
 				ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) ->
 					return callback(error) if error?
 					next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback)
@@ -250,6 +256,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 				callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl)
 
 	upsertDoc: wrapWithLock (project_id, folder_id, docName, docLines, source, userId, callback = (err, doc, folder_id, isNewDoc)->)->
+		if not SafePath.isCleanFilename docName
+			return callback new Errors.InvalidNameError("invalid element name")
 		ProjectLocator.findElement project_id: project_id, element_id: folder_id, type: "folder", (error, folder) ->
 			return callback(error) if error?
 			return callback(new Error("Couldn't find folder")) if !folder?
@@ -272,6 +280,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 	upsertFile: wrapWithLock
 		beforeLock: (next) ->
 			(project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback)->
+				if not SafePath.isCleanFilename fileName
+					return callback new Errors.InvalidNameError("invalid element name")
 				# create a new file
 				fileRef = new File(
 					name: fileName
@@ -301,6 +311,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 						callback null, newFileRef, !existingFile?, existingFile
 
 	upsertDocWithPath: wrapWithLock (project_id, elementPath, docLines, source, userId, callback) ->
+		if not SafePath.isCleanPath elementPath
+			return callback new Errors.InvalidNameError("invalid element name")
 		docName = path.basename(elementPath)
 		folderPath = path.dirname(elementPath)
 		self.mkdirp.withoutLock project_id, folderPath, (err, newFolders, folder) ->
@@ -312,6 +324,8 @@ module.exports = ProjectEntityUpdateHandler = self =
 	upsertFileWithPath: wrapWithLock
 		beforeLock: (next) ->
 			(project_id, elementPath, fsPath, linkedFileData, userId, callback)->
+				if not SafePath.isCleanPath elementPath
+					return callback new Errors.InvalidNameError("invalid element name")
 				fileName = path.basename(elementPath)
 				folderPath = path.dirname(elementPath)
 				# create a new file
@@ -351,6 +365,9 @@ module.exports = ProjectEntityUpdateHandler = self =
 			self.deleteEntity.withoutLock project_id, element._id, type, userId, callback
 
 	mkdirp: wrapWithLock (project_id, path, callback = (err, newlyCreatedFolders, lastFolderInPath)->)->
+		for folder in path.split('/')
+			if folder.length > 0 and not SafePath.isCleanFilename folder
+				return callback new Errors.InvalidNameError("invalid element name")
 		ProjectEntityMongoUpdateHandler.mkdirp project_id, path, callback
 
 	addFolder: wrapWithLock (project_id, parentFolder_id, folderName, callback) ->

services/web/app/coffee/Features/Project/SafePath.coffee

@@ -52,6 +52,7 @@ load = () ->
 	MAX_PATH = 1024 # Maximum path length, in characters. This is fairly arbitrary.
 
 	SafePath =
+		# convert any invalid characters to underscores in the given filename
 		clean: (filename) ->
 			filename = filename.replace BADCHAR_RX, '_'
 			# for BADFILE_RX replace any matches with an equal number of underscores
@@ -61,12 +62,27 @@ load = () ->
 			filename = filename.replace BLOCKEDFILE_RX, "@$1"
 			return filename
 
+		# returns whether the filename is 'clean' (does not contain any invalid
+		# characters or reserved words)
 		isCleanFilename: (filename) ->
 			return SafePath.isAllowedLength(filename) &&
 				!BADCHAR_RX.test(filename) &&
 				!BADFILE_RX.test(filename) &&
 				!BLOCKEDFILE_RX.test(filename)
 
+		# returns whether a full path is 'clean' - e.g. is a full or relative path
+		# that points to a file, and each element passes the rules in 'isCleanFilename'
+		isCleanPath: (path) ->
+			elements = path.split('/')
+
+			lastElementIsEmpty = elements[elements.length - 1].length == 0
+			return false if lastElementIsEmpty
+
+			for element in elements
+				return false if element.length > 0 && !SafePath.isCleanFilename element
+
+			return true
+
 		isAllowedLength: (pathname) ->
 			return pathname.length > 0 && pathname.length <= MAX_PATH
 

services/web/app/views/project/list/modals.pug

@@ -98,7 +98,7 @@ script(type='text/ng-template', id='renameProjectModalTemplate')
 		) ×
 		h3 #{translate("rename_project")}
 	.modal-body
-		.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} 
+		.alert.alert-danger(ng-show="state.error.message") {{state.error.message}}
 		.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
 		form(name="renameProjectForm", novalidate)
 			input.form-control(
@@ -127,7 +127,7 @@ script(type='text/ng-template', id='cloneProjectModalTemplate')
 		) ×
 		h3 #{translate("copy_project")}
 	.modal-body
-		.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} 
+		.alert.alert-danger(ng-show="state.error.message") {{state.error.message === "invalid element name" ? translate("invalid_element_name") : state.error.message}}
 		.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
 		form(name="cloneProjectForm", novalidate)
 			.form-group
@@ -161,7 +161,7 @@ script(type='text/ng-template', id='newProjectModalTemplate')
 		) ×
 		h3 #{translate("new_project")}
 	.modal-body
-		.alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} 
+		.alert.alert-danger(ng-show="state.error.message") {{state.error.message}}
 		.alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")}
 		form(novalidate, name="newProjectForm")
 			input.form-control(
@@ -262,6 +262,20 @@ script(type="text/ng-template", id="uploadProjectModalTemplate")
 	.modal-footer
 		button.btn.btn-default(ng-click="cancel()") #{translate("cancel")}
 
+script(type="text/ng-template", id="showErrorModalTemplate")
+	.modal-header
+		button.close(
+			type="button"
+			data-dismiss="modal"
+			ng-click="cancel()"
+		) ×
+		h3 #{translate("generic_something_went_wrong")}
+	.modal-body
+		.alert.alert-danger(ng-show="error.message") {{error.message === "invalid element name" ? translate("invalid_element_name") : error.message}}
+		.alert.alert-danger(ng-show="error && !error.message") #{translate("generic_something_went_wrong")}
+	.modal-footer
+		button.btn.btn-default(ng-click="cancel()") #{translate("cancel")}
+
 script(type="text/ng-template", id="userProfileModalTemplate")
 	.modal-header
 		button.close(

services/web/public/coffee/main/project-list/modal-controllers.coffee

@@ -2,9 +2,9 @@ define [
 	"base"
 ], (App) ->
 	App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, project, queuedHttp) ->
-		$scope.inputs = 
+		$scope.inputs =
 			projectName: project.name
-		
+
 		$scope.state =
 			inflight: false
 			error: false
@@ -35,7 +35,7 @@ define [
 			$modalInstance.dismiss('cancel')
 
 	App.controller 'CloneProjectModalController', ($scope, $modalInstance, $timeout, project) ->
-		$scope.inputs = 
+		$scope.inputs =
 			projectName: project.name + " (Copy)"
 		$scope.state =
 			inflight: false
@@ -66,7 +66,7 @@ define [
 			$modalInstance.dismiss('cancel')
 
 	App.controller 'NewProjectModalController', ($scope, $modalInstance, $timeout, template) ->
-		$scope.inputs = 
+		$scope.inputs =
 			projectName: ""
 		$scope.state =
 			inflight: false
@@ -123,7 +123,6 @@ define [
 		$scope.cancel = () ->
 			$modalInstance.dismiss('cancel')
 
-
 	App.controller 'UploadProjectModalController', ($scope, $modalInstance, $timeout) ->
 		$scope.cancel = () ->
 			$modalInstance.dismiss('cancel')
@@ -137,3 +136,8 @@ define [
 
 		$scope.dismiss = () ->
 			$modalInstance.dismiss('cancel')
+
+	App.controller 'ShowErrorModalController', ($scope, $modalInstance, error) ->
+		$scope.error = error
+		$scope.cancel = () ->
+			$modalInstance.dismiss('cancel')

services/web/public/coffee/main/project-list/project-list.coffee

@@ -13,7 +13,7 @@ define [
 		$scope.predicate = "lastUpdated"
 		$scope.nUntagged = 0
 		$scope.reverse = true
-		$scope.searchText = 
+		$scope.searchText =
 			value : ""
 
 		$timeout () ->
@@ -37,7 +37,7 @@ define [
 		angular.element($window).bind "resize", () ->
 			recalculateProjectListHeight()
 			$scope.$apply()
-			
+
 		# Allow tags to be accessed on projects as well
 		projectsById = {}
 		for project in $scope.projects
@@ -56,7 +56,7 @@ define [
 					tag.selected = true
 				else
 					tag.selected = false
-		
+
 		$scope.changePredicate = (newPredicate)->
 			if $scope.predicate == newPredicate
 				$scope.reverse = !$scope.reverse
@@ -145,7 +145,7 @@ define [
 					# We don't want hidden selections
 					project.selected = false
 
-			localStorage("project_list", JSON.stringify({ 
+			localStorage("project_list", JSON.stringify({
 				filter: $scope.filter,
 				selectedTagId: selectedTag?._id
 			}))
@@ -461,7 +461,7 @@ define [
 				resolve:
 					project: () -> project
 			)
-			
+
 		if storedUIOpts?.filter?
 			if storedUIOpts.filter == "tag" and storedUIOpts.selectedTagId?
 				markTagAsSelected(storedUIOpts.selectedTagId)
@@ -505,7 +505,16 @@ define [
 			$scope.project.isTableActionInflight = true
 			$scope.cloneProject($scope.project, "#{$scope.project.name} (Copy)")
 				.then () -> $scope.project.isTableActionInflight = false
-				.catch () -> $scope.project.isTableActionInflight = false
+				.catch (response) ->
+					{ data, status } = response
+					error = if status == 400 then message: data else true
+					modalInstance = $modal.open(
+						templateUrl: "showErrorModalTemplate"
+						controller: "ShowErrorModalController"
+						resolve:
+							error: () -> error
+					)
+					$scope.project.isTableActionInflight = false
 
 		$scope.download = (e) ->
 			e.stopPropagation()
@@ -535,11 +544,11 @@ define [
 					url: "/project/#{$scope.project.id}?forever=true"
 					headers:
 						"X-CSRF-Token": window.csrfToken
-				}).then () -> 
+				}).then () ->
 					$scope.project.isTableActionInflight = false
 					$scope._removeProjectFromList $scope.project
 					for tag in $scope.tags
 						$scope._removeProjectIdsFromTagArray(tag, [ $scope.project.id ])
 					$scope.updateVisibleProjects()
-				.catch () -> 
+				.catch () ->
 					$scope.project.isTableActionInflight = false

services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee

@@ -289,71 +289,101 @@ describe 'ProjectEntityUpdateHandler', ->
 				.should.equal true
 
 	describe 'addDoc', ->
-		beforeEach ->
-			@path = "/path/to/doc"
+		describe 'adding a doc', ->
+			beforeEach ->
+				@path = "/path/to/doc"
 
-			@newDoc = _id: doc_id
-			@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory =
-				withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project)
-			@ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback
+				@newDoc = _id: doc_id
+				@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory =
+					withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project)
+				@ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback
 
-		it "creates the doc without history", () ->
-			@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory.withoutLock
-				.calledWith(project_id, folder_id, @docName, @docLines, userId)
-				.should.equal true
+			it "creates the doc without history", () ->
+				@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory.withoutLock
+					.calledWith(project_id, folder_id, @docName, @docLines, userId)
+					.should.equal true
 
-		it "sends the change in project structure to the doc updater", () ->
-			newDocs = [
-				doc: @newDoc
-				path: @path
-				docLines: @docLines.join('\n')
-			]
-			@DocumentUpdaterHandler.updateProjectStructure
-				.calledWith(project_id, projectHistoryId, userId, {newDocs})
-				.should.equal true
+			it "sends the change in project structure to the doc updater", () ->
+				newDocs = [
+					doc: @newDoc
+					path: @path
+					docLines: @docLines.join('\n')
+				]
+				@DocumentUpdaterHandler.updateProjectStructure
+					.calledWith(project_id, projectHistoryId, userId, {newDocs})
+					.should.equal true
+
+		describe 'adding a doc with an invalid name', ->
+			beforeEach ->
+				@path = "/path/to/doc"
+
+				@newDoc = _id: doc_id
+				@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory =
+					withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project)
+				@ProjectEntityUpdateHandler.addDoc project_id, folder_id, "*" + @docName, @docLines, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'addFile', ->
-		beforeEach ->
-			@path = "/path/to/file"
+		describe 'adding a file', ->
+			beforeEach ->
+				@path = "/path/to/file"
 
-			@newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData}
-			@TpdsUpdateSender.addFile = sinon.stub().yields()
-			@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
-			@ProjectEntityUpdateHandler.addFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback
+				@newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData}
+				@TpdsUpdateSender.addFile = sinon.stub().yields()
+				@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback
 
-		it "updates the file in the filestore", () ->
-			@FileStoreHandler.uploadFileFromDisk
-				.calledWith(project_id, file_id, @fileSystemPath)
-				.should.equal true
+			it "updates the file in the filestore", () ->
+				@FileStoreHandler.uploadFileFromDisk
+					.calledWith(project_id, file_id, @fileSystemPath)
+					.should.equal true
 
-		it "updates the file in mongo", () ->
-			fileMatcher = sinon.match (file) =>
-				file.name == @fileName
+			it "updates the file in mongo", () ->
+				fileMatcher = sinon.match (file) =>
+					file.name == @fileName
 
-			@ProjectEntityMongoUpdateHandler.addFile
-				.calledWithMatch(project_id, folder_id, fileMatcher)
-				.should.equal true
+				@ProjectEntityMongoUpdateHandler.addFile
+					.calledWithMatch(project_id, folder_id, fileMatcher)
+					.should.equal true
 
-		it "notifies the tpds", () ->
-			@TpdsUpdateSender.addFile
-				.calledWith({
-					project_id: project_id
-					project_name: @project.name
-					file_id: file_id
-					rev: 0
+			it "notifies the tpds", () ->
+				@TpdsUpdateSender.addFile
+					.calledWith({
+						project_id: project_id
+						project_name: @project.name
+						file_id: file_id
+						rev: 0
+						path: @path
+					})
+					.should.equal true
+
+			it "sends the change in project structure to the doc updater", () ->
+				newFiles = [
+					file: @newFile
 					path: @path
-				})
-				.should.equal true
+					url: @fileUrl
+				]
+				@DocumentUpdaterHandler.updateProjectStructure
+					.calledWith(project_id, projectHistoryId, userId, {newFiles})
+					.should.equal true
 
-		it "sends the change in project structure to the doc updater", () ->
-			newFiles = [
-				file: @newFile
-				path: @path
-				url: @fileUrl
-			]
-			@DocumentUpdaterHandler.updateProjectStructure
-				.calledWith(project_id, projectHistoryId, userId, {newFiles})
-				.should.equal true
+		describe 'adding a file with an invalid name', ->
+			beforeEach ->
+				@path = "/path/to/file"
+
+				@newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData}
+				@TpdsUpdateSender.addFile = sinon.stub().yields()
+				@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addFile project_id, folder_id, "*" + @fileName, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'replaceFile', ->
 		beforeEach ->
@@ -404,83 +434,116 @@ describe 'ProjectEntityUpdateHandler', ->
 				.should.equal true
 
 	describe 'addDocWithoutUpdatingHistory', ->
-		beforeEach ->
-			@path = "/path/to/doc"
+		describe 'adding a doc', ->
+			beforeEach ->
+				@path = "/path/to/doc"
 
-			@project = _id: project_id, name: 'some project'
+				@project = _id: project_id, name: 'some project'
 
-			@TpdsUpdateSender.addDoc = sinon.stub().yields()
-			@DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
-			@ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
-			@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, @docName, @docLines, userId, @callback
+				@TpdsUpdateSender.addDoc = sinon.stub().yields()
+				@DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
+				@ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, @docName, @docLines, userId, @callback
 
-		it "updates the doc in the docstore", () ->
-			@DocstoreManager.updateDoc
-				.calledWith(project_id, doc_id, @docLines, 0, {})
-				.should.equal true
+			it "updates the doc in the docstore", () ->
+				@DocstoreManager.updateDoc
+					.calledWith(project_id, doc_id, @docLines, 0, {})
+					.should.equal true
 
-		it "updates the doc in mongo", () ->
-			docMatcher = sinon.match (doc) =>
-				doc.name == @docName
+			it "updates the doc in mongo", () ->
+				docMatcher = sinon.match (doc) =>
+					doc.name == @docName
 
-			@ProjectEntityMongoUpdateHandler.addDoc
-				.calledWithMatch(project_id, folder_id, docMatcher)
-				.should.equal true
+				@ProjectEntityMongoUpdateHandler.addDoc
+					.calledWithMatch(project_id, folder_id, docMatcher)
+					.should.equal true
 
-		it "notifies the tpds", () ->
-			@TpdsUpdateSender.addDoc
-				.calledWith({
-					project_id: project_id
-					project_name: @project.name
-					doc_id: doc_id
-					rev: 0
-					path: @path
-				})
-				.should.equal true
+			it "notifies the tpds", () ->
+				@TpdsUpdateSender.addDoc
+					.calledWith({
+						project_id: project_id
+						project_name: @project.name
+						doc_id: doc_id
+						rev: 0
+						path: @path
+					})
+					.should.equal true
 
-		it "should not should send the change in project structure to the doc updater", () ->
-			@DocumentUpdaterHandler.updateProjectStructure
-				.called
-				.should.equal false
+			it "should not should send the change in project structure to the doc updater", () ->
+				@DocumentUpdaterHandler.updateProjectStructure
+					.called
+					.should.equal false
+
+		describe 'adding a doc with an invalid name', ->
+			beforeEach ->
+				@path = "/path/to/doc"
+
+				@project = _id: project_id, name: 'some project'
+
+				@TpdsUpdateSender.addDoc = sinon.stub().yields()
+				@DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
+				@ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, "*" + @docName, @docLines, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'addFileWithoutUpdatingHistory', ->
-		beforeEach ->
-			@path = "/path/to/file"
+		describe 'adding a file', ->
+			beforeEach ->
+				@path = "/path/to/file"
 
-			@project = _id: project_id, name: 'some project'
+				@project = _id: project_id, name: 'some project'
 
-			@TpdsUpdateSender.addFile = sinon.stub().yields()
-			@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
-			@ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, @fileName, @fileSystemPath, userId, @callback
+				@TpdsUpdateSender.addFile = sinon.stub().yields()
+				@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback
 
-		it "updates the file in the filestore", () ->
-			@FileStoreHandler.uploadFileFromDisk
-				.calledWith(project_id, file_id, @fileSystemPath)
-				.should.equal true
+			it "updates the file in the filestore", () ->
+				@FileStoreHandler.uploadFileFromDisk
+					.calledWith(project_id, file_id, @fileSystemPath)
+					.should.equal true
 
-		it "updates the file in mongo", () ->
-			fileMatcher = sinon.match (file) =>
-				file.name == @fileName
+			it "updates the file in mongo", () ->
+				fileMatcher = sinon.match (file) =>
+					file.name == @fileName
 
-			@ProjectEntityMongoUpdateHandler.addFile
-				.calledWithMatch(project_id, folder_id, fileMatcher)
-				.should.equal true
+				@ProjectEntityMongoUpdateHandler.addFile
+					.calledWithMatch(project_id, folder_id, fileMatcher)
+					.should.equal true
 
-		it "notifies the tpds", () ->
-			@TpdsUpdateSender.addFile
-				.calledWith({
-					project_id: project_id
-					project_name: @project.name
-					file_id: file_id
-					rev: 0
-					path: @path
-				})
-				.should.equal true
+			it "notifies the tpds", () ->
+				@TpdsUpdateSender.addFile
+					.calledWith({
+						project_id: project_id
+						project_name: @project.name
+						file_id: file_id
+						rev: 0
+						path: @path
+					})
+					.should.equal true
 
-		it "should not should send the change in project structure to the doc updater", () ->
-			@DocumentUpdaterHandler.updateProjectStructure
-				.called
-				.should.equal false
+			it "should not should send the change in project structure to the doc updater", () ->
+				@DocumentUpdaterHandler.updateProjectStructure
+					.called
+					.should.equal false
+
+		describe 'adding a file with an invalid name', ->
+			beforeEach ->
+				@path = "/path/to/file"
+
+				@project = _id: project_id, name: 'some project'
+
+				@TpdsUpdateSender.addFile = sinon.stub().yields()
+				@ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project)
+				@ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, "*" + @fileName, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'upsertDoc', ->
 		describe 'upserting into an invalid folder', ->
@@ -543,6 +606,20 @@ describe 'ProjectEntityUpdateHandler', ->
 			it 'returns the doc', ->
 				@callback.calledWith(null, @newDoc, true)
 
+		describe 'upserting a new doc with an invalid name', ->
+			beforeEach ->
+				@folder = _id: folder_id, docs: []
+				@newDoc = _id: doc_id
+				@ProjectLocator.findElement = sinon.stub().yields(null, @folder)
+				@ProjectEntityUpdateHandler.addDoc = withoutLock: sinon.stub().yields(null, @newDoc)
+
+				@ProjectEntityUpdateHandler.upsertDoc project_id, folder_id, "*" + @docName, @docLines, @source, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
+
 	describe 'upsertFile', ->
 		describe 'upserting into an invalid folder', ->
 			beforeEach ->
@@ -593,63 +670,155 @@ describe 'ProjectEntityUpdateHandler', ->
 			it 'returns the file', ->
 				@callback.calledWith(null, @newFile, true)
 
+		describe 'upserting a new file with an invalid name', ->
+			beforeEach ->
+				@folder = _id: folder_id, fileRefs: []
+				@newFile = _id: file_id
+				@ProjectLocator.findElement = sinon.stub().yields(null, @folder)
+				@ProjectEntityUpdateHandler.addFile = mainTask: sinon.stub().yields(null, @newFile)
+
+				@ProjectEntityUpdateHandler.upsertFile project_id, folder_id, '*' + @fileName, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
+
 	describe 'upsertDocWithPath', ->
-		beforeEach ->
-			@path = "/folder/doc.tex"
-			@newFolders = [ 'mock-a', 'mock-b' ]
-			@folder = _id: folder_id
-			@doc = _id: doc_id
-			@isNewDoc = true
-			@ProjectEntityUpdateHandler.mkdirp =
-				withoutLock: sinon.stub().yields(null, @newFolders, @folder)
-			@ProjectEntityUpdateHandler.upsertDoc =
-				withoutLock: sinon.stub().yields(null, @doc, @isNewDoc)
+		describe 'upserting a doc', ->
+			beforeEach ->
+				@path = "/folder/doc.tex"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@doc = _id: doc_id
+				@isNewDoc = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertDoc =
+					withoutLock: sinon.stub().yields(null, @doc, @isNewDoc)
 
-			@ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback
+				@ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback
 
-		it 'creates any necessary folders', ->
-			@ProjectEntityUpdateHandler.mkdirp.withoutLock
-				.calledWith(project_id, '/folder')
-				.should.equal true
+			it 'creates any necessary folders', ->
+				@ProjectEntityUpdateHandler.mkdirp.withoutLock
+					.calledWith(project_id, '/folder')
+					.should.equal true
 
-		it 'upserts the doc', ->
-			@ProjectEntityUpdateHandler.upsertDoc.withoutLock
-				.calledWith(project_id, @folder._id, 'doc.tex', @docLines, @source, userId)
-				.should.equal true
+			it 'upserts the doc', ->
+				@ProjectEntityUpdateHandler.upsertDoc.withoutLock
+					.calledWith(project_id, @folder._id, 'doc.tex', @docLines, @source, userId)
+					.should.equal true
 
-		it 'calls the callback', ->
-			@callback
-				.calledWith(null, @doc, @isNewDoc, @newFolders, @folder)
-				.should.equal true
+			it 'calls the callback', ->
+				@callback
+					.calledWith(null, @doc, @isNewDoc, @newFolders, @folder)
+					.should.equal true
+
+		describe 'upserting a doc with an invalid path', ->
+			beforeEach ->
+				@path = "/*folder/doc.tex"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@doc = _id: doc_id
+				@isNewDoc = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertDoc =
+					withoutLock: sinon.stub().yields(null, @doc, @isNewDoc)
+
+				@ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
+
+		describe 'upserting a doc with an invalid name', ->
+			beforeEach ->
+				@path = "/folder/*doc.tex"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@doc = _id: doc_id
+				@isNewDoc = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertDoc =
+					withoutLock: sinon.stub().yields(null, @doc, @isNewDoc)
+
+				@ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'upsertFileWithPath', ->
-		beforeEach ->
-			@path = "/folder/file.png"
-			@newFolders = [ 'mock-a', 'mock-b' ]
-			@folder = _id: folder_id
-			@file = _id: file_id
-			@isNewFile = true
-			@ProjectEntityUpdateHandler.mkdirp =
-				withoutLock: sinon.stub().yields(null, @newFolders, @folder)
-			@ProjectEntityUpdateHandler.upsertFile =
-				mainTask: sinon.stub().yields(null, @file, @isNewFile)
-
-			@ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback
-
-		it 'creates any necessary folders', ->
-			@ProjectEntityUpdateHandler.mkdirp.withoutLock
-				.calledWith(project_id, '/folder')
-				.should.equal true
+		describe 'upserting a file', ->
+			beforeEach ->
+				@path = "/folder/file.png"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@file = _id: file_id
+				@isNewFile = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertFile =
+					mainTask: sinon.stub().yields(null, @file, @isNewFile)
+
+				@ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'creates any necessary folders', ->
+				@ProjectEntityUpdateHandler.mkdirp.withoutLock
+					.calledWith(project_id, '/folder')
+					.should.equal true
 
-		it 'upserts the file', ->
-			@ProjectEntityUpdateHandler.upsertFile.mainTask
-				.calledWith(project_id, @folder._id, 'file.png', @fileSystemPath, @linkedFileData, userId)
-				.should.equal true
+			it 'upserts the file', ->
+				@ProjectEntityUpdateHandler.upsertFile.mainTask
+					.calledWith(project_id, @folder._id, 'file.png', @fileSystemPath, @linkedFileData, userId)
+					.should.equal true
 
-		it 'calls the callback', ->
-			@callback
-				.calledWith(null, @file, @isNewFile, undefined, @newFolders, @folder)
-				.should.equal true
+			it 'calls the callback', ->
+				@callback
+					.calledWith(null, @file, @isNewFile, undefined, @newFolders, @folder)
+					.should.equal true
+
+		describe 'upserting a file with an invalid path', ->
+			beforeEach ->
+				@path = "/*folder/file.png"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@file = _id: file_id
+				@isNewFile = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertFile =
+					mainTask: sinon.stub().yields(null, @file, @isNewFile)
+
+				@ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
+
+		describe 'upserting a file with an invalid name', ->
+			beforeEach ->
+				@path = "/folder/*file.png"
+				@newFolders = [ 'mock-a', 'mock-b' ]
+				@folder = _id: folder_id
+				@file = _id: file_id
+				@isNewFile = true
+				@ProjectEntityUpdateHandler.mkdirp =
+					withoutLock: sinon.stub().yields(null, @newFolders, @folder)
+				@ProjectEntityUpdateHandler.upsertFile =
+					mainTask: sinon.stub().yields(null, @file, @isNewFile)
+
+				@ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'deleteEntity', ->
 		beforeEach ->
@@ -721,16 +890,29 @@ describe 'ProjectEntityUpdateHandler', ->
 				.should.equal true
 
 	describe 'addFolder', ->
-		beforeEach ->
-			@parentFolder_id = '123asdf'
-			@folderName = 'new-folder'
-			@ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields()
-			@ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback
+		describe 'adding a folder', ->
+			beforeEach ->
+				@parentFolder_id = '123asdf'
+				@folderName = 'new-folder'
+				@ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields()
+				@ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback
+
+			it 'calls ProjectEntityMongoUpdateHandler', ->
+				@ProjectEntityMongoUpdateHandler.addFolder
+					.calledWith(project_id, @parentFolder_id, @folderName)
+					.should.equal true
 
-		it 'calls ProjectEntityMongoUpdateHandler', ->
-			@ProjectEntityMongoUpdateHandler.addFolder
-				.calledWith(project_id, @parentFolder_id, @folderName)
-				.should.equal true
+		describe 'adding a folder with an invalid name', ->
+			beforeEach ->
+				@parentFolder_id = '123asdf'
+				@folderName = '*new-folder'
+				@ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields()
+				@ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe 'moveEntity', ->
 		beforeEach ->
@@ -763,35 +945,57 @@ describe 'ProjectEntityUpdateHandler', ->
 				.should.equal true
 
 	describe "renameEntity", ->
-		beforeEach ->
-			@project_name = 'project name'
-			@startPath = '/folder/a.tex'
-			@endPath = '/folder/b.tex'
-			@rev = 2
-			@changes = newDocs: ['old-doc'], newFiles: ['old-file']
-			@newDocName = 'b.tex'
-			@ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields(
-				null, @project, @startPath, @endPath, @rev, @changes
-			)
-			@TpdsUpdateSender.moveEntity = sinon.stub()
-			@DocumentUpdaterHandler.updateProjectStructure = sinon.stub()
+		describe 'renaming an entity', ->
+			beforeEach ->
+				@project_name = 'project name'
+				@startPath = '/folder/a.tex'
+				@endPath = '/folder/b.tex'
+				@rev = 2
+				@changes = newDocs: ['old-doc'], newFiles: ['old-file']
+				@newDocName = 'b.tex'
+				@ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields(
+					null, @project, @startPath, @endPath, @rev, @changes
+				)
+				@TpdsUpdateSender.moveEntity = sinon.stub()
+				@DocumentUpdaterHandler.updateProjectStructure = sinon.stub()
 
-			@ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback
+				@ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback
 
-		it 'moves the entity in mongo', ->
-			@ProjectEntityMongoUpdateHandler.renameEntity
-				.calledWith(project_id, doc_id, 'doc', @newDocName)
-				.should.equal true
+			it 'moves the entity in mongo', ->
+				@ProjectEntityMongoUpdateHandler.renameEntity
+					.calledWith(project_id, doc_id, 'doc', @newDocName)
+					.should.equal true
 
-		it 'notifies tpds', ->
-			@TpdsUpdateSender.moveEntity
-				.calledWith({project_id, @project_name, @startPath, @endPath, @rev})
-				.should.equal true
+			it 'notifies tpds', ->
+				@TpdsUpdateSender.moveEntity
+					.calledWith({project_id, @project_name, @startPath, @endPath, @rev})
+					.should.equal true
 
-		it 'sends the changes in project structure to the doc updater',  ->
-			@DocumentUpdaterHandler.updateProjectStructure
-				.calledWith(project_id, projectHistoryId, userId, @changes, @callback)
-				.should.equal true
+			it 'sends the changes in project structure to the doc updater',  ->
+				@DocumentUpdaterHandler.updateProjectStructure
+					.calledWith(project_id, projectHistoryId, userId, @changes, @callback)
+					.should.equal true
+
+		describe 'renaming an entity to an invalid name', ->
+			beforeEach ->
+				@project_name = 'project name'
+				@startPath = '/folder/a.tex'
+				@endPath = '/folder/b.tex'
+				@rev = 2
+				@changes = newDocs: ['old-doc'], newFiles: ['old-file']
+				@newDocName = '*b.tex'
+				@ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields(
+					null, @project, @startPath, @endPath, @rev, @changes
+				)
+				@TpdsUpdateSender.moveEntity = sinon.stub()
+				@DocumentUpdaterHandler.updateProjectStructure = sinon.stub()
+
+				@ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback
+
+			it 'returns an error', ->
+				errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError)
+				@callback.calledWithMatch(errorMatcher)
+					.should.equal true
 
 	describe "resyncProjectHistory", ->
 		describe "a deleted project", ->
@@ -998,5 +1202,3 @@ describe 'ProjectEntityUpdateHandler', ->
 
 			it "should call the callback", ->
 				@callback.called.should.equal true
-
-	

services/web/test/unit/coffee/Project/SafePathTests.coffee

@@ -83,6 +83,59 @@ describe 'SafePath', ->
 			result = @SafePath.isCleanFilename 'foo\\bar'
 			result.should.equal false
 
+	describe 'isCleanPath', ->
+		it 'should accept a valid filename "main.tex"', ->
+			result = @SafePath.isCleanPath 'main.tex'
+			result.should.equal true
+
+		it 'should accept a valid path "foo/main.tex"', ->
+			result = @SafePath.isCleanPath 'foo/main.tex'
+			result.should.equal true
+
+		it 'should accept empty path elements', ->
+			result = @SafePath.isCleanPath 'foo//main.tex'
+			result.should.equal true
+
+		it 'should not accept an empty filename', ->
+			result = @SafePath.isCleanPath 'foo/bar/'
+			result.should.equal false
+
+		it 'should accept a path that starts with a slash', ->
+			result = @SafePath.isCleanPath '/etc/passwd'
+			result.should.equal true
+
+		it 'should not accept a path that has an asterisk as the 0th element', ->
+			result = @SafePath.isCleanPath '*/foo/bar'
+			result.should.equal false
+
+		it 'should not accept a path that has an asterisk as a middle element', ->
+			result = @SafePath.isCleanPath 'foo/*/bar'
+			result.should.equal false
+
+		it 'should not accept a path that has an asterisk as the filename', ->
+			result = @SafePath.isCleanPath 'foo/bar/*'
+			result.should.equal false
+
+		it 'should not accept a path that contains an asterisk in the 0th element', ->
+			result = @SafePath.isCleanPath 'f*o/bar/baz'
+			result.should.equal false
+
+		it 'should not accept a path that contains an asterisk in a middle element', ->
+			result = @SafePath.isCleanPath 'foo/b*r/baz'
+			result.should.equal false
+
+		it 'should not accept a path that contains an asterisk in the filename', ->
+			result = @SafePath.isCleanPath 'foo/bar/b*z'
+			result.should.equal false
+
+		it 'should not accept multiple problematic elements', ->
+			result = @SafePath.isCleanPath 'f*o/b*r/b*z'
+			result.should.equal false
+
+		it 'should not accept a problematic path with an empty element', ->
+			result = @SafePath.isCleanPath 'foo//*/bar'
+			result.should.equal false
+
 	describe 'isAllowedLength', ->
 		it 'should accept a valid path "main.tex"', ->
 			result = @SafePath.isAllowedLength 'main.tex'
@@ -96,7 +149,7 @@ describe 'SafePath', ->
 		it 'should not accept an empty path', ->
 			result = @SafePath.isAllowedLength ''
 			result.should.equal false
-	
+
 	describe 'clean', ->
 		it 'should not modify a valid filename', ->
 			result = @SafePath.clean 'main.tex'
@@ -105,7 +158,7 @@ describe 'SafePath', ->
 		it 'should replace invalid characters with _', ->
 			result = @SafePath.clean 'foo/bar*/main.tex'
 			result.should.equal 'foo_bar__main.tex'
-			
+
 		it 'should replace "." with "_"', ->
 			result = @SafePath.clean '.'
 			result.should.equal '_'
@@ -133,7 +186,7 @@ describe 'SafePath', ->
 		it 'should prefix javascript property names with @', ->
 			result = @SafePath.clean 'prototype'
 			result.should.equal '@prototype'
-		
+
 		it 'should prefix javascript property names in the prototype with @', ->
 			result = @SafePath.clean 'hasOwnProperty'
 			result.should.equal '@hasOwnProperty'