just-mr: Fix missing ExpressionPtr type checks in parsing the config file

8 files changed, 156 insertions, 79 deletions

diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp
index 853cd233..a51307da 100644
--- a/src/other_tools/just_mr/fetch.cpp
+++ b/src/other_tools/just_mr/fetch.cpp
@@ -67,6 +67,12 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                     "Config: Mandatory key \"repositories\" missing");
         return kExitFetchError;
     }
+    if (not repos->IsMap()) {
+        Logger::Log(LogLevel::Error,
+                    "Config: Value for key \"repositories\" is not a map");
+        return kExitFetchError;
+    }
+
     auto fetch_repos =
         std::make_shared<JustMR::SetupRepos>();  // repos to setup and include
     JustMR::Utils::DefaultReachableRepositories(repos, fetch_repos);
@@ -139,6 +145,12 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                         nlohmann::json(repo_name).dump());
             return kExitFetchError;
         }
+        if (not repo_desc->get()->IsMap()) {
+            Logger::Log(LogLevel::Error,
+                        "Config: Config entry for repository {} is not a map",
+                        nlohmann::json(repo_name).dump());
+            return kExitFetchError;
+        }
         auto repo = repo_desc->get()->At("repository");
         if (repo) {
             auto resolved_repo_desc =
@@ -149,6 +161,13 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                             nlohmann::json(repo_name).dump());
                 return kExitFetchError;
             }
+            if (not resolved_repo_desc.value()->IsMap()) {
+                Logger::Log(
+                    LogLevel::Error,
+                    "Config: Repository {} resolves to a non-map description",
+                    nlohmann::json(repo_name).dump());
+                return kExitFetchError;
+            }
             // get repo_type
             auto repo_type = (*resolved_repo_desc)->At("type");
             if (not repo_type) {
@@ -180,8 +199,9 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                     // check "absent" pragma
                     auto repo_desc_pragma = (*resolved_repo_desc)->At("pragma");
                     auto pragma_absent =
-                        repo_desc_pragma ? repo_desc_pragma->get()->At("absent")
-                                         : std::nullopt;
+                        (repo_desc_pragma and repo_desc_pragma->get()->IsMap())
+                            ? repo_desc_pragma->get()->At("absent")
+                            : std::nullopt;
                     auto pragma_absent_value =
                         pragma_absent and pragma_absent->get()->IsBool() and
                         pragma_absent->get()->Bool();
@@ -296,8 +316,9 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                     // check "absent" pragma
                     auto repo_desc_pragma = (*resolved_repo_desc)->At("pragma");
                     auto pragma_absent =
-                        repo_desc_pragma ? repo_desc_pragma->get()->At("absent")
-                                         : std::nullopt;
+                        (repo_desc_pragma and repo_desc_pragma->get()->IsMap())
+                            ? repo_desc_pragma->get()->At("absent")
+                            : std::nullopt;
                     auto pragma_absent_value =
                         pragma_absent and pragma_absent->get()->IsBool() and
                         pragma_absent->get()->Bool();
diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp
index d582c5ff..e4d48086 100644
--- a/src/other_tools/just_mr/setup.cpp
+++ b/src/other_tools/just_mr/setup.cpp
@@ -55,6 +55,17 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config,
         common_args.just_mr_paths->setup_root);
 
     auto repos = (*config)["repositories"];
+    if (not repos.IsNotNull()) {
+        Logger::Log(LogLevel::Error,
+                    "Config: Mandatory key \"repositories\" missing");
+        return std::nullopt;
+    }
+    if (not repos->IsMap()) {
+        Logger::Log(LogLevel::Error,
+                    "Config: Value for key \"repositories\" is not a map");
+        return std::nullopt;
+    }
+
     auto setup_repos =
         std::make_shared<JustMR::SetupRepos>();  // repos to setup and include
     nlohmann::json mr_config{};                  // output config to populate
