Ubuntu
git package

stash: git does not recover untracked files during a pop/apply on conflict

Bug #2026319 reported by Matthew Ruffell on 2023-07-07

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	git (Ubuntu)	Fix Released	Undecided	Unassigned
	Jammy	Fix Released	Medium	Matthew Ruffell

Bug Description

[Impact]

On jammy, untracked files in a working directory are not recovered if you have previously stashed them, if there happens to be a merge conflict when it comes to pop/apply the stash during rebasing operations.

This is a problem because files users intentionally place in their working directories are lost, which could lead to user's losing their data or recent development effort.

The only workaround is to ensure that doing a pop/apply will not cause any merge conflicts, or to ensure that all of your files are added and committed.

[Testcase]

On a Jammy system:

$ git init
$ echo contents > original-file.txt
$ git add original-file.txt
$ git commit -m "Creating the file"

# Create a new file, modify an old one, stash
$ echo foo > new-file.txt
$ echo contents2 > original-file.txt
$ git stash push -u

# Modify the old file in a different way, commit
$ echo contents3 > original-file.txt
$ git commit -am "Altering the file"

# Apply the stash, see conflict, but what about the new file?
$ git stash pop
$ cat new-file.txt
cat: new-file.txt: No such file or directory
# "new-file.txt" is expected to exist, but is gone

There is a test package available in the following ppa:

https://launchpad.net/~mruffell/+archive/ubuntu/sf363767-test

When installed, "new-file.txt" exists and is able to be read.

$ git stash pop
Auto-merging original-file.txt
CONFLICT (content): Merge conflict in original-file.txt
On branch master
Unmerged paths:
(use "git restore --staged <file>..." to unstage)
(use "git add <file>..." to mark resolution)
both modified: original-file.txt

Untracked files:
(use "git add <file>..." to include in what will be committed)
new-file.txt

no changes added to commit (use "git add" and/or "git commit -a")
The stash entry is kept in case you need it again.
$ cat new-file.txt
foo

[Where problems could occur]

We are changing how git restores untracked files during a pop/apply operation during a stash. Currently, these untracked files are "lost" i.e. they vanish from the user's working directory. It currently is possible to get them back, but you need to dig around in orphaned commits, and since they no longer have any references anymore, even finding their commit hashes is difficult.

With the patch applied, user's untracked files will no longer vanish on stash pop/apply, and while I don't think user's would be surprised to find files they intentionally placed in the working directory safely restored, there is a change in behaviour that these files are now restored, instead of being "deleted" or lost forever. It is unlikely any users have built workflows that depend on untracked files being removed on a stash pop/apply, versus users who intentionally put untracked files in their working directory only to find they have lost them forever upon stashing.

If a regression were to occur, it could break worldwide development workflows, due to git being the most popular revision control system, and as such, any changes are high risk.

[Other info]

This was fixed upstream by the following commit in 2.35.0:

commit 71cade5a0b172ece7edf0ccb4420dd5b9a07e71a
Author: Elijah Newren <email address hidden>
Date: Tue, 4 Jan 2022 23:04:58 +0000
Subject: stash: do not return before restoring untracked files
Link: https://github.com/git/git/commit/71cade5a0b172ece7edf0ccb4420dd5b9a07e71a

