Fix user git access

src/pull_module/libgit2.cpp

@@ -253,6 +253,38 @@ Libgt2InitGuard::Libgt2InitGuard(const Libgit2Options& opts) {
     SPDLOG_DEBUG("Initializing libgit2");
     this->status = git_libgit2_init();
     IF_ERROR_SET_MSG_AND_RETURN();
+    // Disable ownership check so repositories owned by a different OS user can be
+    // opened for reading (equivalent to git's `safe.directory = *`).  This is safe
+    // in a serving context where the operator intentionally mounts model directories
+    // that may have been downloaded by a different UID (e.g. root in the build
+    // container vs. a non-root serving user).
+    this->status = git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 0);
+    IF_ERROR_SET_MSG_AND_RETURN();
+    // Redirect all git config search paths to an empty string so libgit2 never reads
+    // host-level git configuration (~/.gitconfig, /etc/gitconfig, etc.).  Without this,
+    // a host gitconfig that sets credential.helper, http.proxy, lfs.*, or safe.directory
+    // can silently override OVMS's intended proxy/token settings and cause spurious
+    // failures or credential leaks in multi-tenant environments.
+    // On Windows, GIT_CONFIG_LEVEL_PROGRAMDATA covers %PROGRAMDATA%\Git\config which is
+    // a machine-wide config that libgit2 reads before SYSTEM; it must be cleared as well.
+    this->status = git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, "");
+    IF_ERROR_SET_MSG_AND_RETURN();
+    this->status = git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, "");
+    IF_ERROR_SET_MSG_AND_RETURN();
+    this->status = git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, "");
+    IF_ERROR_SET_MSG_AND_RETURN();
+#if defined(_WIN32)
+    this->status = git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_PROGRAMDATA, "");
+    IF_ERROR_SET_MSG_AND_RETURN();
+#endif
+    // Skip .keep file existence checks when reading packfiles.  libgit2 performs one
+    // stat() per pack per operation to honour .keep files (which prevent gc from
+    // collecting referenced packs).  In an OVMS deployment the model directory is
+    // never garbage-collected and may live on NFS or other high-latency remote
+    // filesystems, so removing this stat() per open noticeably reduces latency on
+    // resume/status operations against large repositories.
+    this->status = git_libgit2_opts(GIT_OPT_DISABLE_PACK_KEEP_FILE_CHECKS, 1);
+    IF_ERROR_SET_MSG_AND_RETURN();
     SPDLOG_TRACE("Setting libgit2 server connection timeout:{}", opts.serverConnectTimeoutMs);
     this->status = git_libgit2_opts(GIT_OPT_SET_SERVER_CONNECT_TIMEOUT, opts.serverConnectTimeoutMs);
     IF_ERROR_SET_MSG_AND_RETURN();
