From cc33fd47c27f9a1d45d4fde8d03e13909b8b7678 Mon Sep 17 00:00:00 2001 From: reggie-k Date: Wed, 15 Oct 2025 15:17:48 +0300 Subject: [PATCH 1/5] delete index.lock file upon repository init Signed-off-by: reggie-k --- reposerver/repository/repository.go | 24 ++++++++++++++++++++++++ reposerver/repository/repository_test.go | 15 +++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 1abccf6bcd2dd..b758c32c108ce 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -165,6 +165,30 @@ func (s *Service) Init() error { fullPath := filepath.Join(s.rootDir, file.Name()) closer := s.gitRepoInitializer(fullPath) if repo, err := gogit.PlainOpen(fullPath); err == nil { + indexFile := filepath.Join(fullPath, ".git", "index") + indexLockFile := filepath.Join(fullPath, ".git", "index.lock") + if _, err = os.Stat(indexLockFile); err == nil { + log.Warnf("Lock file present in git repository %s, removing it", fullPath) + if err = os.Remove(indexLockFile); err != nil { + log.Errorf("Failed to remove lock file %s: %v", indexLockFile, err) + } + if err = os.Remove(indexFile); err != nil { + log.Errorf("Failed to remove index file %s: %v", indexFile, err) + } + + if err == nil { + wt, _ := repo.Worktree() + headRef, _ := repo.Head() + err = wt.Reset(&gogit.ResetOptions{ + Mode: gogit.MixedReset, + Commit: headRef.Hash(), + }) + } + + if err != nil { + log.Errorf("Failed to reset git repo %s: %v", fullPath, err) + } + } if remotes, err := repo.Remotes(); err == nil && len(remotes) > 0 && len(remotes[0].Config().URLs) > 0 { s.gitRepoPaths.Add(git.NormalizeGitURL(remotes[0].Config().URLs[0]), fullPath) } diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index e96fc3540a95e..b659f714359cd 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -3014,6 +3014,21 @@ func TestInit(t *testing.T) { _, err := os.ReadDir(dir) require.Error(t, err) initGitRepo(t, newGitRepoOptions{path: path.Join(dir, "repo2"), remote: "https://github.com/argo-cd/test-repo2", createPath: true, addEmptyCommit: false}) + + repoPath = path.Join(dir, "repo3") + lockFile := path.Join(repoPath, ".git", "index.lock") + initGitRepo(t, newGitRepoOptions{path: repoPath, remote: "https://github.com/argo-cd/test-repo3", createPath: true, addEmptyCommit: false}) + require.NoError(t, os.WriteFile(lockFile, []byte("test"), 0o644)) + + service = newService(t, ".") + service.rootDir = dir + + _, err = os.Stat(lockFile) + require.NoError(t, err) + require.NoError(t, service.Init()) + _, err = os.Stat(lockFile) + require.Error(t, err, "lock file should be removed after Init()") + require.ErrorContains(t, err, ".git/index.lock: no such file or directory") } // TestCheckoutRevisionCanGetNonstandardRefs shows that we can fetch a revision that points to a non-standard ref. In From db509389867b05b3b4efe9072c94f1fd935a0c28 Mon Sep 17 00:00:00 2001 From: reggie-k Date: Sun, 19 Oct 2025 10:24:35 +0300 Subject: [PATCH 2/5] delete index.lock file upon repository init and fix tests Signed-off-by: reggie-k --- reposerver/repository/repository.go | 37 ++++++++------ reposerver/repository/repository_test.go | 64 ++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index b758c32c108ce..e10cae8352774 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -165,17 +165,27 @@ func (s *Service) Init() error { fullPath := filepath.Join(s.rootDir, file.Name()) closer := s.gitRepoInitializer(fullPath) if repo, err := gogit.PlainOpen(fullPath); err == nil { - indexFile := filepath.Join(fullPath, ".git", "index") - indexLockFile := filepath.Join(fullPath, ".git", "index.lock") - if _, err = os.Stat(indexLockFile); err == nil { - log.Warnf("Lock file present in git repository %s, removing it", fullPath) - if err = os.Remove(indexLockFile); err != nil { - log.Errorf("Failed to remove lock file %s: %v", indexLockFile, err) - } - if err = os.Remove(indexFile); err != nil { - log.Errorf("Failed to remove index file %s: %v", indexFile, err) - } + s.cleanupStaleFiles(fullPath, []string{"index.lock", "index", "HEAD.lock"}, repo) + if remotes, err := repo.Remotes(); err == nil && len(remotes) > 0 && len(remotes[0].Config().URLs) > 0 { + s.gitRepoPaths.Add(git.NormalizeGitURL(remotes[0].Config().URLs[0]), fullPath) + } + } + io.Close(closer) + } + // remove read permissions since no-one should be able to list the directories + return os.Chmod(s.rootDir, 0o300) +} +func (s *Service) cleanupStaleFiles(fullPath string, staleFiles []string, repo *gogit.Repository) { + // for each stale file, if it exists, remove it + for _, staleFile := range staleFiles { + gitFile := filepath.Join(fullPath, ".git", staleFile) + if _, err := os.Stat(gitFile); err == nil { + log.Warnf("Stale file %s present in git repository %s, removing it", staleFile, fullPath) + if err = os.Remove(gitFile); err != nil { + log.Errorf("Failed to remove stale file %s: %v", gitFile, err) + } + if staleFile == "index" { if err == nil { wt, _ := repo.Worktree() headRef, _ := repo.Head() @@ -189,14 +199,9 @@ func (s *Service) Init() error { log.Errorf("Failed to reset git repo %s: %v", fullPath, err) } } - if remotes, err := repo.Remotes(); err == nil && len(remotes) > 0 && len(remotes[0].Config().URLs) > 0 { - s.gitRepoPaths.Add(git.NormalizeGitURL(remotes[0].Config().URLs[0]), fullPath) - } } - io.Close(closer) } - // remove read permissions since no-one should be able to list the directories - return os.Chmod(s.rootDir, 0o300) + } // ListRefs List a subset of the refs (currently, branches and tags) of a git repo diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index b659f714359cd..3dec7e4714b2d 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -3016,21 +3016,75 @@ func TestInit(t *testing.T) { initGitRepo(t, newGitRepoOptions{path: path.Join(dir, "repo2"), remote: "https://github.com/argo-cd/test-repo2", createPath: true, addEmptyCommit: false}) repoPath = path.Join(dir, "repo3") - lockFile := path.Join(repoPath, ".git", "index.lock") - initGitRepo(t, newGitRepoOptions{path: repoPath, remote: "https://github.com/argo-cd/test-repo3", createPath: true, addEmptyCommit: false}) - require.NoError(t, os.WriteFile(lockFile, []byte("test"), 0o644)) + lockFile1 := path.Join(repoPath, ".git", "index.lock") + lockFile2 := path.Join(repoPath, ".git", "index") + lockFile3 := path.Join(repoPath, ".git", "HEAD.lock") + initGitRepo(t, newGitRepoOptions{path: repoPath, remote: "https://github.com/argo-cd/test-repo3", createPath: true, addEmptyCommit: true}) + require.NoError(t, os.WriteFile(lockFile1, []byte("test"), 0o644)) + require.NoError(t, os.WriteFile(lockFile2, []byte("test"), 0o644)) + require.NoError(t, os.WriteFile(lockFile3, []byte("test"), 0o644)) service = newService(t, ".") service.rootDir = dir - _, err = os.Stat(lockFile) + _, err = os.Stat(lockFile1) require.NoError(t, err) + _, err = os.Stat(lockFile2) + require.NoError(t, err) + _, err = os.Stat(lockFile3) + require.NoError(t, err) + require.NoError(t, service.Init()) - _, err = os.Stat(lockFile) + + _, err = os.Stat(lockFile1) require.Error(t, err, "lock file should be removed after Init()") require.ErrorContains(t, err, ".git/index.lock: no such file or directory") + // _, err = os.Stat(lockFile2) + // require.Error(t, err, "lock file should be removed after Init()") + // require.ErrorContains(t, err, ".git/index: no such file or directory") + // _, err = os.Stat(lockFile3) + // require.Error(t, err, "lock file should be removed after Init()") + // require.ErrorContains(t, err, ".git/HEAD.lock: no such file or directory") } +// func TestCleanupStaleFiles(t *testing.T) { +// // Setup: create a temp directory and some files to be cleaned up +// tmpDir := t.TempDir() +// gitDir := filepath.Join(tmpDir, ".git") +// require.NoError(t, os.MkdirAll(gitDir, 0o755)) + +// staleFile1 := filepath.Join(gitDir, "index.lock") +// staleFile2 := filepath.Join(gitDir, "index") +// staleFile3 := filepath.Join(gitDir, "HEAD.lock") + +// require.NoError(t, os.WriteFile(staleFile1, []byte("foo"), 0o644)) +// require.NoError(t, os.WriteFile(staleFile2, []byte("bar"), 0o644)) +// require.NoError(t, os.WriteFile(staleFile3, []byte("baz"), 0o644)) + +// // Sanity check: files exist +// _, err := os.Stat(staleFile1) +// require.NoError(t, err) +// _, err = os.Stat(staleFile2) +// require.NoError(t, err) +// _, err = os.Stat(staleFile3) +// require.NoError(t, err) + +// repo, err := gogit.PlainInit(tmpDir, false) +// require.NoError(t, err) + +// // Call cleanupStaleFiles +// service := newService(t, tmpDir) +// service.cleanupStaleFiles(tmpDir, []string{"index.lock", "index", "HEAD.lock"}, repo) + +// // Assert: files are removed +// _, err = os.Stat(staleFile1) +// assert.True(t, os.IsNotExist(err)) +// _, err = os.Stat(staleFile2) +// assert.True(t, os.IsNotExist(err)) +// _, err = os.Stat(staleFile3) +// assert.True(t, os.IsNotExist(err)) +// } + // TestCheckoutRevisionCanGetNonstandardRefs shows that we can fetch a revision that points to a non-standard ref. In // other words, we haven't regressed and caused this issue again: https://github.com/argoproj/argo-cd/issues/4935 func TestCheckoutRevisionCanGetNonstandardRefs(t *testing.T) { From 1d8035fd508e6d0f0c8954f2a12b32d16b8ba1f9 Mon Sep 17 00:00:00 2001 From: reggie-k Date: Sun, 19 Oct 2025 10:50:52 +0300 Subject: [PATCH 3/5] make sure file is not a symlink, dir, etc Signed-off-by: reggie-k --- reposerver/repository/repository.go | 37 ++++++------ reposerver/repository/repository_test.go | 71 ++++++------------------ 2 files changed, 37 insertions(+), 71 deletions(-) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index e10cae8352774..059cf82227cb9 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -180,28 +180,33 @@ func (s *Service) cleanupStaleFiles(fullPath string, staleFiles []string, repo * // for each stale file, if it exists, remove it for _, staleFile := range staleFiles { gitFile := filepath.Join(fullPath, ".git", staleFile) - if _, err := os.Stat(gitFile); err == nil { - log.Warnf("Stale file %s present in git repository %s, removing it", staleFile, fullPath) - if err = os.Remove(gitFile); err != nil { - log.Errorf("Failed to remove stale file %s: %v", gitFile, err) - } - if staleFile == "index" { - if err == nil { - wt, _ := repo.Worktree() - headRef, _ := repo.Head() - err = wt.Reset(&gogit.ResetOptions{ - Mode: gogit.MixedReset, - Commit: headRef.Hash(), - }) + info, err := os.Lstat(gitFile) + if err == nil { + // Only delete if it's a regular file (not a symlink, dir, etc.) + if info.Mode().IsRegular() { + log.Warnf("Stale file %s present in git repository %s, removing it", staleFile, fullPath) + if err = os.Remove(gitFile); err != nil { + log.Errorf("Failed to remove stale file %s: %v", gitFile, err) } + if staleFile == "index" { + if err == nil { + wt, _ := repo.Worktree() + headRef, _ := repo.Head() + err = wt.Reset(&gogit.ResetOptions{ + Mode: gogit.MixedReset, + Commit: headRef.Hash(), + }) + } - if err != nil { - log.Errorf("Failed to reset git repo %s: %v", fullPath, err) + if err != nil { + log.Errorf("Failed to reset git repo %s: %v", fullPath, err) + } } } + } else { + log.Warnf("Stale file %s in git repository %s is not a regular file, skipping removal", staleFile, fullPath) } } - } // ListRefs List a subset of the refs (currently, branches and tags) of a git repo diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 3dec7e4714b2d..776b9f56b8c9a 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -3016,75 +3016,36 @@ func TestInit(t *testing.T) { initGitRepo(t, newGitRepoOptions{path: path.Join(dir, "repo2"), remote: "https://github.com/argo-cd/test-repo2", createPath: true, addEmptyCommit: false}) repoPath = path.Join(dir, "repo3") - lockFile1 := path.Join(repoPath, ".git", "index.lock") - lockFile2 := path.Join(repoPath, ".git", "index") - lockFile3 := path.Join(repoPath, ".git", "HEAD.lock") + indexLockFile := path.Join(repoPath, ".git", "index.lock") + indexFile := path.Join(repoPath, ".git", "index") + headLockFile := path.Join(repoPath, ".git", "HEAD.lock") initGitRepo(t, newGitRepoOptions{path: repoPath, remote: "https://github.com/argo-cd/test-repo3", createPath: true, addEmptyCommit: true}) - require.NoError(t, os.WriteFile(lockFile1, []byte("test"), 0o644)) - require.NoError(t, os.WriteFile(lockFile2, []byte("test"), 0o644)) - require.NoError(t, os.WriteFile(lockFile3, []byte("test"), 0o644)) + require.NoError(t, os.WriteFile(indexLockFile, []byte("test"), 0o644)) + require.NoError(t, os.WriteFile(headLockFile, []byte("test"), 0o644)) service = newService(t, ".") service.rootDir = dir - _, err = os.Stat(lockFile1) + _, err = os.Stat(indexLockFile) require.NoError(t, err) - _, err = os.Stat(lockFile2) + _, err = os.Stat(indexFile) require.NoError(t, err) - _, err = os.Stat(lockFile3) + _, err = os.Stat(headLockFile) require.NoError(t, err) require.NoError(t, service.Init()) - _, err = os.Stat(lockFile1) - require.Error(t, err, "lock file should be removed after Init()") + _, err = os.Stat(indexLockFile) + require.Error(t, err, "index.lock file should be removed after Init()") require.ErrorContains(t, err, ".git/index.lock: no such file or directory") - // _, err = os.Stat(lockFile2) - // require.Error(t, err, "lock file should be removed after Init()") - // require.ErrorContains(t, err, ".git/index: no such file or directory") - // _, err = os.Stat(lockFile3) - // require.Error(t, err, "lock file should be removed after Init()") - // require.ErrorContains(t, err, ".git/HEAD.lock: no such file or directory") + _, err = os.Stat(indexFile) + //index file should stay after Init(), since it was recreated after repo reset + require.NoError(t, err) + _, err = os.Stat(headLockFile) + require.Error(t, err, "HEAD.lock file should be removed after Init()") + require.ErrorContains(t, err, ".git/HEAD.lock: no such file or directory") } -// func TestCleanupStaleFiles(t *testing.T) { -// // Setup: create a temp directory and some files to be cleaned up -// tmpDir := t.TempDir() -// gitDir := filepath.Join(tmpDir, ".git") -// require.NoError(t, os.MkdirAll(gitDir, 0o755)) - -// staleFile1 := filepath.Join(gitDir, "index.lock") -// staleFile2 := filepath.Join(gitDir, "index") -// staleFile3 := filepath.Join(gitDir, "HEAD.lock") - -// require.NoError(t, os.WriteFile(staleFile1, []byte("foo"), 0o644)) -// require.NoError(t, os.WriteFile(staleFile2, []byte("bar"), 0o644)) -// require.NoError(t, os.WriteFile(staleFile3, []byte("baz"), 0o644)) - -// // Sanity check: files exist -// _, err := os.Stat(staleFile1) -// require.NoError(t, err) -// _, err = os.Stat(staleFile2) -// require.NoError(t, err) -// _, err = os.Stat(staleFile3) -// require.NoError(t, err) - -// repo, err := gogit.PlainInit(tmpDir, false) -// require.NoError(t, err) - -// // Call cleanupStaleFiles -// service := newService(t, tmpDir) -// service.cleanupStaleFiles(tmpDir, []string{"index.lock", "index", "HEAD.lock"}, repo) - -// // Assert: files are removed -// _, err = os.Stat(staleFile1) -// assert.True(t, os.IsNotExist(err)) -// _, err = os.Stat(staleFile2) -// assert.True(t, os.IsNotExist(err)) -// _, err = os.Stat(staleFile3) -// assert.True(t, os.IsNotExist(err)) -// } - // TestCheckoutRevisionCanGetNonstandardRefs shows that we can fetch a revision that points to a non-standard ref. In // other words, we haven't regressed and caused this issue again: https://github.com/argoproj/argo-cd/issues/4935 func TestCheckoutRevisionCanGetNonstandardRefs(t *testing.T) { From e8d44ae6d5f49723ac3d1aeb8d0f1d10c9a3568e Mon Sep 17 00:00:00 2001 From: reggie-k Date: Sun, 19 Oct 2025 10:57:23 +0300 Subject: [PATCH 4/5] make sure file is not a symlink, dir, etc and update test Signed-off-by: reggie-k --- reposerver/repository/repository_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 776b9f56b8c9a..713bef209a085 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -3044,6 +3044,23 @@ func TestInit(t *testing.T) { _, err = os.Stat(headLockFile) require.Error(t, err, "HEAD.lock file should be removed after Init()") require.ErrorContains(t, err, ".git/HEAD.lock: no such file or directory") + + repoPath = path.Join(dir, "repo4") + headLockDir := path.Join(repoPath, ".git", "HEAD.lock") + initGitRepo(t, newGitRepoOptions{path: repoPath, remote: "https://github.com/argo-cd/test-repo4", createPath: true, addEmptyCommit: false}) + require.NoError(t, os.Mkdir(headLockDir, 0o755)) + + service = newService(t, ".") + service.rootDir = dir + + _, err = os.Stat(headLockDir) + require.NoError(t, err) + + require.NoError(t, service.Init()) + + _, err = os.Stat(headLockDir) + //headLockDir should stay after Init(), since it is a directory + require.NoError(t, err) } // TestCheckoutRevisionCanGetNonstandardRefs shows that we can fetch a revision that points to a non-standard ref. In From f0c681669daf9df82c2a972cee7a5c2b6416c267 Mon Sep 17 00:00:00 2001 From: reggie-k Date: Sun, 19 Oct 2025 11:42:16 +0300 Subject: [PATCH 5/5] make sure file is not a symlink, dir, etc and update test Signed-off-by: reggie-k --- reposerver/repository/repository_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 713bef209a085..0402414a289bf 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -3039,7 +3039,7 @@ func TestInit(t *testing.T) { require.Error(t, err, "index.lock file should be removed after Init()") require.ErrorContains(t, err, ".git/index.lock: no such file or directory") _, err = os.Stat(indexFile) - //index file should stay after Init(), since it was recreated after repo reset + // index file should stay after Init(), since it was recreated after repo reset require.NoError(t, err) _, err = os.Stat(headLockFile) require.Error(t, err, "HEAD.lock file should be removed after Init()") @@ -3059,7 +3059,7 @@ func TestInit(t *testing.T) { require.NoError(t, service.Init()) _, err = os.Stat(headLockDir) - //headLockDir should stay after Init(), since it is a directory + // headLockDir should stay after Init(), since it is a directory require.NoError(t, err) }