Fix user git access by rasapala · Pull Request #4253 · openvinotoolkit/model_server

rasapala · 2026-05-28T09:18:14Z

🛠 Summary

JIRA CVS-187604
Disables libgit2 owner validation for repositories opened by OVMS.
Clears selected libgit2 git config search paths to avoid reading host/user git configuration.
Disables pack .keep file checks to reduce stat calls during repository operations.

🧪 Checklist

Unit tests added.
The documentation updated.
Change follows security best practices.
``

Copilot

Pull request overview

This PR updates libgit2 initialization for Hugging Face git-clone based model pulls to avoid repository access failures when model directories are owned by a different OS user.

Changes:

Disables libgit2 owner validation for repositories opened by OVMS.
Clears selected libgit2 git config search paths to avoid reading host/user git configuration.
Disables pack .keep file checks to reduce stat calls during repository operations.

…/OpenVINO-model-server into CVS-187604_user_download

atobiszei · 2026-06-08T08:53:32Z

+    // 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();


Shouldn't this be limited only to container environments? Even then(in container) one could want to override some git settings used by OVMS.

The settings may affect the resume and download logic. We do not want to risk this kind of situation.

atobiszei · 2026-06-08T08:55:26Z

-    candidates.hasWipMarker = libgit2::hasLfsWipMarker(downloadPath);
-    candidates.hasLfsErrorFile = libgit2::hasLfsErrorFile(downloadPath);
-
+ResumeCandidates buildResumeCandidates(git_repository* repo, const std::string& downloadPath, ResumeCandidates candidates) {


Minor but we create a copy of resume candidates. Couldn't we return ptr then?

atobiszei · 2026-06-08T09:36:55Z

-    // 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) {


First/Second -> whats the meaning of First/Second? Be more descriptive in naming. eg resumeCheckDotGitExistence

atobiszei · 2026-06-08T09:41:01Z

+    return StatusCode::OK;
+}
+
+Status handleExistingRepositoryWithoutOverwrite(const std::string& downloadPath,


In the context of:
It could just be to resumeClone? Or maybe resumeCloneIfNotLocal?handleExistingRepository
overwrite is confusing at this point

atobiszei · 2026-06-08T09:45:36Z

    }

-    auto candidates = buildResumeCandidates(repoGuard.get(), downloadPath);
+    candidates = buildResumeCandidates(repoGuard.get(), downloadPath, std::move(candidates));


Just pass candidates as reference?

atobiszei · 2026-06-08T09:47:27Z

+
+class Libgt2InitGuardTest : public ::testing::Test {
+protected:
+    TempDir td;


atobiszei · 2026-06-10T08:27:50Z

+    // container vs. a non-root serving user).
+    this->status = git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 0);
+    IF_ERROR_SET_MSG_AND_RETURN();
+    const char* enableSearchPathEnv = std::getenv("GIT_OPT_SET_ENABLE_SEARCH_PATHS");


use ovms::GetEnvVar

atobiszei · 2026-06-10T08:38:58Z

