Modifying copy_dir to ignore symlinks when copying sandbox data (fixes #31).

Author: Martin Krulis

CMakeLists.txt

@@ -95,12 +95,12 @@ else()
 	include_directories(AFTER, vendor/libarchive/libarchive)
 endif()
 
-# Use C++11
+# Use C++17
 if(UNIX)
 	#-Wno-deprecated-declarations hides warning in yaml-cpp (using std::auto_ptr)
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall -Wno-deprecated-declarations")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -Wall -Wno-deprecated-declarations")
 elseif(MSVC)
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc /MP /D NOMINMAX")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /std:c++17 /EHsc /MP /D NOMINMAX")
 endif()
 
 

appveyor.yml

@@ -81,7 +81,7 @@ before_build:
 
     echo Running cmake...
 
-    cmake -G "Visual Studio 14 2015" -DBOOST_ROOT=C:\deps\boost -DCURL_LIBRARY=C:\deps\curl\lib\libcurl.lib -DCURL_INCLUDE_DIR=C:\deps\curl\include -DZMQ_LIBRARY_DIR=C:\deps\libzmq\lib -DZMQ_INCLUDE_DIR=C:\deps\libzmq\include .
+    cmake -G "Visual Studio 15 2017" -DBOOST_ROOT=C:\deps\boost -DCURL_LIBRARY=C:\deps\curl\lib\libcurl.lib -DCURL_INCLUDE_DIR=C:\deps\curl\include -DZMQ_LIBRARY_DIR=C:\deps\libzmq\lib -DZMQ_INCLUDE_DIR=C:\deps\libzmq\include .
 build:
   project: ./ALL_BUILD.vcxproj
   parallel: true

recodex-worker.spec

@@ -1,8 +1,8 @@
 %define name recodex-worker
 %define short_name worker
 %define version 1.7.1
-%define unmangled_version 45f6b839084c1793a5092f9d035c482ea259fa80
-%define release 1
+%define unmangled_version 483a474e5c2098588958e7c7e48eff4f25516aca
+%define release 2
 
 %define spdlog_name spdlog
 %define spdlog_version 0.13.0

src/helpers/filesystem.cpp

@@ -1,35 +1,97 @@
 #include "filesystem.h"
 #include <iostream>
+#include <map>
 
