filesystem: Add logic for handling (non-upwards) symlinks

8 files changed, 150 insertions, 25 deletions

diff --git a/test/buildtool/file_system/create_fs_test_git_bundle_symlinks.sh b/test/buildtool/file_system/create_fs_test_git_bundle_symlinks.sh
index dbe027c9..0cfd0cca 100644
--- a/test/buildtool/file_system/create_fs_test_git_bundle_symlinks.sh
+++ b/test/buildtool/file_system/create_fs_test_git_bundle_symlinks.sh
@@ -18,25 +18,27 @@ set -e
 # ---
 # Structure of test_repo_symlinks:
 # ---
-# <root>   <--kTreeSymId: e00aa80fd1600090930c7ec0b7146028693074bf (tree)
+# <root>   <--kTreeSymId: 18770dacfe14c15d88450c21c16668e13ab0e7f9 (tree)
 # |
 # +--bar     <--kBarId: same as in test_repo.bundle (blob)
 # +--foo     <--kFooId: same as in test_repo.bundle (blob)
 # +--baz_l   <-- kBazLinkId: 3f9538666251333f5fa519e01eb267d371ca9c78 (blob)
-# +--baz     <--kBazSymId: 9df184c5a0b324f488176432724deae97adaace8 (tree)
+# +--foo_l   <-- kFooLinkId: b24736f10d3c60015386047ebc98b4ab63056041 (blob)
+# +--baz     <--kBazSymId: 1868f82682c290f0b1db3cacd092727eef1fa57f (tree)
 # |  +--bar
 # |  +--foo
 # |  +--bar_l   <-- kBazBarLinkId: 79264abecd108745abb4086427ac988c7df7b639 (blob)
-# |  +--foo_l   <-- kBazFooLinkId: 7013e0db1095e8276a6e249830d999aecb7abd3d (blob)
 # |
 # ---
 #
+# kRootCoomit: 3ecce3f5b19ad7941c6354d65d841590662f33ef (commit)
+#
 # foo is a regular file
 # bar is an executable
 #
-# foo_l is a symlink to ../foo
-# bar_l is a symlink to ../bar
-# baz_l is a symlink to ./baz
+# foo_l is a symlink to "baz/foo"
+# bar_l is a symlink to "bar" (i.e., baz/bar)
+# baz_l is a symlink to "baz"
 #
 # Bundle name: test_repo_symlinks.bundle
 #
@@ -51,8 +53,8 @@ chmod +x bar
 mkdir -p baz/
 cp foo baz/foo
 cp bar baz/bar
-ln -s ../foo baz/foo_l
-ln -s ../bar baz/bar_l
+ln -s baz/foo foo_l
+ln -s bar baz/bar_l
 ln -s baz baz_l
 
 # create the repo
@@ -70,4 +72,4 @@ git bundle create ../data/test_repo_symlinks.bundle HEAD master
 # unlink the symlinks ourselves, to avoid broken symlinks issues on cleanup
 unlink baz_l
 unlink baz/bar_l
-unlink baz/foo_l
+unlink foo_l
diff --git a/test/buildtool/file_system/file_root.test.cpp b/test/buildtool/file_system/file_root.test.cpp
index a90e92ea..e6b4a930 100644
--- a/test/buildtool/file_system/file_root.test.cpp
+++ b/test/buildtool/file_system/file_root.test.cpp
@@ -31,7 +31,7 @@ auto const kBarId = std::string{"ba0e162e1c47469e3fe4b393a8bf8c569f302116"};
 
 auto const kBundleSymPath =
     std::string{"test/buildtool/file_system/data/test_repo_symlinks.bundle"};
