Remove unneeded extra checks for upwards symlinks

6 files changed, 6 insertions, 67 deletions

diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
index 6c0c5ae4..68da1b87 100644
--- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
+++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp
@@ -382,23 +382,12 @@ auto BazelMsgFactory::CreateDirectoryDigestFromGitTree(
         if (not tree_content) {
             return unexpected{tree_content.error()};
         }
-        auto const check_symlinks =
-            [&read_git](std::vector<ArtifactDigest> const& ids) {
-                return std::all_of(ids.begin(),
-                                   ids.end(),
-                                   [&read_git](auto const& id) -> bool {
-                                       auto content = GetContentFromGitEntry(
-                                           read_git, id, ObjectType::Symlink);
-                                       return content and
-                                              PathIsNonUpwards(*content);
-                                   });
-            };
-
         // Git-SHA1 hashing is used for reading from git
         HashFunction const hash_function{HashFunction::Type::GitSHA1};
         // the tree digest is in native mode, so no need for rehashing content
+        auto skip_symlinks = [](auto const& /*unused*/) { return true; };
         auto const entries = GitRepo::ReadTreeData(
-            *tree_content, digest.hash(), check_symlinks, /*is_hex_id=*/true);
+            *tree_content, digest.hash(), skip_symlinks, /*is_hex_id=*/true);
         if (not entries) {
             return unexpected{fmt::format("failed reading entries of tree {}",
                                           digest.hash())};
@@ -645,11 +634,6 @@ auto BazelMsgFactory::CreateGitTreeDigestFromDirectory(
                 }
             }
             else {
-                // check validity of symlink
-                if (not PathIsNonUpwards(sym.target())) {
-                    return unexpected{fmt::format(
-                        "found non-upwards symlink {}", sym_digest.hash())};
-                }
                 // rehash symlink
                 auto const blob_digest = store_symlink(sym.target());
                 if (not blob_digest) {
diff --git a/src/buildtool/execution_api/common/TARGETS b/src/buildtool/execution_api/common/TARGETS
index b13ac8b5..31a308ad 100644
--- a/src/buildtool/execution_api/common/TARGETS
+++ b/src/buildtool/execution_api/common/TARGETS
@@ -29,7 +29,6 @@
     [ ["@", "json", "", "json"]
     , ["src/buildtool/logging", "log_level"]
     , ["src/utils/cpp", "hex_string"]
-    , ["src/utils/cpp", "path"]
     ]
   , "stage": ["src", "buildtool", "execution_api", "common"]
   }
diff --git a/src/buildtool/execution_api/common/tree_reader_utils.cpp b/src/buildtool/execution_api/common/tree_reader_utils.cpp
index 2ec90cc2..4d687bc5 100644
--- a/src/buildtool/execution_api/common/tree_reader_utils.cpp
+++ b/src/buildtool/execution_api/common/tree_reader_utils.cpp
@@ -29,7 +29,6 @@
 #include "src/buildtool/logging/logger.hpp"
 #include "src/utils/cpp/expected.hpp"
 #include "src/utils/cpp/hex_string.hpp"
-#include "src/utils/cpp/path.hpp"
 
 namespace {
 [[nodiscard]] auto CreateObjectInfo(HashFunction hash_function,
@@ -104,12 +103,6 @@ auto TreeReaderUtils::ReadObjectInfos(bazel_re::Directory const& dir,
         }
 
         for (auto const& l : dir.symlinks()) {
-            // check validity of symlinks
-            if (not PathIsNonUpwards(l.target())) {
-                Logger::Log(
-                    LogLevel::Error, "found invalid symlink at {}", l.name());
-                return false;
-            }
             if (not store_info(l.name(), CreateObjectInfo(hash_function, l))) {
                 return false;
             }
diff --git a/src/buildtool/storage/local_cas.tpp b/src/buildtool/storage/local_cas.tpp
index 7cc6bea0..91edb1f9 100644
--- a/src/buildtool/storage/local_cas.tpp
+++ b/src/buildtool/storage/local_cas.tpp
@@ -120,31 +120,10 @@ auto LocalCAS<kDoGlobalUplink>::LocalUplinkGitTree(
 
     // Determine tree entries.
     auto content = FileSystemManager::ReadFile(*tree_path);
-    auto check_symlinks =
-        [this](std::vector<ArtifactDigest> const& ids) -> bool {
-        for (auto const& id : ids) {
-            auto link_path = cas_file_.BlobPath(id);
-            TmpFile::Ptr spliced;
-            if (not link_path) {
-                spliced = TrySplice<ObjectType::File>(id);
-                link_path = spliced != nullptr
-                                ? std::optional{spliced->GetPath()}
-                                : std::nullopt;
-            }
-            if (not link_path) {
-                return false;
-            }
-            // in the local CAS we store as files
-            auto content = FileSystemManager::ReadFile(*link_path);
-            if (not content or not PathIsNonUpwards(*content)) {
-                return false;
-            }
-        }
-        return true;
-    };
+    auto skip_symlinks = [](auto const& /*unused*/) { return true; };
     auto tree_entries = GitRepo::ReadTreeData(*content,
                                               digest.hash(),
-                                              check_symlinks,
+                                              skip_symlinks,
                                               /*is_hex_id=*/true);
     if (not tree_entries) {
         return false;
diff --git a/src/buildtool/tree_structure/TARGETS b/src/buildtool/tree_structure/TARGETS
index c103ea93..7f235188 100644
--- a/src/buildtool/tree_structure/TARGETS
+++ b/src/buildtool/tree_structure/TARGETS
@@ -47,7 +47,6 @@
     , ["src/buildtool/file_system", "git_repo"]
     , ["src/buildtool/file_system", "object_type"]
     , ["src/utils/cpp", "hex_string"]
-    , ["src/utils/cpp", "path"]
     , ["src/utils/cpp", "tmp_dir"]
     ]
   }
diff --git a/src/buildtool/tree_structure/tree_structure_utils.cpp b/src/buildtool/tree_structure/tree_structure_utils.cpp
index 758e47fc..b8b9dcc3 100644
--- a/src/buildtool/tree_structure/tree_structure_utils.cpp
+++ b/src/buildtool/tree_structure/tree_structure_utils.cpp
@@ -14,7 +14,6 @@
 
 #include "src/buildtool/tree_structure/tree_structure_utils.hpp"
 
-#include <algorithm>
 #include <cstddef>
 #include <filesystem>
 #include <functional>
@@ -38,7 +37,6 @@
 #include "src/buildtool/file_system/git_repo.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 #include "src/utils/cpp/hex_string.hpp"
-#include "src/utils/cpp/path.hpp"
 #include "src/utils/cpp/tmp_dir.hpp"
 
 auto TreeStructureUtils::Compute(ArtifactDigest const& tree,
@@ -65,22 +63,9 @@ auto TreeStructureUtils::Compute(ArtifactDigest const& tree,
             fmt::format("Failed to read content of: {}", tree.hash())};
     }
 
-    auto const check_symlinks =
-        [&storage](std::vector<ArtifactDigest> const& ids) {
-            return std::all_of(
-                ids.begin(), ids.end(), [&storage](auto const& id) -> bool {
-                    auto path_to_symlink =
-                        storage.CAS().BlobPath(id, /*is_executable=*/false);
-                    if (not path_to_symlink) {
-                        return false;
-                    }
-                    auto const content =
-                        FileSystemManager::ReadFile(*path_to_symlink);
-                    return content and PathIsNonUpwards(*content);
-                });
-        };
+    auto skip_symlinks = [](auto const& /*unused*/) { return true; };
     auto const entries = GitRepo::ReadTreeData(
-        *tree_content, tree.hash(), check_symlinks, /*is_hex_id=*/true);
+        *tree_content, tree.hash(), skip_symlinks, /*is_hex_id=*/true);
     if (not entries) {
         return unexpected{
             fmt::format("Failed to parse git tree: {}", tree.hash())};