remote execution: Check validity of symlinks

Invalid entries, currently all upwards symlinks (pending
implementation of a better way of handling them), are now
identified and handled properly: in compatible mode on the client
side, during handling of remote response, and in native mode on
the server side during the population of the action result.

3 files changed, 60 insertions, 12 deletions

diff --git a/src/buildtool/execution_api/execution_service/TARGETS b/src/buildtool/execution_api/execution_service/TARGETS
index d8108279..7f8e0e0d 100644
--- a/src/buildtool/execution_api/execution_service/TARGETS
+++ b/src/buildtool/execution_api/execution_service/TARGETS
@@ -22,6 +22,7 @@
     , ["src/buildtool/logging", "log_level"]
     , "operation_cache"
     , ["src/utils/cpp", "hex_string"]
+    , ["src/utils/cpp", "path"]
     , ["src/buildtool/execution_api/local", "local"]
     , ["src/buildtool/common", "common"]
     , ["src/buildtool/common", "artifact_digest_factory"]
diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp
index d59eb91f..cbf61edd 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.cpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.cpp
@@ -22,12 +22,14 @@
 #include "fmt/core.h"
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/common/artifact_digest_factory.hpp"
+#include "src/buildtool/common/protocol_traits.hpp"
 #include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/execution_api/local/local_cas_reader.hpp"
 #include "src/buildtool/file_system/file_system_manager.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
 #include "src/utils/cpp/hex_string.hpp"
+#include "src/utils/cpp/path.hpp"
 
 namespace {
 void UpdateTimeStamp(
@@ -284,15 +286,21 @@ namespace {
     ::bazel_re::OutputDirectory out_dir{};
     *(out_dir.mutable_path()) = std::move(path);
 
+    LocalCasReader reader(&storage.CAS());
     if (ProtocolTraits::IsNative(storage.GetHashFunction().GetType())) {
-        // In native mode: Set the directory digest directly.
+        // In native mode: Check validity of tree entries, otherwise set the
+        // digest directly.
+        if (not reader.ReadGitTree(digest)) {
+            auto const error = fmt::format(
+                "Found invalid entry in the Git Tree {}", digest.hash());
+            return unexpected{error};
+        }
         (*out_dir.mutable_tree_digest()) =
             ArtifactDigestFactory::ToBazel(digest);
     }
     else {
         // In compatible mode: Create a tree digest from directory
         // digest on the fly and set tree digest.
-        LocalCasReader reader(&storage.CAS());
         auto const tree = reader.MakeTree(digest);
         if (not tree) {
             return unexpected{fmt::format("Failed to build bazel Tree for {}",
@@ -333,6 +341,13 @@ namespace {
             "Failed to read the symlink content for {}", digest.hash())};
     }
 
+    // in native mode, check that we do not pass invalid symlinks
+    if (ProtocolTraits::IsNative(storage.GetHashFunction().GetType()) and
+        not PathIsNonUpwards(*content)) {
+        auto const error = fmt::format("Invalid symlink for {}", digest.hash());
+        return unexpected{error};
+    }
+
     *(out_link.mutable_target()) = *std::move(content);
     return std::move(out_link);
 }
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
index e089888a..7771fa8e 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
@@ -21,6 +21,7 @@
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/common/artifact_digest_factory.hpp"
 #include "src/buildtool/common/bazel_digest_factory.hpp"
+#include "src/buildtool/common/protocol_traits.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp"
 #include "src/buildtool/execution_api/common/common_api.hpp"
@@ -33,7 +34,14 @@ namespace {
 
 auto ProcessDirectoryMessage(HashFunction hash_function,
                              bazel_re::Directory const& dir) noexcept
-    -> BazelBlob {
+    -> expected<BazelBlob, std::string> {
+    // in compatible mode: check validity of all symlinks
+    for (auto const& link : dir.symlinks()) {
+        if (not PathIsNonUpwards(link.target())) {
+            return unexpected{
+                fmt::format("found invalid symlink at {}", link.name())};
+        }
+    }
     auto data = dir.SerializeAsString();
     auto digest =
         BazelDigestFactory::HashDataAs<ObjectType::File>(hash_function, data);
@@ -123,6 +131,13 @@ auto BazelResponse::Populate() noexcept -> std::optional<std::string> {
     // collect all symlinks and store them
     for (auto const& link : action_result.output_file_symlinks()) {
         try {
+            // in compatible mode: check symlink validity
+            if (not ProtocolTraits::IsNative(
+                    network_->GetHashFunction().GetType()) and
+                not PathIsNonUpwards(link.target())) {
+                return fmt::format("BazelResponse: found invalid symlink at {}",
+                                   link.path());
+            }
             artifacts.emplace(
                 link.path(),
                 Artifact::ObjectInfo{
@@ -140,6 +155,13 @@ auto BazelResponse::Populate() noexcept -> std::optional<std::string> {
     }
     for (auto const& link : action_result.output_directory_symlinks()) {
         try {
+            // in compatible mode: check symlink validity
+            if (not ProtocolTraits::IsNative(
+                    network_->GetHashFunction().GetType()) and
+                not PathIsNonUpwards(link.target())) {
+                return fmt::format("BazelResponse: found invalid symlink at {}",
+                                   link.path());
+            }
             artifacts.emplace(
                 link.path(),
                 Artifact::ObjectInfo{
@@ -249,33 +271,43 @@ auto BazelResponse::UploadTreeMessageDirectories(
     BazelBlobContainer dir_blobs{};
 
     auto rootdir_blob = ProcessDirectoryMessage(hash_function, tree.root());
-    auto const root_digest = rootdir_blob.digest;
+    if (not rootdir_blob) {
+        return unexpected{std::move(rootdir_blob).error()};
+    }
+    auto const root_digest = rootdir_blob->digest;
     // store or upload rootdir blob, taking maximum transfer size into account
     if (not UpdateContainerAndUpload<bazel_re::Digest>(
             &dir_blobs,
-            std::move(rootdir_blob),
+            *std::move(rootdir_blob),
             /*exception_is_fatal=*/false,
             upload_callback)) {
-        return unexpected{fmt::format("failed to upload root for Tree {}",
-                                      tree.SerializeAsString())};
+        return unexpected{fmt::format(
+            "failed to upload Tree with root digest {}", root_digest.hash())};
     }
 
     for (auto const& subdir : tree.children()) {
         // store or upload blob, taking maximum transfer size into account
+        auto blob = ProcessDirectoryMessage(hash_function, subdir);
+        if (not blob) {
+            return unexpected{std::move(blob).error()};
+        }
+        auto const blob_digest = blob->digest;
         if (not UpdateContainerAndUpload<bazel_re::Digest>(
                 &dir_blobs,
-                ProcessDirectoryMessage(hash_function, subdir),
+                *std::move(blob),
                 /*exception_is_fatal=*/false,
                 upload_callback)) {
-            return unexpected{fmt::format("failed to upload subdir for Tree {}",
-                                          tree.SerializeAsString())};
+            return unexpected{
+                fmt::format("failed to upload Tree subdir with digest {}",
+                            blob_digest.hash())};
         }
     }
 
     // upload any remaining blob
     if (not std::invoke(upload_callback, std::move(dir_blobs))) {
-        return unexpected{fmt::format("failed to upload blobs for Tree {}",
-                                      tree.SerializeAsString())};
+        return unexpected{
+            fmt::format("failed to upload blobs for Tree with root digest {}",
+                        root_digest.hash())};
     }
     return ArtifactDigestFactory::FromBazel(hash_function.GetType(),
                                             root_digest)