diff --git a/src/other_tools/just_mr/setup_utils.cpp b/src/other_tools/just_mr/setup_utils.cpp
index 9eaa2e86..c3799b1f 100644
--- a/src/other_tools/just_mr/setup_utils.cpp
+++ b/src/other_tools/just_mr/setup_utils.cpp
@@ -79,72 +79,67 @@ void ReachableRepositories(
     std::shared_ptr<JustMR::SetupRepos> const& setup_repos) {
     // use temporary sets to avoid duplicates
     std::unordered_set<std::string> include_repos_set{};
-    if (repos->IsMap()) {
-        // traversal of bindings
-        std::function<void(std::string const&)> traverse =
-            [&](std::string const& repo_name) {
-                if (not include_repos_set.contains(repo_name)) {
-                    // if not found, add it and repeat for its bindings
-                    include_repos_set.insert(repo_name);
-                    // check bindings
-                    auto repos_repo_name =
-                        repos->Get(repo_name, Expression::none_t{});
-                    if (not repos_repo_name.IsNotNull()) {
-                        return;
-                    }
-                    auto bindings =
-                        repos_repo_name->Get("bindings", Expression::none_t{});
-                    if (bindings.IsNotNull() and bindings->IsMap()) {
-                        for (auto const& bound : bindings->Map().Values()) {
-                            if (bound.IsNotNull() and bound->IsString()) {
-                                traverse(bound->String());
-                            }
+    // traversal of bindings
+    std::function<void(std::string const&)> traverse =
+        [&](std::string const& repo_name) {
+            if (not include_repos_set.contains(repo_name)) {
+                // if not found, add it and repeat for its bindings
+                include_repos_set.insert(repo_name);
+                // check bindings
+                auto repos_repo_name =
+                    repos->Get(repo_name, Expression::none_t{});
+                if (not repos_repo_name.IsNotNull()) {
+                    return;
+                }
+                auto bindings =
+                    repos_repo_name->Get("bindings", Expression::none_t{});
+                if (bindings.IsNotNull() and bindings->IsMap()) {
+                    for (auto const& bound : bindings->Map().Values()) {
+                        if (bound.IsNotNull() and bound->IsString()) {
+                            traverse(bound->String());
                         }
                     }
                 }
-            };
-        traverse(main);  // traverse all bindings of main repository
+            }
+        };
+    traverse(main);  // traverse all bindings of main repository
 
-        // Add overlay repositories
-        std::unordered_set<std::string> setup_repos_set{include_repos_set};
-        for (auto const& repo : include_repos_set) {
-            auto repos_repo = repos->Get(repo, Expression::none_t{});
-            if (repos_repo.IsNotNull()) {
-                // copy over any present alternative root dirs
-                for (auto const& layer : kAltDirs) {
-                    auto layer_val =
-                        repos_repo->Get(layer, Expression::none_t{});
-                    if (layer_val.IsNotNull() and layer_val->IsString()) {
-                        auto repo_name = layer_val->String();
-                        setup_repos_set.insert(repo_name);
-                    }
+    // Add overlay repositories
+    std::unordered_set<std::string> setup_repos_set{include_repos_set};
+    for (auto const& repo : include_repos_set) {
+        auto repos_repo = repos->Get(repo, Expression::none_t{});
+        if (repos_repo.IsNotNull()) {
+            // copy over any present alternative root dirs
+            for (auto const& layer : kAltDirs) {
+                auto layer_val = repos_repo->Get(layer, Expression::none_t{});
+                if (layer_val.IsNotNull() and layer_val->IsString()) {
+                    auto repo_name = layer_val->String();
+                    setup_repos_set.insert(repo_name);
                 }
             }
         }
-
-        // copy to vectors
-        setup_repos->to_setup.clear();
-        setup_repos->to_setup.reserve(setup_repos_set.size());
-        std::copy(
-            setup_repos_set.begin(),
-            setup_repos_set.end(),
-            std::inserter(setup_repos->to_setup, setup_repos->to_setup.end()));
-        setup_repos->to_include.clear();
-        setup_repos->to_include.reserve(include_repos_set.size());
-        std::copy(include_repos_set.begin(),
-                  include_repos_set.end(),
-                  std::inserter(setup_repos->to_include,
-                                setup_repos->to_include.end()));
     }
+
+    // copy to vectors
+    setup_repos->to_setup.clear();
+    setup_repos->to_setup.reserve(setup_repos_set.size());
+    std::copy(
+        setup_repos_set.begin(),
+        setup_repos_set.end(),
+        std::inserter(setup_repos->to_setup, setup_repos->to_setup.end()));
+    setup_repos->to_include.clear();
+    setup_repos->to_include.reserve(include_repos_set.size());
+    std::copy(
+        include_repos_set.begin(),
+        include_repos_set.end(),
+        std::inserter(setup_repos->to_include, setup_repos->to_include.end()));
 }
 
 void DefaultReachableRepositories(
     ExpressionPtr const& repos,
     std::shared_ptr<JustMR::SetupRepos> const& setup_repos) {
-    if (repos.IsNotNull() and repos->IsMap()) {
-        setup_repos->to_setup = repos->Map().Keys();
-        setup_repos->to_include = setup_repos->to_setup;
-    }
+    setup_repos->to_setup = repos->Map().Keys();
+    setup_repos->to_include = setup_repos->to_setup;
 }
 
 auto ReadConfiguration(
diff --git a/src/other_tools/just_mr/setup_utils.hpp b/src/other_tools/just_mr/setup_utils.hpp
index d66a1ba1..80f01d75 100644
--- a/src/other_tools/just_mr/setup_utils.hpp
+++ b/src/other_tools/just_mr/setup_utils.hpp
@@ -43,12 +43,14 @@ struct SetupRepos {
 namespace Utils {
 
 /// \brief Get the repo dependency closure for a given main repository.
+/// \param repos ExpressionPtr of Map type.
 void ReachableRepositories(
     ExpressionPtr const& repos,
     std::string const& main,
     std::shared_ptr<JustMR::SetupRepos> const& setup_repos);
 
 /// \brief By default, we set up and include the full repo dependency closure.
+/// \param repos ExpressionPtr of Map type.
 void DefaultReachableRepositories(
     ExpressionPtr const& repos,
     std::shared_ptr<JustMR::SetupRepos> const& setup_repos);
diff --git a/src/other_tools/just_mr/update.cpp b/src/other_tools/just_mr/update.cpp
index 25fd60d3..885f5421 100644
--- a/src/other_tools/just_mr/update.cpp
+++ b/src/other_tools/just_mr/update.cpp
@@ -50,6 +50,11 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config,
                     "Config: Mandatory key \"repositories\" missing");
         return kExitUpdateError;
     }
+    if (not repos->IsMap()) {
+        Logger::Log(LogLevel::Error,
+                    "Config: Value for key \"repositories\" is not a map");
+        return kExitUpdateError;
+    }
     // gather repos to update
     std::vector<std::pair<std::string, std::string>> repos_to_update{};
     repos_to_update.reserve(update_args.repos_to_update.size());
@@ -72,6 +77,13 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config,
                                         nlohmann::json(repo_name).dump()));
                 return kExitUpdateError;
             }
+            if (not resolved_repo_desc.value()->IsMap()) {
+                Logger::Log(
+                    LogLevel::Error,
+                    "Config: Repository {} resolves to a non-map description",
+                    nlohmann::json(repo_name).dump());
+                return kExitUpdateError;
+            }
             // get repo_type
             auto repo_type = (*resolved_repo_desc)->At("type");
             if (not repo_type) {
diff --git a/src/other_tools/just_mr/utils.cpp b/src/other_tools/just_mr/utils.cpp
index 7e99eb43..031e58cc 100644
--- a/src/other_tools/just_mr/utils.cpp
+++ b/src/other_tools/just_mr/utils.cpp
@@ -30,7 +30,22 @@ auto ResolveRepo(ExpressionPtr const& repo_desc,
         return std::nullopt;
     }
     [[maybe_unused]] auto insert_res = seen->insert(desc_str);
-    return ResolveRepo(repos[desc_str]["repository"], repos, seen);
+    auto new_repo_desc = repos[desc_str];
+    if (not new_repo_desc->IsMap()) {
+        Logger::Log(LogLevel::Error,
+                    "Config: While resolving dependencies:\nDescription of "
+                    "repository {} is not a map",
+                    desc_str);
+        return std::nullopt;
+    }
+    if (not new_repo_desc->At("repository")) {
+        Logger::Log(LogLevel::Error,
+                    "Config: While resolving dependencies:\nKey \"repository\" "
+                    "missing for repository {}",
+                    desc_str);
+        return std::nullopt;
+    }
+    return ResolveRepo(new_repo_desc->At("repository")->get(), repos, seen);
 }
 
 auto ResolveRepo(ExpressionPtr const& repo_desc,
@@ -41,7 +56,7 @@ auto ResolveRepo(ExpressionPtr const& repo_desc,
         return ResolveRepo(repo_desc, repos, &seen);
     } catch (std::exception const& e) {
         Logger::Log(LogLevel::Error,
-                    "Config: While resolving dependencies: {}",
+                    "Config: While resolving dependencies:\n{}",
                     e.what());
         return std::nullopt;
     }
diff --git a/src/other_tools/just_mr/utils.hpp b/src/other_tools/just_mr/utils.hpp
index 54abd3d5..c0c98c7a 100644
--- a/src/other_tools/just_mr/utils.hpp
+++ b/src/other_tools/just_mr/utils.hpp
@@ -151,6 +151,7 @@ namespace JustMR::Utils {
 
 /// \brief Recursive part of the ResolveRepo function.
 /// Keeps track of repository names to detect any cyclic dependencies.
+/// \param repos ExpressionPtr of Map type.
 [[nodiscard]] auto ResolveRepo(
     ExpressionPtr const& repo_desc,
     ExpressionPtr const& repos,
@@ -158,9 +159,10 @@ namespace JustMR::Utils {
     -> std::optional<ExpressionPtr>;
 
 /// \brief Resolves any cyclic dependency issues and follows the repository
-/// dependencies until the one containing the WS root is found.
+/// dependencies until the one containing the workspace root is found.
 /// Returns a repository entry as an ExpressionPtr, or nullopt if cyclic
 /// dependency found.
+/// \param repos ExpressionPtr of Map type.
 [[nodiscard]] auto ResolveRepo(ExpressionPtr const& repo_desc,
                                ExpressionPtr const& repos) noexcept
     -> std::optional<ExpressionPtr>;
diff --git a/src/other_tools/repo_map/repos_to_setup_map.cpp b/src/other_tools/repo_map/repos_to_setup_map.cpp
index 3af2db3f..3bad60f4 100644
--- a/src/other_tools/repo_map/repos_to_setup_map.cpp
+++ b/src/other_tools/repo_map/repos_to_setup_map.cpp
@@ -121,9 +121,10 @@ void GitCheckout(ExpressionPtr const& repo_desc,
     }
     // check "special" pragma
     auto repo_desc_pragma = repo_desc->At("pragma");
-    auto pragma_special = repo_desc_pragma
-                              ? repo_desc_pragma->get()->At("special")
-                              : std::nullopt;
+    bool const& pragma_is_map =
+        repo_desc_pragma and repo_desc_pragma->get()->IsMap();
+    auto pragma_special =
+        pragma_is_map ? repo_desc_pragma->get()->At("special") : std::nullopt;
     auto pragma_special_value =
         pragma_special and pragma_special->get()->IsString() and
                 kPragmaSpecialMap.contains(pragma_special->get()->String())
@@ -132,7 +133,7 @@ void GitCheckout(ExpressionPtr const& repo_desc,
             : std::nullopt;
     // check "absent" pragma
     auto pragma_absent =
-        repo_desc_pragma ? repo_desc_pragma->get()->At("absent") : std::nullopt;
+        pragma_is_map ? repo_desc_pragma->get()->At("absent") : std::nullopt;
     auto pragma_absent_value = pragma_absent and
                                pragma_absent->get()->IsBool() and
                                pragma_absent->get()->Bool();
@@ -242,9 +243,10 @@ void ArchiveCheckout(ExpressionPtr const& repo_desc,
     }
     // check "special" pragma
     auto repo_desc_pragma = repo_desc->At("pragma");
-    auto pragma_special = repo_desc_pragma
-                              ? repo_desc_pragma->get()->At("special")
-                              : std::nullopt;
+    bool const& pragma_is_map =
+        repo_desc_pragma and repo_desc_pragma->get()->IsMap();
+    auto pragma_special =
+        pragma_is_map ? repo_desc_pragma->get()->At("special") : std::nullopt;
     auto pragma_special_value =
         pragma_special and pragma_special->get()->IsString() and
                 kPragmaSpecialMap.contains(pragma_special->get()->String())
@@ -253,7 +255,7 @@ void ArchiveCheckout(ExpressionPtr const& repo_desc,
             : std::nullopt;
     // check "absent" pragma
     auto pragma_absent =
-        repo_desc_pragma ? repo_desc_pragma->get()->At("absent") : std::nullopt;
+        pragma_is_map ? repo_desc_pragma->get()->At("absent") : std::nullopt;
     auto pragma_absent_value = pragma_absent and
                                pragma_absent->get()->IsBool() and
                                pragma_absent->get()->Bool();
@@ -334,9 +336,10 @@ void FileCheckout(ExpressionPtr const& repo_desc,
         std::filesystem::absolute(repo_desc_path->get()->String()));
     // check "special" pragma
     auto repo_desc_pragma = repo_desc->At("pragma");
-    auto pragma_special = repo_desc_pragma
-                              ? repo_desc_pragma->get()->At("special")
-                              : std::nullopt;
+    bool const& pragma_is_map =
+        repo_desc_pragma and repo_desc_pragma->get()->IsMap();
+    auto pragma_special =
+        pragma_is_map ? repo_desc_pragma->get()->At("special") : std::nullopt;
     auto pragma_special_value =
         pragma_special and pragma_special->get()->IsString() and
                 kPragmaSpecialMap.contains(pragma_special->get()->String())
@@ -345,14 +348,14 @@ void FileCheckout(ExpressionPtr const& repo_desc,
             : std::nullopt;
     // check "to_git" pragma
     auto pragma_to_git =
-        repo_desc_pragma ? repo_desc_pragma->get()->At("to_git") : std::nullopt;
+        pragma_is_map ? repo_desc_pragma->get()->At("to_git") : std::nullopt;
     // resolving symlinks implies also to_git
     if (pragma_special_value == PragmaSpecial::ResolvePartially or
         pragma_special_value == PragmaSpecial::ResolveCompletely or
         (pragma_to_git and pragma_to_git->get()->IsBool() and
          pragma_to_git->get()->Bool())) {
         // check "absent" pragma
-        auto pragma_absent = repo_desc_pragma
+        auto pragma_absent = pragma_is_map
                                  ? repo_desc_pragma->get()->At("absent")
                                  : std::nullopt;
         auto pragma_absent_value = pragma_absent and
@@ -425,8 +428,9 @@ void DistdirCheckout(ExpressionPtr const& repo_desc,
     }
     // check "absent" pragma
     auto repo_desc_pragma = repo_desc->At("pragma");
-    auto pragma_absent =
-        repo_desc_pragma ? repo_desc_pragma->get()->At("absent") : std::nullopt;
+    auto pragma_absent = (repo_desc_pragma and repo_desc_pragma->get()->IsMap())
+                             ? repo_desc_pragma->get()->At("absent")
+                             : std::nullopt;
     auto pragma_absent_value = pragma_absent and
                                pragma_absent->get()->IsBool() and
                                pragma_absent->get()->Bool();
@@ -727,9 +731,10 @@ void GitTreeCheckout(ExpressionPtr const& repo_desc,
     }
     // check "special" pragma
     auto repo_desc_pragma = repo_desc->At("pragma");
-    auto pragma_special = repo_desc_pragma
-                              ? repo_desc_pragma->get()->At("special")
-                              : std::nullopt;
+    bool const& pragma_is_map =
+        repo_desc_pragma and repo_desc_pragma->get()->IsMap();
+    auto pragma_special =
+        pragma_is_map ? repo_desc_pragma->get()->At("special") : std::nullopt;
     auto pragma_special_value =
         pragma_special and pragma_special->get()->IsString() and
                 kPragmaSpecialMap.contains(pragma_special->get()->String())
@@ -738,7 +743,7 @@ void GitTreeCheckout(ExpressionPtr const& repo_desc,
             : std::nullopt;
     // check "absent" pragma
     auto pragma_absent =
-        repo_desc_pragma ? repo_desc_pragma->get()->At("absent") : std::nullopt;
+        pragma_is_map ? repo_desc_pragma->get()->At("absent") : std::nullopt;
     auto pragma_absent_value = pragma_absent and
                                pragma_absent->get()->IsBool() and
                                pragma_absent->get()->Bool();
@@ -819,6 +824,13 @@ auto CreateReposToSetupMap(std::shared_ptr<Configuration> const& config,
                           /*fatal=*/true);
                 return;
             }
+            if (not repo_desc_key->get()->IsMap()) {
+                (*logger)(fmt::format("Config: Config entry for repository {} "
+                                      "is not a map",
+                                      nlohmann::json(key).dump()),
+                          /*fatal=*/true);
+                return;
+            }
             auto repo_desc = repo_desc_key->get()->At("repository");
             if (not repo_desc) {
                 (*logger)(fmt::format("Config: Mandatory key \"repository\" "
@@ -836,6 +848,13 @@ auto CreateReposToSetupMap(std::shared_ptr<Configuration> const& config,
                           /*fatal=*/true);
                 return;
             }
+            if (not resolved_repo_desc.value()->IsMap()) {
+                Logger::Log(
+                    LogLevel::Error,
+                    "Config: Repository {} resolves to a non-map description",
+                    nlohmann::json(key).dump());
+                return;
+            }
             auto repo_type = (*resolved_repo_desc)->At("type");
             if (not repo_type) {
                 (*logger)(fmt::format("Config: Mandatory key \"type\" missing "