-void helpers::copy_directory(const fs::path &src, const fs::path &dest)
+/**
+ * Try to find matching hardlink in hardlinks map. If src is found in the map, dest is filled with corresponding file.
+ * @param hardlinks the hardlinks map (src -> dst)
+ * @param src source path being looked up in hardlinks using equvalent func
+ * @param dest output arg which is filled in case of success
+ * @return true if the hardlink match is found
+ */
+bool find_matching_hadlink(std::map<fs::path, fs::path> &hardlinks, const fs::path &src, fs::path &dest)
+{
+	for (auto& [s, d] : hardlinks) {
+		if (fs::equivalent(s, src)) { // both paths point to the same data on the disk (same hardlinks)
+			dest = d;
+			return true;
+		}
+	}
+	return false;
+}
+
+void copy_diretory_internal(const fs::path &src, const fs::path &dest, bool skip_symlinks, std::map<fs::path, fs::path> &hardlinks)
 {
 	try {
+		// routine checks
 		if (!fs::exists(src)) {
-			throw filesystem_exception(
+			throw helpers::filesystem_exception(
 				"helpers::copy_directory: Source directory does not exist '" + src.string() + "'");
-		} else if (!fs::is_directory(fs::symlink_status(src))) {
-			throw filesystem_exception(
+		}
+
+		if (skip_symlinks && fs::is_symlink(src)) {
+			return;
+		}
+		
+		if (!fs::is_directory(fs::symlink_status(src))) {
+			throw helpers::filesystem_exception(
 				"helpers::copy_directory: Source directory is not a directory '" + src.string() + "'");
-		} else if (!fs::exists(dest) && !fs::create_directories(dest)) {
-			throw filesystem_exception(
+		}
+		
+		if (!fs::exists(dest) && !fs::create_directories(dest)) {
+			throw helpers::filesystem_exception(
 				"helpers::copy_directory: Destination directory cannot be created '" + dest.string() + "'");
 		}
 
+		// proceed with copying
 		fs::directory_iterator endit;
 		for (fs::directory_iterator it(src); it != endit; ++it) {
+			auto srcPath = it->path();
+			auto destPath = dest / it->path().filename();
+
+			if (skip_symlinks && fs::is_symlink(srcPath)) {
+				continue;
+			}
+
 			if (fs::is_directory(it->symlink_status())) {
-				helpers::copy_directory(it->path(), dest / it->path().filename());
+				copy_diretory_internal(srcPath, destPath, skip_symlinks, hardlinks);
 			} else {
-				fs::copy(it->path(), dest / it->path().filename());
+				// a file may be either copied or hardlinked
+				if (!fs::is_symlink(srcPath) && fs::hard_link_count(srcPath) > 1) {
+					fs::path destPathHardlink;
+					if (find_matching_hadlink(hardlinks, it->path(), destPathHardlink)) {
+						// another file refering to the same data already exists in dest directory
+						fs::create_hard_link(destPathHardlink, destPath);
+						continue; // move to next file, hardlink replaced copying
+					} else {
+						// this is the first time we encoutered this data, lets register them in hardlinks map
+						hardlinks.emplace(std::make_pair(it->path(), destPath));
+					}
+				}
+
+				// no hardlink created, lets proceed with copying
+				fs::copy(it->path(), destPath);
 			}
 		}
 	} catch (fs::filesystem_error &e) {
-		throw filesystem_exception("helpers::copy_directory: Error in copying directories: " + std::string(e.what()));
+		throw helpers::filesystem_exception("helpers::copy_directory: Error in copying directories: " + std::string(e.what()));
 	}
 
 	return;
 }
 
+void helpers::copy_directory(const fs::path &src, const fs::path &dest, bool skip_symlinks)
+{
+	/*
+	 * Hardlinks map provide mapping between files in src and dest which have been harlinked.
+	 * Everytime a file with > 1 hadlink count is copied from src to dest, it is registered here.
+	 * When files with > 1 hardlinks are encountered, this map is searched and if match is found
+	 * the new file is hadlinked inside dest instead of coping it from src.
+	 */
+	std::map<fs::path, fs::path> hardlinks;
+	::copy_diretory_internal(src, dest, skip_symlinks, hardlinks);
+}
+
 fs::path helpers::normalize_path(const fs::path &path)
 {
 	// prepare root and path chunks

src/helpers/filesystem.h

@@ -14,9 +14,11 @@ namespace helpers
 	 * Recursively copy directory from source to destination.
 	 * @param src source directory which content will be copied into @a dest
 	 * @param dest destination path which should not exist
+	 * @param skip_symlinks if true, all symlinks in src will be ignored
+	 *                      (security feature when dealing with sandboxed data)
 	 * @throws filesystem_exception with approprite description
 	 */
-	void copy_directory(const fs::path &src, const fs::path &dest);
+	void copy_directory(const fs::path &src, const fs::path &dest, bool skip_symlinks = false);
 
 	/**
 	 * Normalize dots and double dots from given path.

src/sandbox/isolate_sandbox.cpp

@@ -25,7 +25,7 @@ namespace
 	void move_or_throw(std::shared_ptr<spdlog::logger> logger, const std::string &from, const std::string &to)
 	{
 		try {
-			helpers::copy_directory(from, to);
+			helpers::copy_directory(from, to, true); // true = skip symlinks for security reasons
 		} catch (fs::filesystem_error &e) {
 			log_and_throw(logger, "Failed moving ", from, " to ", to, ", error: ", e.what());
 		}