Commit 94eccd1
fix bare repo pull/checkout path handling bug
Our "git lfs checkout" and "git lfs pull" commands may both, at present,
be executed in a bare repository, although the former has no utility in
a bare repository, and the latter often performs no actions, but can be
used to fetch Git LFS objects in a bare repository.
The "git lfs checkout" and "git lfs pull" commands are the only commands
which make use of the methods of the singleCheckout structure in our
"commands" package, and in a subsequent commit we will update these methods
so they change the current working directory to the root of the current
working tree, so long as one exists.
Before we make these revisions, though, we first need to guarantee that
the singleCheckout structure's methods correctly handle the case where no
current work tree is defined, such as in a bare repository when the
GIT_WORK_TREE environment variable has not been set.
When no working tree is defined, the "git lfs pull" command should perform
no action other than fetching objects, since there is no work tree into
which the command should write any Git LFS file content.
For the same reason, the "git lfs checkout" should have no effect when
no work tree is defined, since the command's only purpose is to check
out Git LFS file content into a working tree.
Unfortunately, both the "git lfs checkout" and "git lfs pull" commands
may, under unusual circumstances, try to check out Git LFS files by
writing their object data into files either inside or outside a
bare repository.
In bare repositories, when the "git lfs checkout" and "git lfs pull"
commands try to determine whether to check out a Git LFS file into the
(non-existent) working tree, they incorrectly treat the path to a file
from the root of the repository as if it were instead an absolute path
starting from the root of the current filesystem. For instance, given
the path "foo/bar.bin" to a Git LFS file in a repository, the commands
will instead treat this path as if it were the path "/foo/bar.bin".
Normally, no file will exist at this location, so the "git lfs checkout"
and "git lfs pull" commands then check the Git index to try to determine
whether the user has staged the file for deletion. Since bare
repositories typically have no index, the commands will assume the user
has intentionally removed the file, and skip any further processing for
the file.
If the user has added an index entry for the file, though, the commands
will assume the file should be re-created in the (non-existent) working
tree with the content of the object referenced by the Git LFS pointer
stored in Git's version of the file. Taking the file's path from the
root of the repository as if it was an absolute path, the commands will
try to create any missing directories in that path, and then try to
either create a new file or truncate an existing one before writing
the Git LFS object content into the file.
An alternative sequence of events which leads to the same result may
occur in the extremely unlikely case that a file already exists at
the location specified by the incorrectly-determined absolute path,
and that the file contains a Git LFS pointer with the same object ID
as that of the given file in the repository. In other words, using
the same example file paths as above, this means a "/foo/bar.bin" file
would have to already exist and contain the same raw Git LFS pointer
data as the "foo/bar.bin" file in the Git repository. Should this
happen, the "git lfs checkout" and "git lfs pull" commands would assume
the file should be overwritten with the contents of the corresponding
Git LFS object.
Of course, even if the "git lfs checkout" and "git lfs pull" commands
try to create or overwrite a file at the path they are incorrectly
treating as an absolute path, the current user may not have sufficient
permissions to permit the necessary filesystem operations to complete.
Regardless, the Git LFS client should not try to read or write files
outside of the current repository unless specifically requested to do
so with an argument such as the --to option of the "git lfs checkout"
command.
In conjunction with our remediation of the vulnerability assigned the
identifier CVE-2025-26625, we therefore revise the "git lfs checkout"
and "git lfs pull" commands now to ensure they will never treat paths
relative to the root of the current repository as if they were absolute
filesystem paths.
We also adjust the "git lfs checkout" command so that it generates the
same error message as commands like "git lfs status" when no working
tree is defined, and exits immediately afterwards. This change will
make clear to our users why the "git lfs checkout" command has no effect
in a bare repository, while also simplifying our test requirements as we
do not have to verify the command's behaviour in a bare repository beyond
checking that it exits with the appropriate warning message.
The specific problem addressed in this commit is the result of the
joining an empty path, which signals the lack of a current working
tree, to a file's path from the root of the repository, and adding
a file separator character between the two strings. This occurs
within the Convert() method of the repoToCurrentPathConverter structure
type from our "lfs" package.
In a subsequent commit we will be able to remove the
repoToCurrentPathConverter structure and its methods entirely, when we
revise the "git lfs checkout" and "git lfs pull" commands to change
the current working directory to the root of the current working tree.
In this commit, however, we simply alter the commands so that they
never call the structure's Convert() method if no working tree exists.
First, we add a "hasWorkTree" element to the singleCheckout structure
type in our "commands" package, and when we initialize a new structure
in the newSingleCheckout() function, we set the "hasWorkTree" element's
value to "true" only if the LocalWorkingDir() method of the Configuration
structure type from our "config" package returns a non-empty path.
The LocalWorkingDir() method returns the absolute path to the root of
the current working tree, or an empty path if no working tree is defined,
as determined by the GitAndRootDirs() function in our "git" package.
The GitAndRootDirs() function runs the "git rev-parse" command with the
--show-toplevel option, and then interprets that command's output and
exit code so that if no working tree is defined, an empty path is
returned instead of a path to the work tree's root directory.
Second, we update the Run() method of the singleCheckout structure
so that it returns immediately unless the "hasWorkTree" element is
set to a "true" value, meaning a work tree exists and it is safe to
create and write files within that directory tree.
To verify these changes work as we expect, we introduce a new "pull: bare
repository" test to our t/t-pull.sh test script, and in this test we
specifically add a Git LFS pointer file to the test repository at a path
that, if treated as an absolute path instead of a path from the root of
the repository, could be created by the current test process. After the
test clones the repository, it adds this file's path to the index, runs
the "git lfs pull" command, and then checks that no file is created either
inside the bare repository or, most importantly, outside the repository.
(The test also ensures that a Git LFS filter attribute is defined in
the "$GIT_DIR/info/attributes" file, which guarantees that regardless
of which Git version is installed, the "git lfs pull" command will find
our new Git LFS pointer file in the repository's contents and process it.
We describe the issues pertaining to the need to use a local Git
attributes file instead of a ".gitattributes" file further below.)
Without our changes to the singleCheckout structure and its methods
in this commit, the revised "pull: bare repository" test will fail, so
we can be confident that it validates that our remediation is effective.
As for the "git lfs checkout" command, we alter its main checkoutCommand()
function so that after calling the setupRepository() function, the
checkoutCommand() function checks whether a path to the current working
tree has been found, and if not, outputs a warning message and stops
execution of the command. This new check is modelled on that performed
by the requireWorkingCopy() function, but causes the command to return a
zero (i.e., successful) exit code rather than a non-zero one.
Our new check relies on the functions invoked by the setupRepository()
function to have already called the GitAndRootDirs() function in our
"git" package. That function runs the "git rev-parse" command with the
--show-toplevel option, and then interprets the command's output and exit
code so that if no current work tree is present, an empty path will be
returned by the LocalWorkingDir() method of our "config" package's
Configuration structure instead of a path to the work tree's root
directory.
It would be more straightforward for us to revise the checkoutCommand()
function to simply call the setupWorkingCopy() function rather than the
setupRepository() function, because the setupWorkingCopy() function calls
the requireWorkingCopy() function and so would enforce the presence of
a working tree in the same manner as we employ in other commands such as
the "git lfs status" and "git lfs track" commands.
However, this implementation would result in a backwards-incompatible
change to the behaviour the "git lfs checkout" command when it is run in a
bare repository, which could result in the unexpected failure of automated
CI jobs, for instance. Although the use of the "git lfs checkout" command
in a bare repository has no purpose, we defer the simpler implementation
to a future release, and for now ensure that the command still returns
a zero exit code when run in a bare repository.
We do, though, update our git-lfs-checkout(1) manual page to clarify that
the command requires a working tree, and that in the future the command
may exit with an error when run in a bare repository. We also add a new
"checkout: bare repository" test to our t/t-checkout.sh test script, which
just verifies that the command generates the expected error message and
returns a zero exit code when it is run in a bare repository.
Both the "git lfs checkout" and "git lfs pull" commands currently exhibit
the erroneous behaviour addressed by this commit because the singleCheckout
structure's Run() method relies on the Convert() method of the
repoToCurrentPathConverter structure type to rewrite file paths
relative to the root of the repository into paths relative to the
current working directory, and this method returns invalid paths when
no working tree is defined, as is the case in a bare repository.
The Run() method is executed, either directly or indirectly, for
each Git LFS pointer file path found by the ScanLFSFiles() method of
the GitScanner structure in our "lfs" package. This method retrieves
a list of files from Git, and for each one that corresponds to a Git
LFS pointer, the method invokes an anonymous function which in turn
causes the Run() method to be performed.
In the case of the "git lfs pull" command, if a local copy of the object
associated with a Git LFS pointer is found, the Run() method is invoked
directly within the anonymous function, and otherwise it is invoked by
a goroutine for each object whose data is successfully retrieved from
the Git LFS remote by the transfer queue.
In the case of the "git lfs checkout" command, the anonymous function
called by the ScanLFSFiles() method appends each Git LFS pointer to a
slice, and then the Run() method is invoked for each pointer in the
slice after the scan through the list of files is complete.
To retrieve a list of files from Git, the runScanLFSFiles() function,
which is called by the ScanLFSFiles() method, uses one of two Git
commands. If the installed version of Git is 2.42.0 or higher, the
"git ls-files" command is executed, and otherwise the "git ls-tree"
command is used. This difference accounts for one of the reasons
why the "git lfs pull" command, in particular, may perform no action
when run within a bare repository.
Specifically, as noted in issue git-lfs#6004, the "git ls-files" command lists
the files in the Git index, while the "git ls-tree" command lists the
files in the Git tree associated with a given reference, which in the
case of our "git lfs checkout" and "git lfs pull" commands is always
the current "HEAD" symbolic reference.
If the installed version of Git is older than v2.42.0, when our commands
run the "git ls-tree" command they will receive a list of files from
the Git tree associated with the "HEAD" reference, and will process
any Git LFS pointers found in that list. (Note that pointer files
will be processed even if they no longer match any Git LFS filter
attributes; for instance, if there are no ".gitattributes" files in
the index or in the Git tree associated with the "HEAD" reference,
and no local Git attributes files.)
If the installed version of Git is at least v2.42.0, our commands
run the "git ls-files" command instead of the "git ls-tree" command.
For two separate reasons, in a bare repository the "git ls-files"
command will often return an empty list, so our "git lfs checkout"
and "git lfs pull" commands will take no further action.
The more obvious reason is that by default, Git creates bare
repositories without an index, so unless the user has explicitly
added entries to the index for Git LFS pointer files, no results
will be returned by the "git ls-files" command.
The less obvious reason is due to the "attr:filter=lfs" pathspec
our commands pass to the "git ls-files" command, which causes the
command to only return paths for files which match a Git LFS filter
attribute definition. However, in a bare repository Git's internal
read_attr() function by default ignores all ".gitattributes" files
found in either the index or the tree associated with the "HEAD"
reference:
https://github.com/git/git/blob/v2.50.1/attr.c#L851-L867
Since there is no working tree in a bare repository, this means all
".gitattributes" files are ignored by default, and because users
typically define Git LFS attributes in those files, the "git ls-files"
command will not match any files even if entries for Git LFS pointer
files have been added to the index. Users would have to specifically
set the GIT_ATTR_SOURCE environment variable to a reference like "HEAD"
or add Git LFS filter attributes to a local Git attributes file such
as the "$GIT_DIR/info/attributes" file in order for the "git ls-files"
command to match pointer files in the index to the "attr:filter=lfs"
pathspec and return a non-empty list.
Regardless of the source, though, if Git LFS pointers are identified from
the list of files returned by Git, the "git lfs pull" command will fetch
the objects referenced by those pointers unless the objects already exist
in the local storage directories under "lfs/objects". (Note that in a
bare repository, the usual leading ".git" directory is not necessary.)
As objects are fetched by the transfer queue, the separate goroutine
started by the "git lfs pull" command passes their pointer data to the
Run() method of the singleCheckout structure, one pointer at a time.
In a "git lfs checkout" command, by contrast, no objects are fetched,
and the Run() method is instead invoked directly by the command's main
function for each Git LFS pointer file path collected during the
execution of the ScanLFSFiles() method.
As described above, the Run() method begins by converting the file path
of the Git LFS pointer provided in its "p" parameter into a file path
relative to the current working directory using the Convert() method of
the repoToCurrentPathConverter structure type.
We initialize a structure of that type in the newSingleCheckout()
function by calling the NewRepoToCurrentPathConverter() function.
That function uses an internal function named pathConverterArgs() to set
the new structure's "repoDir" element to the file path returned by the
LocalWorkingDir() method of the Configuration structure type, which as
mentioned above will be an empty path if no current work tree is defined.
When the repoToCurrentPathConverter structure type's Convert() method
is called, it first joins the structure's "repoDir" element to the
file path provided in the method's "p" parameter using a local wrapper
function around the Join() function from the Go standard library's
"strings" package, rather than the Join() function from the
"path/filepath" package. (This change was made in commit
fd69029 of PR git-lfs#2875, presumably to
make more efficient the handling of file paths which we expect to
always be defined.)
In a bare repository, however, the "repoDir" element contains an empty
path, so the result of joining it with a file path relative to the root
of the repository using the Join() function from the "strings" package
is the same file path but with a leading "/" character prepended to it,
in effect creating an invalid absolute path from the root of the
filesystem. Note that if the Join() function from the "path/filepath"
package was used instead, it ignores empty parameters, so the file path
would be returned unchanged.
The Convert() method then passes this invalid absolute path to the
Rel() function of the "path/filepath" package of the Go standard library,
along with the absolute path to the current working directory. We expect
this call to return a relative path from the current working directory
to the location within the current Git work tree where a file should
be created or updated with the contents of a Git LFS object.
In a bare repository, though, what is returned by the Convert() method
to the Run() method is a relative path from the current working directory
to a location constructed by treating a Git LFS pointer's path within
the repository as if it was a path descending from the root of the
current filesystem. For instance, given the path "foo/bar" of a
Git LFS pointer within the repository, and current working directory
of "/path/to/bare/repo", the Convert() method would return the path
"../../../../foo/bar".
After this path is returned to the Run() method, it is passed to the
DecodePointerFromFile() function in our "lfs" package, which checks
whether a file exists at the given location, and if so, reads it and
checks whether it contains a valid Git LFS pointer.
In the large majority of cases, of course, files will not exist in
the locations identifed by the invalid paths that the Convert() method
generates when the "git lfs checkout" or "git lfs pull" commands are
executed in a bare repository. Hence the DecodePointerFromFile()
function will return an error which the IsNotExist() function of the
"os" package considers equivalent to an ErrNotExist error. The Run()
method will then execute a "git diff-index" command to determine whether
the user has intentionally removed the file from the Git index, and
will pass the original file path (the one relative to the root of the
repository) to that command.
If the installed version of Git is older than v2.42.0, and the bare
repository has no index, as is normally the case in such repositories,
the "git diff-index" command's output will indicate that the file does
not exist in the index and so the Run() method will return without
taking further action.
On the other hand, if the installed version of Git is 2.42.0 or higher,
then the index must include an entry for the original file path (the
one relative to the root of the repository), since otherwise the
"git ls-files" command would not have listed the file at all and the
Run() method would never have been called. Thus the "git diff-index"
command will also list the file as present in the index, and so the
Run() method will proceed on the assumption that the file is just
missing in the (non-existent) working tree and should be created, even
though there is no actual work tree.
Even if a version of Git older than 2.42.0 is installed, though,
the user may have created an index entry for the file, in which case
the Run() method will likewise proceed because the "git diff-index"
command's output will indicate that the file is present in the index.
It is also possible, although extremely unlikely, that the
DecodePointerFromFile() function finds a file at the incorrectly-
generated path it was given, and is able to open it and parse it as a
valid Git LFS pointer. The Run() method will then check to see if the
pointer's ID matches that of the pointer under consideration. If it
does not, the method will return without taking action, but if it does,
it will proceed on the assumption that the pointer file should be
overwritten with the contents of the associated object file.
In summary, in a bare repository the singleCheckout structure's Run()
method will only proceed under one of two conditions: either a Git index
entry exists for the file path under consideration, which is unlikely
to be the case in a bare repository since the index is typically empty,
or a Git LFS pointer file with the expected object ID happens to exist
at the absolute path derived by prepending a file separator to the
file's path within the repository, which is even more unlikely.
Should one of these circumstances occur, though, the Run() method
will invoke the RunToPath() method of the singleCheckout structure,
which will in turn call the SmudgeToFile() method of the GitFilter
structure in our "lfs" package. That method will attempt to create
or truncate a file at the incorrect path, and then write the contents
of a Git LFS object into the file.
With the changes in this commit, however, this incorrect behaviour
should no longer occur under any circumstances.1 parent 444cf54 commit 94eccd1
File tree
6 files changed
+176
-0
lines changed- commands
- docs/man
- t
6 files changed
+176
-0
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
27 | 36 | | |
28 | 37 | | |
29 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
36 | 37 | | |
37 | 38 | | |
38 | 39 | | |
| |||
49 | 50 | | |
50 | 51 | | |
51 | 52 | | |
| 53 | + | |
52 | 54 | | |
53 | 55 | | |
54 | 56 | | |
| |||
66 | 68 | | |
67 | 69 | | |
68 | 70 | | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
69 | 75 | | |
70 | 76 | | |
71 | 77 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
53 | 56 | | |
54 | 57 | | |
55 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
39 | 49 | | |
40 | 50 | | |
41 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
961 | 961 | | |
962 | 962 | | |
963 | 963 | | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
964 | 981 | | |
965 | 982 | | |
966 | 983 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1117 | 1117 | | |
1118 | 1118 | | |
1119 | 1119 | | |
| 1120 | + | |
| 1121 | + | |
| 1122 | + | |
| 1123 | + | |
| 1124 | + | |
| 1125 | + | |
| 1126 | + | |
| 1127 | + | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
| 1140 | + | |
| 1141 | + | |
| 1142 | + | |
| 1143 | + | |
| 1144 | + | |
| 1145 | + | |
| 1146 | + | |
| 1147 | + | |
| 1148 | + | |
| 1149 | + | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
| 1157 | + | |
| 1158 | + | |
| 1159 | + | |
| 1160 | + | |
| 1161 | + | |
| 1162 | + | |
| 1163 | + | |
| 1164 | + | |
| 1165 | + | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
| 1186 | + | |
| 1187 | + | |
| 1188 | + | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
| 1196 | + | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
| 1200 | + | |
| 1201 | + | |
| 1202 | + | |
| 1203 | + | |
| 1204 | + | |
| 1205 | + | |
| 1206 | + | |
| 1207 | + | |
| 1208 | + | |
| 1209 | + | |
| 1210 | + | |
| 1211 | + | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
| 1219 | + | |
| 1220 | + | |
| 1221 | + | |
| 1222 | + | |
| 1223 | + | |
| 1224 | + | |
| 1225 | + | |
| 1226 | + | |
| 1227 | + | |
| 1228 | + | |
| 1229 | + | |
| 1230 | + | |
| 1231 | + | |
| 1232 | + | |
| 1233 | + | |
| 1234 | + | |
| 1235 | + | |
| 1236 | + | |
| 1237 | + | |
| 1238 | + | |
| 1239 | + | |
| 1240 | + | |
| 1241 | + | |
| 1242 | + | |
| 1243 | + | |
| 1244 | + | |
| 1245 | + | |
| 1246 | + | |
| 1247 | + | |
| 1248 | + | |
| 1249 | + | |
| 1250 | + | |
1120 | 1251 | | |
1121 | 1252 | | |
1122 | 1253 | | |
| |||
0 commit comments