The issue was introduced in commit bee8691 ("stash: restore untracked files AFTER restoring tracked files" in version 2.33.1, so Focal is not affected. Since the fix is in 2.35.0, kinetic is already fixed.

See original description

Tags:

CVE References

2022-39253

Matthew Ruffell (mruffell) on 2023-07-07

Changed in git (Ubuntu):
status:	New → Fix Released
Changed in git (Ubuntu Jammy):
status:	New → In Progress
importance:	Undecided → Medium
assignee:	nobody → Matthew Ruffell (mruffell)
tags:	added: jammy sts

Revision history for this message

Matthew Ruffell (mruffell) wrote on 2023-07-07:

Debdiff for git on jammy Edit (5.0 KiB, text/plain)

Attached is a debdiff for Jammy that solves the issue.

description:

updated

Revision history for this message

Richard Laager (rlaager) wrote on 2023-07-07:

I am able to confirm that the test package, 1:2.34.1-1ubuntu1.9+sf363767v20230707b1, fixes the issue.

I installed the test package, confirmed it was fixed, downgraded to git 1:2.34.1-1ubuntu1.9 git-man=1:2.34.1-1ubuntu1.9 (the normal jammy package), confirmed it was broken, upgraded again to the test package, and re-confirmed it was fixed.

I also reviewed the debdiff. Everything appears in order to me.

Heitor Alves de Siqueira (halves) on 2023-07-31

tags:

added: se-sponsor-halves
removed: sts

Revision history for this message

Robie Basak (racb) wrote on 2023-08-09: Please test proposed package

Hello Matthew, or anyone else affected,

Accepted git into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/git/1:2.34.1-1ubuntu1.10 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in git (Ubuntu Jammy):
status:	In Progress → Fix Committed
tags:	added: verification-needed verification-needed-jammy

Revision history for this message

Matthew Ruffell (mruffell) wrote on 2023-08-09:

Download full text (5.4 KiB)

Performing verification for Jammy. I installed git 2.34.1-1ubuntu1.9 from -updates:

$ sudo apt-cache policy git | grep Installed
Installed: 1:2.34.1-1ubuntu1.9

Running the reproducer:

$ git init
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint:
hint: git config --global init.defaultBranch <name>
hint:
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint:
hint: git branch -m <name>
Initialized empty Git repository in /home/ubuntu/test/.git/
$ git config --global user.email "<email address hidden>"
$ git config --global user.name "Your Name"
$ echo contents > original-file.txt
$ git add original-file.txt
$ git commit -m "Creating the file"
[master (root-commit) 0a02f82] Creating the file
1 file changed, 1 insertion(+)
create mode 100644 original-file.txt
$ echo foo > new-file.txt
$ echo contents2 > original-file.txt
$ git stash push -u
Saved working directory and index state WIP on master: 0a02f82 Creating the file
$ echo contents3 > original-file.txt
$ git commit -am "Altering the file"
[master 4533623] Altering the file
1 file changed, 1 insertion(+), 1 deletion(-)
$ git stash pop
Auto-merging original-file.txt
CONFLICT (content): Merge conflict in original-file.txt
The stash entry is kept in case you need it again.
$ cat new-file.txt
cat: new-file.txt: No such file or directory

Upon conflict we don't get a full explanation of what is outstanding such as
"unmerged paths" and "untracked files". The untracked file of new-file.txt is
missing and has not been restored to the working directory.

I then enabled -proposed and installed git 2.34.1-1ubuntu1.10 and re-ran the
reproducer:

$ sudo apt-cache policy git | grep Installed
Installed: 1:2.34.1-1ubuntu1.10

$ git init
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint:
hint: git config --global init.defaultBranch <name>
hint:
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint:
hint: git branch -m <name>
Initialized empty Git repository in /home/ubuntu/proposed/.git/
$ echo contents > original-file.txt
$ git add original-file.txt
$ git commit -m "Creating the file"
[master (root-commit) 0508716] Creating the file
1 file changed, 1 insertion(+)
create mode 100644 original-file.txt
$ echo foo > new-file.txt
$ echo contents2 > original-file.txt
$ git stash push -u
Saved working directory and index state WIP on master: 0508716 Creating the file
$ echo contents3 > original-file.txt
$ git commit -am "Altering the file"
[master b6d8396] Altering the file
1 file changed, 1 insertion(+), 1 deletion(-)
$ git stash pop
Auto-merging original-file.txt
CONFLICT (content): Merge conflict in original-file.txt
On branch mast...

Performing verification for Jammy. I installed git 2.34.1-1ubuntu1.9 from -updates:

$ sudo apt-cache policy git | grep Installed
  Installed: 1:2.34.1-1ubuntu1.9

Running the reproducer:

$ git init
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint: 
hint: 	git config --global init.defaultBranch <name>
hint: 
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint: 
hint: 	git branch -m <name>
Initialized empty Git repository in /home/ubuntu/test/.git/
$ git config --global user.email "you@example.com"
$ git config --global user.name "Your Name"
$ echo contents > original-file.txt
$ git add original-file.txt
$ git commit -m "Creating the file"
[master (root-commit) 0a02f82] Creating the file
 1 file changed, 1 insertion(+)
 create mode 100644 original-file.txt
$ echo foo > new-file.txt
$ echo contents2 > original-file.txt
$ git stash push -u
Saved working directory and index state WIP on master: 0a02f82 Creating the file
$ echo contents3 > original-file.txt
$ git commit -am "Altering the file"
[master 4533623] Altering the file
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git stash pop
Auto-merging original-file.txt
CONFLICT (content): Merge conflict in original-file.txt
The stash entry is kept in case you need it again.
$ cat new-file.txt
cat: new-file.txt: No such file or directory

I then enabled -proposed and installed git 2.34.1-1ubuntu1.10 and re-ran the
reproducer:

$ sudo apt-cache policy git | grep Installed
  Installed: 1:2.34.1-1ubuntu1.10

$ git init
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint: 
hint: 	git config --global init.defaultBranch <name>
hint: 
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint: 
hint: 	git branch -m <name>
Initialized empty Git repository in /home/ubuntu/proposed/.git/
$ echo contents > original-file.txt
$ git add original-file.txt
$ git commit -m "Creating the file"
[master (root-commit) 0508716] Creating the file
 1 file changed, 1 insertion(+)
 create mode 100644 original-file.txt
$ echo foo > new-file.txt
$ echo contents2 > original-file.txt
$ git stash push -u
Saved working directory and index state WIP on master: 0508716 Creating the file
$ echo contents3 > original-file.txt
$ git commit -am "Altering the file"
[master b6d8396] Altering the file
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git stash pop
Auto-merging original-file.txt
CONFLICT (content): Merge conflict in original-file.txt
On branch master
Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
	both modified:   original-file.txt

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	new-file.txt

no changes added to commit (use "git add" and/or "git commit -a")
The stash entry is kept in case you need it again.
$ cat new-file.txt
foo

This time we get the inline "git status" when the conflict arises, and the
untracked file is restored correctly, along with its contents.

The unit test also executes correctly, if we examine the buildlog:

https://launchpadlibrarian.net/681062898/buildlog_ubuntu-jammy-amd64.git_1%3A2.34.1-1ubuntu1.10_BUILDING.txt.gz

...
expecting success of 3903.111 'restore untracked files even when we hit conflicts': 
	git init restore_untracked_after_conflict &&
	(
		cd restore_untracked_after_conflict &&

echo hi >a &&
		echo there >b &&
		git add . &&
		git commit -m first &&
		echo hello >a &&
		echo something >c &&

git stash push --include-untracked &&

echo conflict >a &&
		git add a &&
		git commit -m second &&

test_must_fail git stash pop &&

test_path_is_file c
	)

Initialized empty Git repository in /<<PKGBUILDDIR>>/t/trash directory.t3903-stash/restore_untracked_after_conflict/.git/
[main (root-commit) 1dc9add] first
 Author: A U Thor <author@example.com>
 2 files changed, 2 insertions(+)
 create mode 100644 a
 create mode 100644 b
Saved working directory and index state WIP on main: 1dc9add first
[main 9e228ae] second
 Author: A U Thor <author@example.com>
 1 file changed, 1 insertion(+), 1 deletion(-)
Merging Updated upstream with Stashed changes
Merging:
virtual Updated upstream
virtual Stashed changes
found 1 common ancestor:
virtual c017b915e83a1d32cc014487285a63a4712e6201
Auto-merging a
CONFLICT (content): Merge conflict in a
On branch main
Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
	both modified:   a

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	c

no changes added to commit (use "git add" and/or "git commit -a")
The stash entry is kept in case you need it again.
ok 111 - restore untracked files even when we hit conflicts
...

The package in -proposed fixes the issue. Happy to mark as verified for Jammy.

tags:

added: verification-done-jammy
removed: verification-needed verification-needed-jammy

Revision history for this message

Richard Laager (rlaager) wrote on 2023-08-09:

I tested using the scenario from:
https://www.databasesandlife.com/git-stash-loses-untracked-files/

I agree that the version in jammy-proposed fixes the problem:
$ dpkg --status git | grep Version
Version: 1:2.34.1-1ubuntu1.10

This has already been tagged verification-done-jammy, so no changes there.

Revision history for this message

Ubuntu SRU Bot (ubuntu-sru-bot) wrote on 2023-08-09: Autopkgtest regression report (git/1:2.34.1-1ubuntu1.10)

All autopkgtests for the newly accepted git (1:2.34.1-1ubuntu1.10) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

git-remote-hg/1.0.3.2~ds-2 (amd64, arm64, armhf, ppc64el, s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#git

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Revision history for this message

Matthew Ruffell (mruffell) wrote on 2023-08-11:

The autopkgtest regression for git-remote-hg fails due to the patches git has to fix CVE-2022-39253, and is documented in bug 1993586, "Cannot add submodule using file transport".

Andreas had already forseen this in 2022:
https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586/comments/6
"The DEP8 tests need to be updated/changed, because they are all failing in git 2.38 in lunar currently, and I suspect the next non-security SRU for git will also trigger those in stable."

https://autopkgtest.ubuntu.com/results/autopkgtest-jammy/jammy/amd64/g/git-remote-hg/20230809_223307_203b4@/log.gz

...
521s Initialized empty Git repository in /tmp/autopkgtest.ZQXd3r/build.nXc/src/test/trash directory.main-push/tmp/gitrepo/.git/
522s Cloning into '/tmp/autopkgtest.ZQXd3r/build.nXc/src/test/trash directory.main-push/tmp/gitrepo/sub'...
522s fatal: transport 'file' not allowed
522s fatal: clone of '/tmp/autopkgtest.ZQXd3r/build.nXc/src/test/trash directory.main-push/tmp/sub' into submodule path '/tmp/autopkgtest.ZQXd3r/build.nXc/src/test/trash directory.main-push/tmp/gitrepo/sub' failed
522s not ok 52 - push with submodule
...

The testcase in git-remote-hg would ideally need to be changed, but since it would be a more or less no change upload apart from the testcase, it would likely get stuck in block-proposed due to not actually changing anything and introducing additional regression risk. I'll ask around to see if its worthwhile fixing the testcase or not.

Revision history for this message

Robie Basak (racb) wrote on 2023-08-16:

IIUC the autopkgtest failure flags should be cleared by a retry with migration-reference/0 so I've requested these. This will effectively disable git-remote-hg's autopkgtests from being consulted for any future git SRU, but on balance against the effort in individually fixing or disabling that particular test and holding up this SRU, this seems OK to me. I think it's OK to fix the test case though and to hold it in -proposed to help with future git SRUs, if someone wants to do that.

Revision history for this message

Robie Basak (racb) wrote on 2023-08-16:

> IIUC the autopkgtest failure flags should be cleared by a retry with migration-reference/0 so I've requested these.

This worked.

Revision history for this message

Launchpad Janitor (janitor) wrote on 2023-08-16:

#11

This bug was fixed in the package git - 1:2.34.1-1ubuntu1.10

---------------
git (1:2.34.1-1ubuntu1.10) jammy; urgency=medium

  * Fix issue where untracked files are not recovered during a stash
    pop/apply operation when a merge conflict is present. Untracked
    files are now correctly restored regardless if a conflict is
    present or not. (LP: #2026319)
    - d/p/lp-2026319-stash-do-not-return-before-restoring-untracked-files.patch

-- Matthew Ruffell <email address hidden> Fri, 07 Jul 2023 14:31:14 +1200

Changed in git (Ubuntu Jammy):
status:	Fix Committed → Fix Released

Revision history for this message

Robie Basak (racb) wrote on 2023-08-16: Update Released

#10

The verification of the Stable Release Update for git has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Patches

Debdiff for git on jammy Edit

Add patch

Remote bug watches

Bug watches keep track of this bug in other bug trackers.

Ubuntugit package

stash: git does not recover untracked files during a pop/apply on conflict

Bug Description

CVE References

Other bug subscribers

Patches

Remote bug watches

Ubuntu
git package