Merge pull request #2260 from jesseduffield/could-not-access

.github/workflows/ci.yml

@@ -73,8 +73,9 @@ jobs:
           restore-keys: |
             ${{runner.os}}-go-
       - name: Test code
+        # for file.allow thing see https://vielmetti.typepad.com/logbook/2022/10/git-security-fixes-lead-to-fatal-transport-file-not-allowed-error-in-ci-systems-cve-2022-39253.html
         run: |
-          PARALLEL_TOTAL=${{ matrix.parallelism }} PARALLEL_INDEX=${{ matrix.index }} go test pkg/integration/deprecated/*.go
+          git config --global protocol.file.allow always && PARALLEL_TOTAL=${{ matrix.parallelism }} PARALLEL_INDEX=${{ matrix.index }} go test pkg/integration/deprecated/*.go
   integration-tests:
     runs-on: ubuntu-latest
     name: "Integration Tests"

pkg/commands/git_commands/working_tree.go

@@ -241,7 +241,7 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain
 	if cached {
 		cachedArg = " --cached"
 	}
-	if !node.GetIsTracked() && !node.GetHasStagedChanges() && !cached {
+	if !node.GetIsTracked() && !node.GetHasStagedChanges() && !cached && node.GetIsFile() {
 		trackedArg = "--no-index -- /dev/null"
 	}
 	if plain {

pkg/commands/models/file.go

@@ -29,6 +29,7 @@ type IFile interface {
 	GetIsTracked() bool
 	GetPath() string
 	GetPreviousPath() string
+	GetIsFile() bool
 }
 
 func (f *File) IsRename() bool {
@@ -92,6 +93,10 @@ func (f *File) GetPreviousPath() string {
 	return f.PreviousName
 }
 
+func (f *File) GetIsFile() bool {
+	return true
+}
+
 type StatusFields struct {
 	HasStagedChanges        bool
 	HasUnstagedChanges      bool

pkg/gui/filetree/file_node.go

@@ -42,6 +42,10 @@ func (self *FileNode) GetIsTracked() bool {
 	return self.SomeFile(func(file *models.File) bool { return file.Tracked })
 }
 
+func (self *FileNode) GetIsFile() bool {
+	return self.IsFile()
+}
+
 func (self *FileNode) GetPreviousPath() string {
 	if self.File == nil {
 		return ""

pkg/integration/components/runner.go

@@ -144,6 +144,7 @@ func createFixture(test *IntegrationTest, paths Paths) error {
 	shell.RunCommand(`git config user.email "CI@example.com"`)
 	shell.RunCommand(`git config user.name "CI"`)
 	shell.RunCommand(`git config commit.gpgSign false`)
+	shell.RunCommand(`git config protocol.file.allow always`)
 
 	test.SetupRepo(shell)
 

pkg/integration/components/shell.go

@@ -45,6 +45,15 @@ func (s *Shell) CreateFile(path string, content string) *Shell {
 	return s
 }
 
+func (s *Shell) CreateDir(path string) *Shell {
+	fullPath := filepath.Join(s.dir, path)
+	if err := os.MkdirAll(fullPath, 0o755); err != nil {
+		panic(fmt.Sprintf("error creating directory: %s\n%s", fullPath, err))
+	}
+
+	return s
+}
+
 func (s *Shell) UpdateFile(path string, content string) *Shell {
 	fullPath := filepath.Join(s.dir, path)
 	err := os.WriteFile(fullPath, []byte(content), 0o644)

pkg/integration/tests/file/dir_with_untracked_file.go

@@ -0,0 +1,32 @@
+package file
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DirWithUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs{
+	// notably, we currently _don't_ actually see the untracked file in the diff. Not sure how to get around that.
+	Description:  "When selecting a directory that contains an untracked file, we should not get an error",
+	ExtraCmdArgs: "",
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+		config.UserConfig.Gui.ShowFileTree = true
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateDir("dir")
+		shell.CreateFile("dir/file", "foo")
+		shell.GitAddAll()
+		shell.Commit("first commit")
+		shell.CreateFile("dir/untracked", "bar")
+		shell.UpdateFile("dir/file", "baz")
+	},
+	Run: func(shell *Shell, input *Input, assert *Assert, keys config.KeybindingConfig) {
+		assert.CommitCount(1)
+
+		assert.MatchMainViewContent(NotContains("error: Could not access"))
+		// we show baz because it's a modified file but we don't show bar because it's untracked
+		// (though it would be cool if we could show that too)
+		assert.MatchMainViewContent(Contains("baz"))
+	},
+})

pkg/integration/tests/tests.go

@@ -15,6 +15,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/integration/tests/cherry_pick"
 	"github.com/jesseduffield/lazygit/pkg/integration/tests/commit"
 	"github.com/jesseduffield/lazygit/pkg/integration/tests/custom_commands"
+	"github.com/jesseduffield/lazygit/pkg/integration/tests/file"
 	"github.com/jesseduffield/lazygit/pkg/integration/tests/interactive_rebase"
 	"github.com/jesseduffield/lazygit/pkg/integration/tests/stash"
 )
@@ -40,6 +41,7 @@ var tests = []*components.IntegrationTest{
 	cherry_pick.CherryPickConflicts,
 	custom_commands.FormPrompts,
 	stash.Rename,
+	file.DirWithUntrackedFile,
 }
 
 func GetTests() []*components.IntegrationTest {

test/integration/submoduleAdd/setup.sh

@@ -11,6 +11,11 @@ git init
 
 git config user.email "CI@example.com"
 git config user.name "CI"
+# see https://vielmetti.typepad.com/logbook/2022/10/git-security-fixes-lead-to-fatal-transport-file-not-allowed-error-in-ci-systems-cve-2022-39253.html
+# NOTE: I don't think this actually works if it's only applied to the repo.
+# On CI we set the global setting, but given it's a security concern I don't want
+# people to do that for their locals.
+git config protocol.file.allow always
 
 echo test1 > myfile1
 git add .

test/integration/submoduleEnter/setup.sh

@@ -26,5 +26,5 @@ cd ..
 git clone --bare ./repo other_repo
 cd repo
 
-git submodule add ../other_repo
+git -c protocol.file.allow=always submodule add ../other_repo
 git commit -am "add submodule"

test/integration/submoduleRemove/setup.sh

@@ -23,5 +23,5 @@ cd ..
 git clone --bare ./repo other_repo
 cd repo
 
-git submodule add ../other_repo
+git -c protocol.file.allow=always submodule add ../other_repo
 git commit -am "add submodule"

test/integration/submoduleReset/setup.sh

@@ -23,5 +23,5 @@ cd ..
 git clone --bare ./repo other_repo
 cd repo
 
-git submodule add ../other_repo
+git -c protocol.file.allow=always submodule add ../other_repo
 git commit -am "add submodule"

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/COMMIT_EDITMSG

@@ -0,0 +1 @@
+first commit

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/master

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/config

@@ -0,0 +1,12 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+[user]
+	email = CI@example.com
+	name = CI
+[commit]
+	gpgSign = false

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/info/exclude

@@ -0,0 +1,7 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~
+.DS_Store

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/logs/HEAD

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 763788c33660f53eecaecce8dae27c34e647ac57 CI <CI@example.com> 1668129994 +1100	commit (initial): first commit

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/logs/refs/heads/master

@@ -0,0 +1 @@
+0000000000000000000000000000000000000000 763788c33660f53eecaecce8dae27c34e647ac57 CI <CI@example.com> 1668129994 +1100	commit (initial): first commit

test/integration_new/file/dir_with_untracked_file/expected/repo/.git_keep/refs/heads/master

@@ -0,0 +1 @@
+763788c33660f53eecaecce8dae27c34e647ac57

test/integration_new/file/dir_with_untracked_file/expected/repo/dir/file

@@ -0,0 +1 @@
+baz

test/integration_new/file/dir_with_untracked_file/expected/repo/dir/untracked

@@ -0,0 +1 @@
+bar