@@ -1096,6 +1128,7 @@ Status resumeLfsDownloadForFile(git_repository* repo, const char* filePathInRepo
 namespace {
 
 struct ResumeCandidates {
+    bool shouldResume = false;
     bool hasWipMarker = false;
     bool hasLfsErrorFile = false;
     bool interruptionLikely = false;
@@ -1111,11 +1144,7 @@ struct ResumeCandidates {
  * @return ResumeCandidates containing LFS and non-LFS recovery targets.
  * @note Works on local repository metadata and filesystem; no network operations.
  */
-ResumeCandidates buildResumeCandidates(git_repository* repo, const std::string& downloadPath) {
-    ResumeCandidates candidates;
-    candidates.hasWipMarker = libgit2::hasLfsWipMarker(downloadPath);
-    candidates.hasLfsErrorFile = libgit2::hasLfsErrorFile(downloadPath);
-
+ResumeCandidates buildResumeCandidates(git_repository* repo, const std::string& downloadPath, ResumeCandidates candidates) {
     // Checking if the download was partially finished for any files in repository,
     // including tracked LFS pointer blobs missing from the worktree after abrupt termination.
     candidates.lfsMatches = libgit2::findResumableLfsFiles(repo, downloadPath, candidates.hasWipMarker || candidates.hasLfsErrorFile);
@@ -1246,14 +1275,26 @@ Status resumeExistingRepository(git_repository* repo,
     return StatusCode::OK;
 }
 
-Status handleExistingRepositoryWithoutOverwrite(const std::string& downloadPath,
-    const std::function<Status(bool)>& checkRepositoryStatusFn) {
-    // If the directory does not contain a .git entry, treat it as a user-provided model directory.
-    // The user has copied model files in by hand; skip the pull and let model loading proceed
-    // against whatever files are already on disk. Use --overwrite_models to replace it with a
-    // fresh download.
+bool resumeCheckSecondCondition(const std::string& downloadPath, const ResumeCandidates& candidates) {
+    auto existingMatches = ovms::libgit2::findLfsLikeFiles(downloadPath, true);
+
+    // Use repository object only when interruption markers indicate a previous
+    // pull likely failed and resume logic may be required.
+    if (!candidates.hasWipMarker && !candidates.hasLfsErrorFile && existingMatches.empty()) {
+        SPDLOG_DEBUG("Model pull operation found no interruption markers for this path: {}", downloadPath);
+        SPDLOG_INFO("Path already exists on local filesystem. Skipping download to path: {}", downloadPath);
+        return false;
+    }
+
+    if (!existingMatches.empty()) {
+        SPDLOG_DEBUG("Found {} LFS-like file(s) under path: {}. Enabling resume check.", existingMatches.size(), downloadPath);
+    }
+    return true;
+}
+
+Status resumeCheckFirstCondition(const std::string& downloadPath, bool& gitEntryExists) {
     std::error_code ec;
-    const bool gitEntryExists = fs::exists(fs::path(downloadPath) / ".git", ec);
+    gitEntryExists = fs::exists(fs::path(downloadPath) / ".git", ec);
     if (ec) {
         // Probe itself failed (permission denied, I/O error, ...). Do not silently fall through
         // to the "not a git repository" branch, that would mask real filesystem problems.
@@ -1264,6 +1305,42 @@ Status handleExistingRepositoryWithoutOverwrite(const std::string& downloadPath,
         SPDLOG_INFO("Path \"{}\" exists but is not a git repository. "
                     "Skipping download and using existing files.",
             downloadPath);
+    }
+    return StatusCode::OK;
+}
+
+Status checkSufficientResumeConditions(const std::string& downloadPath, ResumeCandidates& candidates) {
+    candidates.shouldResume = false;
+
+    bool gitEntryExists = false;
+    auto firstConditionStatus = resumeCheckFirstCondition(downloadPath, gitEntryExists);
+    if (!firstConditionStatus.ok()) {
+        return firstConditionStatus;
+    }
+    if (!gitEntryExists) {
+        return StatusCode::OK;
+    }
+
+    // Probe interruption markers once and reuse them later when building candidates.
+    candidates.hasWipMarker = libgit2::hasLfsWipMarker(downloadPath);
+    candidates.hasLfsErrorFile = libgit2::hasLfsErrorFile(downloadPath);
+
+    candidates.shouldResume = resumeCheckSecondCondition(downloadPath, candidates);
+    return StatusCode::OK;
+}
+
+Status handleExistingRepositoryWithoutOverwrite(const std::string& downloadPath,
+    const std::function<Status(bool)>& checkRepositoryStatusFn) {
+    // If the directory does not contain a .git entry, treat it as a user-provided model directory.
+    // The user has copied model files in by hand; skip the pull and let model loading proceed
+    // against whatever files are already on disk. Use --overwrite_models to replace it with a
+    // fresh download.
+    ResumeCandidates candidates;
+    auto sufficientConditionsStatus = checkSufficientResumeConditions(downloadPath, candidates);
+    if (!sufficientConditionsStatus.ok()) {
+        return sufficientConditionsStatus;
+    }
+    if (!candidates.shouldResume) {
         return StatusCode::OK;
     }
 
@@ -1276,9 +1353,9 @@ Status handleExistingRepositoryWithoutOverwrite(const std::string& downloadPath,
         return mapRepositoryOpenFailureToStatus(repoGuard);
     }
 
-    auto candidates = buildResumeCandidates(repoGuard.get(), downloadPath);
+    candidates = buildResumeCandidates(repoGuard.get(), downloadPath, std::move(candidates));
     if (!candidates.interruptionLikely) {
-        SPDLOG_DEBUG("Model pull operation found no interruption signals for this path: {}", downloadPath);
+        SPDLOG_WARN("Interruption marker(s) were found but no resumable candidates were detected for path: {}", downloadPath);
         SPDLOG_INFO("Path already exists on local filesystem. Skipping download to path: {}", downloadPath);
         return StatusCode::OK;
     }

src/test/libgit2_test.cpp

@@ -849,3 +849,75 @@ TEST(LibGit2LfsWipMarker, MarkersForDifferentRepositoriesAreIndependent) {
     EXPECT_FALSE(ovms::libgit2::hasLfsWipMarker(repoAPath));
     EXPECT_TRUE(ovms::libgit2::hasLfsWipMarker(repoBPath));
 }
+
+// ---------------------------------------------------------------------------
+// Libgt2InitGuard initialization behavior
+//
+// These tests exercise the process-global libgit2 options set in
+// Libgt2InitGuard's constructor: ownership-validation suppression and config
+// search-path isolation.  Each test creates its own guard so that the options
+// are set fresh; as git_libgit2_init() is ref-counted by libgit2 it is safe to
+// call it multiple times within the same process.
+// ---------------------------------------------------------------------------
+
+class Libgt2InitGuardTest : public ::testing::Test {
+protected:
+    TempDir td;
+    ovms::Libgit2Options defaultOpts;
+};
+
+TEST_F(Libgt2InitGuardTest, ConstructionSucceeds) {
+    ovms::Libgt2InitGuard guard(defaultOpts);
+    EXPECT_GE(guard.status, 0);
+    EXPECT_TRUE(guard.errMsg.empty());
+    EXPECT_TRUE(guard.countedAsInitialized);
+}
+
+// After the guard is constructed, libgit2 must have owner-validation disabled
+// so that repositories owned by a different OS user can be opened.
+TEST_F(Libgt2InitGuardTest, OwnerValidationIsDisabled) {
+    ovms::Libgt2InitGuard guard(defaultOpts);
+    ASSERT_GE(guard.status, 0);
+
+    int ownerValidation = 1;  // preset to non-zero; guard must set it to 0
+    int rc = git_libgit2_opts(GIT_OPT_GET_OWNER_VALIDATION, &ownerValidation);
+    EXPECT_EQ(rc, 0);
+    EXPECT_EQ(ownerValidation, 0);
+}
+
+// The guard must clear the config search paths for all host-level config
+// scopes so that no host gitconfig can interfere with OVMS's settings.
+TEST_F(Libgt2InitGuardTest, ConfigSearchPathsAreCleared) {
+    ovms::Libgt2InitGuard guard(defaultOpts);
+    ASSERT_GE(guard.status, 0);
+
+    static const int levels[] = {
+        GIT_CONFIG_LEVEL_SYSTEM,
+        GIT_CONFIG_LEVEL_XDG,
+        GIT_CONFIG_LEVEL_GLOBAL,
+    };
+    for (int level : levels) {
+        git_buf buf = GIT_BUF_INIT;
+        int rc = git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, level, &buf);
+        EXPECT_EQ(rc, 0) << "GIT_OPT_GET_SEARCH_PATH failed for config level " << level;
+        const char* path = (buf.ptr != nullptr) ? buf.ptr : "";
+        EXPECT_STREQ(path, "") << "Config search path not cleared for level " << level;
+        git_buf_dispose(&buf);
+    }
+}
+
+#if defined(_WIN32)
+// On Windows, libgit2 also supports GIT_CONFIG_LEVEL_PROGRAMDATA which maps to
+// %PROGRAMDATA%\Git\config — a machine-wide config that must also be suppressed.
+TEST_F(Libgt2InitGuardTest, ConfigSearchPathProgramdataClearedOnWindows) {
+    ovms::Libgt2InitGuard guard(defaultOpts);
+    ASSERT_GE(guard.status, 0);
+
+    git_buf buf = GIT_BUF_INIT;
+    int rc = git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_PROGRAMDATA, &buf);
+    EXPECT_EQ(rc, 0);
+    const char* path = (buf.ptr != nullptr) ? buf.ptr : "";
+    EXPECT_STREQ(path, "");
+    git_buf_dispose(&buf);
+}
+#endif