+    envGuard.set("GIT_OPT_SET_ENABLE_SEARCH_PATHS", "1");
+
+    std::array<std::string, 3> baselinePaths;
+    ASSERT_EQ(git_libgit2_init(), 1);
+    {
+        git_buf systemBuf = GIT_BUF_INIT;
+        ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, &systemBuf), 0);
+        baselinePaths[0] = (systemBuf.ptr != nullptr) ? systemBuf.ptr : "";
+        git_buf_dispose(&systemBuf);
+
+        git_buf xdgBuf = GIT_BUF_INIT;
+        ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, &xdgBuf), 0);
+        baselinePaths[1] = (xdgBuf.ptr != nullptr) ? xdgBuf.ptr : "";
+        git_buf_dispose(&xdgBuf);
+
+        git_buf globalBuf = GIT_BUF_INIT;
+        ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, &globalBuf), 0);
+        baselinePaths[2] = (globalBuf.ptr != nullptr) ? globalBuf.ptr : "";
+        git_buf_dispose(&globalBuf);
+    }
+    git_libgit2_shutdown();
+
+    ovms::Libgt2InitGuard guard(defaultOpts);
+    ASSERT_GE(guard.status, 0);
+
+    git_buf systemBuf = GIT_BUF_INIT;
+    ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, &systemBuf), 0);
+    EXPECT_EQ((systemBuf.ptr != nullptr) ? systemBuf.ptr : "", baselinePaths[0]);
+    git_buf_dispose(&systemBuf);
+
+    git_buf xdgBuf = GIT_BUF_INIT;
+    ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, &xdgBuf), 0);
+    EXPECT_EQ((xdgBuf.ptr != nullptr) ? xdgBuf.ptr : "", baselinePaths[1]);
+    git_buf_dispose(&xdgBuf);
+
+    git_buf globalBuf = GIT_BUF_INIT;
+    ASSERT_EQ(git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, &globalBuf), 0);
+    EXPECT_EQ((globalBuf.ptr != nullptr) ? globalBuf.ptr : "", baselinePaths[2]);
+    git_buf_dispose(&globalBuf);
+}


Create utility that will check expected variable values with boolean parameter (useGlobal)
Then we need either 1 test or 3 tests when we check
checkifUsingGlobalConfig( boolean expectToUseGlobal)

3 cases:

Undefined env GIT_OPT_SET_ENABLE_SEARCH_PATHS

defined set to 0

defined set to 1

Fix user git access

9f2fc21

Copilot AI review requested due to automatic review settings May 28, 2026 09:18

Copilot started reviewing on behalf of rasapala May 28, 2026 09:18 View session

Style

7fff31c

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread src/pull_module/libgit2.cpp Outdated

Comment thread src/pull_module/libgit2.cpp Outdated

rasapala added 2 commits May 28, 2026 11:41

Add tests

e9648ec

Fix tests

2c95e0f

rasapala requested review from dtrawins and mzegla May 28, 2026 12:03

rasapala added 7 commits May 28, 2026 16:53

Add should resume logic

42140f2

Merge branch 'CVS-187604_user_download' of https://github.com/IntelAI…

3a3439a

…/OpenVINO-model-server into CVS-187604_user_download

Style

62a3a68

Add new tests

e28abc4

Style

06367dc

Add lfs condition

8450339

Syle

6764e1c

rasapala requested a review from atobiszei June 8, 2026 08:45

atobiszei reviewed Jun 8, 2026

View reviewed changes

rasapala added 5 commits June 8, 2026 12:55

Merge branch 'main' into CVS-187604_user_download

effdc6e

Code review

051faee

Add env to set path searches

99bf761

Fix

03bdd70

Update doc and name

a6d0262

atobiszei reviewed Jun 10, 2026

View reviewed changes

Comment thread src/pull_module/libgit2.cpp

atobiszei reviewed Jun 10, 2026

View reviewed changes

Fix tests

a9a9867

atobiszei approved these changes Jun 10, 2026

View reviewed changes

rasapala added 4 commits June 10, 2026 15:05

Merge branch 'main' into CVS-187604_user_download

3df5f1e

Update descriptions

a9e451c

Test names

6620102

Add test timeout log

5f1ae0a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix user git access#4253

Fix user git access#4253
rasapala wants to merge 21 commits into
mainfrom
CVS-187604_user_download

rasapala commented May 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

atobiszei Jun 8, 2026

Uh oh!

rasapala Jun 8, 2026

Uh oh!

atobiszei Jun 8, 2026

Uh oh!

atobiszei Jun 8, 2026

Uh oh!

atobiszei Jun 8, 2026 •

edited

Loading

Uh oh!

atobiszei Jun 8, 2026

Uh oh!

atobiszei Jun 8, 2026

Uh oh!

atobiszei Jun 10, 2026

Uh oh!

Uh oh!

atobiszei Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rasapala commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠 Summary

🧪 Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atobiszei Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rasapala commented May 28, 2026 •

edited

Loading

atobiszei Jun 8, 2026 •

edited

Loading