-auto const kTreeSymId = std::string{"e00aa80fd1600090930c7ec0b7146028693074bf"};
+auto const kTreeSymId = std::string{"18770dacfe14c15d88450c21c16668e13ab0e7f9"};
 
 [[nodiscard]] auto GetTestDir() -> std::filesystem::path {
     auto* tmp_dir = std::getenv("TEST_TMPDIR");
@@ -105,7 +105,7 @@ void TestFileRootReadFile(FileRoot const& root) {
 
     // Check symlinks are missing
     CHECK_FALSE(root.Exists("baz_l"));
-    CHECK_FALSE(root.Exists("baz/foo_l"));
+    CHECK_FALSE(root.Exists("foo_l"));
     CHECK_FALSE(root.Exists("baz/bar_l"));
 }
 
diff --git a/test/buildtool/file_system/file_system_manager.test.cpp b/test/buildtool/file_system/file_system_manager.test.cpp
index 4b48308e..b4a22035 100644
--- a/test/buildtool/file_system/file_system_manager.test.cpp
+++ b/test/buildtool/file_system/file_system_manager.test.cpp
@@ -18,6 +18,7 @@
 #include <filesystem>
 #include <fstream>
 #include <iostream>
+#include <unordered_map>
 
 #include "catch2/catch_test_macros.hpp"
 #include "catch2/generators/catch_generators_all.hpp"
@@ -62,6 +63,104 @@ class WriteFileFixture {
                                            "file"};
 };
 
+class SymlinkTestsFixture {
+  public:
+    SymlinkTestsFixture() noexcept {
+        REQUIRE(FileSystemManager::CreateDirectory(root_dir_));
+        create_files();
+        create_symlinks();
+    }
+    SymlinkTestsFixture(SymlinkTestsFixture const&) = delete;
+    SymlinkTestsFixture(SymlinkTestsFixture&&) = delete;
+    ~SymlinkTestsFixture() noexcept {
+        CHECK(std::filesystem::remove_all(root_dir_));
+    }
+    auto operator=(SymlinkTestsFixture const&) -> SymlinkTestsFixture& = delete;
+    auto operator=(SymlinkTestsFixture&&) -> SymlinkTestsFixture& = delete;
+
+    std::filesystem::path const root_dir_{"./tmp-Symlinks"};
+
+    using filetree_t = std::unordered_map<std::string, ObjectType>;
+    filetree_t const kExpected = {{"foo", ObjectType::File},
+                                  {"baz", ObjectType::Tree},
+                                  {"baz/foo", ObjectType::File}};
+
+    struct LinkInfo {
+        std::string to;
+        std::string link;
+        bool resolvesToExisting;
+        bool isNonUpwards;
+    };
+    std::vector<LinkInfo> const kSymExpected = {
+        {.to = "baz",
+         .link = "baz_l",
+         .resolvesToExisting = true,
+         .isNonUpwards = true},
+        {.to = "../foo",
+         .link = "baz/foo_l",
+         .resolvesToExisting = true,
+         .isNonUpwards = false},
+        {.to = "baz/foo_l",
+         .link = "bar_l",
+         .resolvesToExisting = true,
+         .isNonUpwards = true},
+        {.to = "does_not_exist",
+         .link = "baz/non_existing_l",
+         .resolvesToExisting = false,
+         .isNonUpwards = true},
+        {.to = "non_existing_l",
+         .link = "baz/non_existing_indirect_l",
+         .resolvesToExisting = false,
+         .isNonUpwards = true},
+        {.to = "baz/../../does_not_exist",
+         .link = "non_existing_sneaky_l",
+         .resolvesToExisting = false,
+         .isNonUpwards = false}};
+
+    void create_files() {
+        for (auto const& [path, type] : kExpected) {
+            switch (type) {
+                case ObjectType::File: {
+                    if (not FileSystemManager::WriteFile("",
+                                                         root_dir_ / path)) {
+                        Logger::Log(LogLevel::Error,
+                                    "Could not create test file at path {}",
+                                    (root_dir_ / path).string());
+                        std::exit(1);
+                    };
+                } break;
+                case ObjectType::Tree: {
+                    if (not FileSystemManager::CreateDirectory(root_dir_ /
+                                                               path)) {
+                        Logger::Log(LogLevel::Error,
+                                    "Could not create test dir at path {}",
+                                    (root_dir_ / path).string());
+                        std::exit(1);
+                    };
+                } break;
+                default: {
+                    Logger::Log(LogLevel::Error,
+                                "File system failure in creating test dir");
+                    std::exit(1);
+                }
+            }
+        }
+    }
+
+    void create_symlinks() {
+        for (auto const& link_info : kSymExpected) {
+            if (not FileSystemManager::CreateSymlink(
+                    link_info.to, root_dir_ / link_info.link)) {
+                Logger::Log(
+                    LogLevel::Error,
+                    "File system failure in creating symlink at path {}",
+                    (root_dir_ / link_info.link).string());
+                std::exit(1);
+            }
+        }
+    }
+};
+
 namespace {
 
 namespace fs = std::filesystem;
@@ -677,3 +776,20 @@ TEST_CASE_METHOD(CopyFileFixture, "CreateFileHardlinkAs", "[file_system]") {
         }
     }
 }
+
+TEST_CASE_METHOD(SymlinkTestsFixture, "Symlinks", "[file_system]") {
+    auto i = GENERATE(range(0U, 6U /* kSymExpected.size() */));
+
+    SECTION(fmt::format("Non-upwards symlinks - entry {}", i)) {
+        CHECK(FileSystemManager::IsNonUpwardsSymlink(root_dir_ /
+                                                     kSymExpected[i].link) ==
+              kSymExpected[i].isNonUpwards);
+    }
+
+    SECTION(fmt::format("Resolve symlinks - entry {}", i)) {
+        auto path = root_dir_ / kSymExpected[i].link;
+        REQUIRE(FileSystemManager::ResolveSymlinks(&path));
+        CHECK(FileSystemManager::Exists(path) ==
+              kSymExpected[i].resolvesToExisting);
+    }
+}
diff --git a/test/buildtool/file_system/git_repo.test.cpp b/test/buildtool/file_system/git_repo.test.cpp
index d2c0f1c3..69ab7e6c 100644
--- a/test/buildtool/file_system/git_repo.test.cpp
+++ b/test/buildtool/file_system/git_repo.test.cpp
@@ -26,9 +26,9 @@ namespace {
 auto const kBundlePath =
     std::string{"test/buildtool/file_system/data/test_repo_symlinks.bundle"};
 auto const kRootCommit =
-    std::string{"3a8dc005262e2ea32d48dbe123e1dabe20e039c1"};
-auto const kRootId = std::string{"e00aa80fd1600090930c7ec0b7146028693074bf"};
-auto const kBazId = std::string{"9df184c5a0b324f488176432724deae97adaace8"};
+    std::string{"3ecce3f5b19ad7941c6354d65d841590662f33ef"};
+auto const kRootId = std::string{"18770dacfe14c15d88450c21c16668e13ab0e7f9"};
+auto const kBazId = std::string{"1868f82682c290f0b1db3cacd092727eef1fa57f"};
 
 }  // namespace
 
diff --git a/test/buildtool/file_system/git_tree.test.cpp b/test/buildtool/file_system/git_tree.test.cpp
index 79654ad5..79eb11f4 100644
--- a/test/buildtool/file_system/git_tree.test.cpp
+++ b/test/buildtool/file_system/git_tree.test.cpp
@@ -31,12 +31,11 @@ auto const kFailId = std::string{"0123456789abcdef0123456789abcdef01234567"};
 
 auto const kBundleSymPath =
     std::string{"test/buildtool/file_system/data/test_repo_symlinks.bundle"};
-auto const kTreeSymId = std::string{"e00aa80fd1600090930c7ec0b7146028693074bf"};
+auto const kTreeSymId = std::string{"18770dacfe14c15d88450c21c16668e13ab0e7f9"};
 auto const kBazLinkId = std::string{"3f9538666251333f5fa519e01eb267d371ca9c78"};
 auto const kBazBarLinkId =
-    std::string{"79264abecd108745abb4086427ac988c7df7b639"};
-auto const kBazFooLinkId =
-    std::string{"7013e0db1095e8276a6e249830d999aecb7abd3d"};
+    std::string{"ba0e162e1c47469e3fe4b393a8bf8c569f302116"};
+auto const kFooLinkId = std::string{"b24736f10d3c60015386047ebc98b4ab63056041"};
 
 [[nodiscard]] auto HexToRaw(std::string const& hex) -> std::string {
     return FromHexString(hex).value_or<std::string>({});
@@ -128,8 +127,8 @@ TEST_CASE("Read Git Objects", "[git_cas]") {
         CHECK(cas->ReadObject(kBazBarLinkId, /*is_hex_id=*/true));
         CHECK(cas->ReadObject(HexToRaw(kBazBarLinkId), /*is_hex_id=*/false));
 
-        CHECK(cas->ReadObject(kBazFooLinkId, /*is_hex_id=*/true));
-        CHECK(cas->ReadObject(HexToRaw(kBazFooLinkId), /*is_hex_id=*/false));
+        CHECK(cas->ReadObject(kFooLinkId, /*is_hex_id=*/true));
+        CHECK(cas->ReadObject(HexToRaw(kFooLinkId), /*is_hex_id=*/false));
     }
 
     SECTION("invalid ids") {
@@ -168,8 +167,8 @@ TEST_CASE("Read Git Headers", "[git_cas]") {
         CHECK(cas->ReadHeader(kBazBarLinkId, /*is_hex_id=*/true));
         CHECK(cas->ReadHeader(HexToRaw(kBazBarLinkId), /*is_hex_id=*/false));
 
-        CHECK(cas->ReadHeader(kBazFooLinkId, /*is_hex_id=*/true));
-        CHECK(cas->ReadHeader(HexToRaw(kBazFooLinkId), /*is_hex_id=*/false));
+        CHECK(cas->ReadHeader(kFooLinkId, /*is_hex_id=*/true));
+        CHECK(cas->ReadHeader(HexToRaw(kFooLinkId), /*is_hex_id=*/false));
     }
 
     SECTION("invalid ids") {
diff --git a/test/other_tools/git_operations/critical_git_ops.test.cpp b/test/other_tools/git_operations/critical_git_ops.test.cpp
index 6a78e577..cfd651c4 100644
--- a/test/other_tools/git_operations/critical_git_ops.test.cpp
+++ b/test/other_tools/git_operations/critical_git_ops.test.cpp
@@ -26,7 +26,7 @@ namespace {
 auto const kBundlePath =
     std::string{"test/buildtool/file_system/data/test_repo_symlinks.bundle"};
 auto const kRootCommit =
-    std::string{"3a8dc005262e2ea32d48dbe123e1dabe20e039c1"};
+    std::string{"3ecce3f5b19ad7941c6354d65d841590662f33ef"};
 
 }  // namespace
 
diff --git a/test/other_tools/git_operations/git_repo_remote.test.cpp b/test/other_tools/git_operations/git_repo_remote.test.cpp
index 03fffd3d..37be3737 100644
--- a/test/other_tools/git_operations/git_repo_remote.test.cpp
+++ b/test/other_tools/git_operations/git_repo_remote.test.cpp
@@ -26,8 +26,8 @@ namespace {
 auto const kBundlePath =
     std::string{"test/buildtool/file_system/data/test_repo_symlinks.bundle"};
 auto const kRootCommit =
-    std::string{"3a8dc005262e2ea32d48dbe123e1dabe20e039c1"};
-auto const kRootId = std::string{"e00aa80fd1600090930c7ec0b7146028693074bf"};
+    std::string{"3ecce3f5b19ad7941c6354d65d841590662f33ef"};
+auto const kRootId = std::string{"18770dacfe14c15d88450c21c16668e13ab0e7f9"};
 
 }  // namespace
 
diff --git a/test/utils/cpp/path.test.cpp b/test/utils/cpp/path.test.cpp
index d6d4eb85..5a450fd5 100644
--- a/test/utils/cpp/path.test.cpp
+++ b/test/utils/cpp/path.test.cpp
@@ -32,3 +32,11 @@ TEST_CASE("normalization", "[path]") {
     CHECK(ToNormalPath(std::filesystem::path{"foo/.."}).string() == ".");
     CHECK(ToNormalPath(std::filesystem::path{"./foo/.."}).string() == ".");
 }
+
+TEST_CASE("non-upwards condition", "[path]") {
+    CHECK_FALSE(PathIsNonUpwards("/foo"));    // absolute path
+    CHECK(PathIsNonUpwards("foo"));           // relative non-upwards
+    CHECK_FALSE(PathIsNonUpwards("../foo"));  // relative not non-upwards
+    CHECK_FALSE(PathIsNonUpwards(
+        "foo/../bar/../../foo"));  // relative with non-upwards indirection
+}