已开启
Fix CVE-2025-26625 #27
wang-qi创建于 6月4日
Fix CVE-2025-26625 #27
已开启
共 5 个文件变更+2963-2
| @@ -0,0 +1,1835 @@ | |||
| 1 | +From 0cffe93176b870055c9dadbb3cc9a4a440e98396 Mon Sep 17 00:00:00 2001 | ||
| 2 | +From: Chris Darroch <chrisd8088@github.com> | ||
| 3 | +Date: Sun, 24 Aug 2025 21:17:41 -0700 | ||
| 4 | +Subject: [PATCH] check for dir/symlink conflicts on checkout/pull | ||
| 5 | + | ||
| 6 | +Our "git lfs checkout" and "git lfs pull" commands, at present, | ||
| 7 | +follow any extant symbolic links when they populate the current working | ||
| 8 | +tree with files containing the content of Git LFS objects, even if | ||
| 9 | +the symbolic links point to locations outside of the working tree. | ||
| 10 | +This vulnerability has been assigned the identifier CVE-2025-26625. | ||
| 11 | + | ||
| 12 | +In previous commits we partially addressed this vulnerability by | ||
| 13 | +ensuring that the "git lfs checkout" and "git lfs pull" commands remove | ||
| 14 | +any file or symbolic link which already exists at the location where | ||
| 15 | +they intend to write the contents of a Git LFS file, and by checking for | ||
| 16 | +symbolic links at these locations first in the DecodePointerFromBlob() | ||
| 17 | +function of the "lfs" package. | ||
| 18 | + | ||
| 19 | +However, these changes still allow for the possibility that a symbolic | ||
| 20 | +link exists in place of a directory in the path between the root of | ||
| 21 | +the working tree and the location where the commands intend to create | ||
| 22 | +a file. At present, the "git lfs checkout" and "git lfs pull" commands | ||
| 23 | +will not detect such links, and so may be induced to write to a location | ||
| 24 | +outside of the working tree. | ||
| 25 | + | ||
| 26 | +To address this issue, revise the "git lfs checkout" and "git lfs pull" | ||
| 27 | +commands so they check each path component from the root of the working | ||
| 28 | +tree to a Git LFS file. If any are missing, a directory is created, and | ||
| 29 | +if any already exist but are not directories, the commands report an | ||
| 30 | +error and do not try to create the Git LFS file or write to it. | ||
| 31 | + | ||
| 32 | +In our implementation of these checks, we adopt a similar approach to | ||
| 33 | +the one used by Git, which also tries to avoid accidentally traversing | ||
| 34 | +symbolic links when updating the files in a working tree. For | ||
| 35 | +performance and compatibility reasons, though, Git does not try to | ||
| 36 | +completely eliminate all TOCTOU (time-of-check/time-of-use) races | ||
| 37 | +involving symbolic links. | ||
| 38 | + | ||
| 39 | +Likewise, we do not aim to prevent every possible race which might | ||
| 40 | +allow the Git LFS client to unintentionally write through a symbolic | ||
| 41 | +link. Instead, we try to limit the chances of this occurring as far as | ||
| 42 | +we reasonably can, while avoiding significant performance penalties. | ||
| 43 | + | ||
| 44 | +One difference between our approach and that taken by Git is that | ||
| 45 | +when the we check whether a directory exists and find something other | ||
| 46 | +than a directory, we do not try to remove it. This design choice | ||
| 47 | +retains compatibility with the legacy behaviour of the Git LFS client, | ||
| 48 | +which simply invoked the MkdirAll() function of the "os" package in | ||
| 49 | +the Go standard library. That function returns an error if any of the | ||
| 50 | +directories in the given path do not already exist and cannot be created, | ||
| 51 | +and the "git lfs checkout" and "git lfs pull" commands would just report | ||
| 52 | +that error rather than attempt to resolve it by removing anything. | ||
| 53 | + | ||
| 54 | +Another difference between the way Git checks for directory path | ||
| 55 | +conflicts and the implementation we introduce in this commit is that | ||
| 56 | +Git retains the results of its checks in a simple single-entry cache | ||
| 57 | +while we repeat our checks for each new Git LFS file we process. We | ||
| 58 | +can add caching logic in the future if we find it valuable, but we | ||
| 59 | +would require a more complex and thread-safe cache than Git's due to | ||
| 60 | +our use of multiple goroutines in the "git lfs pull" command, and initial | ||
| 61 | +testing indicates that the performance gains would be relatively limited. | ||
| 62 | + | ||
| 63 | +When the "git checkout" command runs, the checkout_entry_ca() function | ||
| 64 | +performs the necessary changes in the working tree in order to be able | ||
| 65 | +to write a copy of a given file at its expected location. This function | ||
| 66 | +invokes the create_directories() function to ensure that all of the | ||
| 67 | +directories between the root of the working tree and the file are present | ||
| 68 | +before the file is created. If the create_directories() function detects | ||
| 69 | +a conflict in place of any directory, such as a file or symbolic link, | ||
| 70 | +it tries to remove the conflicting entry and then create a new directory | ||
| 71 | +in its place. | ||
| 72 | + | ||
| 73 | +As noted above, though, Git does not re-check every directory entry in | ||
| 74 | +a file's path in all cases, and also does not try to avoid TOCTOU races | ||
| 75 | +in the checks it does perform. The create_directories() function relies | ||
| 76 | +on the has_dirs_only_path() function to report whether a path consists | ||
| 77 | +of only directories, and that function ultimately invokes the | ||
| 78 | +lstat_cache_matchlen() function to determine whether Git believes this | ||
| 79 | +to be the case or not: | ||
| 80 | + | ||
| 81 | + https://github.com/git/git/blob/v2.50.1/entry.c#L582 | ||
| 82 | + https://github.com/git/git/blob/v2.50.1/entry.c#L41-L42 | ||
| 83 | + https://github.com/git/git/blob/v2.50.1/symlinks.c#L257 | ||
| 84 | + https://github.com/git/git/blob/v2.50.1/symlinks.c#L276-L278 | ||
| 85 | + https://github.com/git/git/blob/v2.50.1/symlinks.c#L199-L200 | ||
| 86 | + https://github.com/git/git/blob/v2.50.1/symlinks.c#L63-L193 | ||
| 87 | + | ||
| 88 | +The lstat_cache_matchlen() function accepts a path from the root of the | ||
| 89 | +repository as its "name" parameter, and for each component of the path | ||
| 90 | +for which the function does not have any cached information, it uses | ||
| 91 | +the lstat(2) POSIX system call to test whether that path component exists | ||
| 92 | +and if it is a directory or not. The final result is then retained in | ||
| 93 | +the function's single-entry cache. | ||
| 94 | + | ||
| 95 | +The use of a cache with only a single entry is viable for Git because in | ||
| 96 | +almost all cases, it processes files in sorted order. Thus it can make | ||
| 97 | +use of the cached lstat(2) information about the directory "abc" from the | ||
| 98 | +path "abc/bar.txt" when checking the path of "abc/foo.txt", for instance. | ||
| 99 | + | ||
| 100 | +The use of cache in this function, though, is one of the reasons Git is | ||
| 101 | +not immune to TOCTOU races involving symbolic links. If a directory is | ||
| 102 | +replaced with a symbolic link after the lstat_cache_matchlen() function | ||
| 103 | +has checked the path, the lstat_cache_matchlen() function will assume | ||
| 104 | +another file with the same leading path components can be created without | ||
| 105 | +re-checking for symbolic links, and Git will traverse the new symbolic | ||
| 106 | +link when writing the file, even if it leads to a location outside of | ||
| 107 | +the working tree. | ||
| 108 | + | ||
| 109 | +Git also has to be careful to reset the cache whenever it removes any | ||
| 110 | +of the directories in the cached path, as may occur when Git processes | ||
| 111 | +files that are not in sorted order and their paths conflict with each | ||
| 112 | +other due to case-insensitivity or case-folding on the part of the | ||
| 113 | +filesystem. This type of situation was described in commit | ||
| 114 | +git/git@684dd4c2b414bcf648505e74498a608f28de4592, which added logic to | ||
| 115 | +ensure the cache is cleared under these conditions as part of the | ||
| 116 | +remediation for the vulnerability identified as CVE-2021-21300. | ||
| 117 | + | ||
| 118 | +Further, Git would also need to consistently use the openat(2) family of | ||
| 119 | +POSIX system calls in conjunction with their O_NOFOLLOW flags, or their | ||
| 120 | +equivalent on Windows, in order to guarantee that a given path consists | ||
| 121 | +solely of directories and no symbolic links. As noted in commit | ||
| 122 | +git/git@f4aa8c8bb11dae6e769cd930565173808cbb69c8 in relation to the | ||
| 123 | +vulnerability identified as CVE-2024-32004, on Windows this type of | ||
| 124 | +implementation would require the use of the relatively expensive | ||
| 125 | +NtCreateFile() system call (and its FILE_OPEN_REPARSE_POINT flag): | ||
| 126 | + | ||
| 127 | + https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html | ||
| 128 | + https://pubs.opengroup.org/onlinepubs/9699919799/functions/fstatat.html | ||
| 129 | + https://www.man7.org/linux/man-pages/man2/openat.2.html | ||
| 130 | + https://www.man7.org/linux/man-pages/man2/stat.2.html | ||
| 131 | + https://learn.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntcreatefile | ||
| 132 | + | ||
| 133 | +Beginning with version 1.24.0, Go introduced a Root structure type in | ||
| 134 | +the "os" package of the standard library, with a set of methods which | ||
| 135 | +explicitly enforces file path boundaries, using the openat(2) family | ||
| 136 | +of system calls where they are available, and the NtCreateFile() system | ||
| 137 | +call on Windows. Go v1.25.0 expanded the set of methods in the Root | ||
| 138 | +type, and in particular added a MkdirAll() method which mirrors | ||
| 139 | +the regular MkdirAll() function in the "os" package, but checks that | ||
| 140 | +none of the components in a path are symbolic links to locations | ||
| 141 | +outside a given initial "root" path. The development of the Root | ||
| 142 | +type and its API was tracked in golang/go#67002. | ||
| 143 | + | ||
| 144 | +One minor caveat with the MkdirAll() method of the Root structure type | ||
| 145 | +is that it allows symbolic links to exist in a path, so long as they do | ||
| 146 | +not resolve to location outside the path that was initially passed to | ||
| 147 | +the OpenRoot() function. We would prefer to avoid these types of "local" | ||
| 148 | +symbolic links as well when they conflict with a directory we expect | ||
| 149 | +to exist, so the Root type's MkdirAll() method would not suffice for | ||
| 150 | +our purposes. | ||
| 151 | + | ||
| 152 | +A more important challenge with the Root structure type is that | ||
| 153 | +consistent use of its methods would result in a noticeable increase in | ||
| 154 | +the execution time of our commands when processing even moderate numbers | ||
| 155 | +of Git LFS files. Each of the type's methods, including Lstat(), | ||
| 156 | +Mkdir(), OpenFile(), and Remove(), traverses the directories in its | ||
| 157 | +path parameter and checks that none are symbolic links to locations | ||
| 158 | +outside the path initially passed to the OpenRoot() function. Each | ||
| 159 | +method's cost therefore scales with the number of directories in its | ||
| 160 | +path parameter; i.e., given "m" method calls and "n" directories in a path, | ||
| 161 | +the number of system calls scales as O(m*n). For this reason, the Go | ||
| 162 | +documentation states that: | ||
| 163 | + | ||
| 164 | + "Root operations on filenames containing many directory components can | ||
| 165 | + be much more expensive than the equivalent non-Root operation." | ||
| 166 | + | ||
| 167 | + https://go.dev/blog/osroot#performance | ||
| 168 | + | ||
| 169 | +We verified this performance penalty in tests of a modified "git lfs | ||
| 170 | +checkout" command which checks for symbolic links in each Git LFS file's | ||
| 171 | +path within the repository by calling the methods of the Root structure | ||
| 172 | +type. We also tested the implementation from this commit, and we report | ||
| 173 | +those results in more detail below. In brief, even without a cache like | ||
| 174 | +the one in Git's lstat_cache_matchlen() function, the technique we | ||
| 175 | +introduce in this commit adds a modest overhead, while the use of the | ||
| 176 | +Root structure's methods significantly increased the command's runtime. | ||
| 177 | + | ||
| 178 | +Our preferred technique relies on several enhancements we made in | ||
| 179 | +previous commits to the "git lfs checkout" and "git lfs pull" commands. | ||
| 180 | +These commands retrieve a list of Git LFS pointer files from the | ||
| 181 | +ScanLFSFiles() method of the GitScanner structure type in our "lfs" | ||
| 182 | +package, and for each file, invoke the Run() method of the | ||
| 183 | +singleCheckout structure type in our "commands" package. The Run() | ||
| 184 | +method then determines whether or not to write the contents of the | ||
| 185 | +object referenced by the pointer into a file in the working tree at | ||
| 186 | +the appropriate path. | ||
| 187 | + | ||
| 188 | +In prior commits we revised the newSingleCheckout() function to verify | ||
| 189 | +whether a working tree exists when it initializes a new singleCheckout | ||
| 190 | +structure, and if a tree is present, to change the current working | ||
| 191 | +directory to the root of the tree. We also adjusted the Run() method | ||
| 192 | +so that it returns immediately without taking action if no working | ||
| 193 | +tree was found by the newSingleCheckout() function. | ||
| 194 | + | ||
| 195 | +We now introduce a new DirWalker structure type in our "tools" package, | ||
| 196 | +with Walk() and WalkAndCreate() methods which check that each component | ||
| 197 | +of a given path is a directory, and return an error if a conflict is | ||
| 198 | +found. If a directory is missing, the Walk() method will return an | ||
| 199 | +error, while the WalkAndCreate() method will try to create the directory. | ||
| 200 | +Both methods are simple wrappers around the internal walk() method, | ||
| 201 | +whose "create" parameter indicates whether the method should try to | ||
| 202 | +create missing directories or not. | ||
| 203 | + | ||
| 204 | +To initialize a DirWalker structure we define a NewDirWalkerForFile() | ||
| 205 | +function, which requires three parameters. The first is an initial | ||
| 206 | +parent path which should be specified as a path relative to the current | ||
| 207 | +working directory, and which is stored in the "parentPath" element of | ||
| 208 | +the new DirWalker structure. The second parameter is a file path which | ||
| 209 | +should be specified as a path relative to the parent path. If the parent | ||
| 210 | +path is empty, the file path is understood to be relative to the current | ||
| 211 | +working directory. The third parameter must be a structure with a | ||
| 212 | +RepositoryPermissions() method which conforms to the | ||
| 213 | +repositoryPermissionFetcher interface type from our "tools" package. | ||
| 214 | + | ||
| 215 | +The NewDirWalkerForFile() function removes the final filename path | ||
| 216 | +segment from its second "filePath" parameter in order to populate the | ||
| 217 | +new DirWalker structure's "path" element with leading directories in the | ||
| 218 | +file's path, if any. If the "filePath" parameter contains a bare | ||
| 219 | +filename, because the file resides at the root of the repository, then | ||
| 220 | +the "path" element is set to an empty path. Note that we do not use the | ||
| 221 | +Dir() function from the "path/filepath" package in the Go standard | ||
| 222 | +library to remove the filename from the "filePath" parameter because | ||
| 223 | +that function returns a "." path when a path has no leading directory | ||
| 224 | +components, and because it replaces the "/" separator with the "\" | ||
| 225 | +separator on Windows, which we do not want to do in this context. | ||
| 226 | + | ||
| 227 | +When the DirWalker structure's walk() method is called, it assumes that | ||
| 228 | +the path identified by the structure's "parentPath" element exists within | ||
| 229 | +the current working directory, and then checks each of the directories | ||
| 230 | +in the "path" element until either an error is returned or all the | ||
| 231 | +directories have been checked. If a directory does not exist, the | ||
| 232 | +walk() method returns an ErrNotExist error unless the "create" | ||
| 233 | +parameter is set to "true", in which case the walk() method will try | ||
| 234 | +to create the missing directory. If a conflict is found in the place | ||
| 235 | +of a directory, such as a pre-existing file or symbolic link with the | ||
| 236 | +same name, then the walk() method returns a custom errNotDir error. | ||
| 237 | + | ||
| 238 | +Assuming that the newSingleCheckout() function found an extant working | ||
| 239 | +tree and was able to change the current working directory to the root | ||
| 240 | +of the tree, the singleCheckout structure's Run() method creates a new | ||
| 241 | +DirWalker structure and calls its Walk() method to determine which | ||
| 242 | +directories in the given Git LFS pointer file's path already exist, | ||
| 243 | +without at first trying to create any new directories. Since the current | ||
| 244 | +working directory is the root of the work tree, the Run() method passes | ||
| 245 | +an empty path to the NewDirWalkerForFile() function as its "parentPath" | ||
| 246 | +parameter, and the pointer file's path as the "filePath" parameter. | ||
| 247 | + | ||
| 248 | +The pointer file paths processed by the Run() method are guaranteed to | ||
| 249 | +be those supplied by Git, since they are the paths returned by the | ||
| 250 | +ScanLFSFiles() method of the GitScanner structure, which reads the | ||
| 251 | +paths from the output of either a "git ls-files" or "git ls-tree" | ||
| 252 | +command. As such, we expect these paths to always use forward slash | ||
| 253 | +characters as separators, to always be relative paths and not absolute | ||
| 254 | +paths, and to never contain empty path components or "." or ".." path | ||
| 255 | +components. For safety, the DirWalker structure's walk() method rejects | ||
| 256 | +any path which contains any of these path components and returns an | ||
| 257 | +error in such a case. | ||
| 258 | + | ||
| 259 | +If the call to the Walk() method returns an error, the Run() method | ||
| 260 | +checks whether the error was due to a missing directory or some other | ||
| 261 | +issue. If an ErrNotExist error from the "os" package was returned, this | ||
| 262 | +indicates that at least one directory in the current Git LFS pointer | ||
| 263 | +file's path does not exist, in which case the Run() method skips calling | ||
| 264 | +the DecodePointerFromFile() function from our "lfs" package, since there | ||
| 265 | +is no value in trying to read a non-existent file's contents to see if | ||
| 266 | +it contains a raw Git LFS pointer. If some other type of error was | ||
| 267 | +returned, the Run() method logs the error and returns without proceeding | ||
| 268 | +further, and if no error was returned, then all the file's ancestor | ||
| 269 | +directories were found, so the Run() method does call the | ||
| 270 | +DecodePointerFromFile() function in that case. | ||
| 271 | + | ||
| 272 | +The Run() method then proceeds to check the results from the | ||
| 273 | +DecodePointerFromFile() function, if it was called at all. This logic | ||
| 274 | +remains unchanged, but can take advantage of the fact that an ErrNotExist | ||
| 275 | +error from the call to the Walk() method implies that no pointer file | ||
| 276 | +exists. When this type of error is returned by either the Walk() method | ||
| 277 | +or the DecodePointerFromFile() function, the Run() method then calls the | ||
| 278 | +DiffIndexWithPaths() function in our "git" package to check if the user | ||
| 279 | +has intentionally removed the file from Git's index, in which case no | ||
| 280 | +further action should be taken. | ||
| 281 | + | ||
| 282 | +If an ErrNotExist error was returned by either the Walk() method or the | ||
| 283 | +DecodePointerFromFile() function, and the user has not removed the file | ||
| 284 | +from Git's index, then the Run() method calls the DirWalker structure's | ||
| 285 | +WalkAndCreate() method in order to create any directories in the file's | ||
| 286 | +path which are missing. For this call, the internal walk() method of | ||
| 287 | +the DirWalker structure continues where the previous invocation left off, | ||
| 288 | +based on the values of the internal "parentPath" and "path" elements of | ||
| 289 | +the structure. | ||
| 290 | + | ||
| 291 | +The previous invocation of the walk() method by the Walk() method will | ||
| 292 | +have set the structure's "parentPath" element to contain the leading | ||
| 293 | +directories in the file's path that were found to exist, and set the | ||
| 294 | +"path" element to contain just those directories which need to be | ||
| 295 | +created. Note that either of these paths may be empty, since there may | ||
| 296 | +be no missing directories, or all the directories in the file's path may | ||
| 297 | +be missing, or the file may be located in the top-level directory. | ||
| 298 | + | ||
| 299 | +To verify that the DirWalker structure's internal walk() method handles | ||
| 300 | +all of these potential conditions, along with various types of directory | ||
| 301 | +conflicts such as pre-existing files or symbolic links, we add a | ||
| 302 | +TestDirWalkerWalk() Go test function and define a large number of valid | ||
| 303 | +and invalid test cases for this function. The test function then | ||
| 304 | +exercises the walk() method in all the defined test cases, both with an | ||
| 305 | +empty parent path and with a non-empty parent path. | ||
| 306 | + | ||
| 307 | +When the Run() method calls the DirWalker's WalkAndCreate() method, this | ||
| 308 | +passes a "true" value to the walk() method for its "create" parameter, so | ||
| 309 | +any directories that are missing will be created. This means that when | ||
| 310 | +the Run() method then calls the RunToPath() method, and it invokes the | ||
| 311 | +SmudgeToFile() method of the GitFilter structure in our "lfs" package, | ||
| 312 | +that method no longer needs to try to create any directories. We | ||
| 313 | +therefore remove the call to the MkdirAll() function in our "tools" | ||
| 314 | +package from the SmudgeToFile() method. | ||
| 315 | + | ||
| 316 | +However, the MkdirAll() function in our "tools" package is designed to | ||
| 317 | +enforce any umask settings defined by Git's "core.sharedRepository" | ||
| 318 | +configuration option, which is why the SmudgeToFile() method did not | ||
| 319 | +simply invoke the MkdirAll() function from the "os" package. Since | ||
| 320 | +we want to retain support for this Git configuration option, we add | ||
| 321 | +a Mkdir() function to our "tools" package which mirrors the MkdirAll() | ||
| 322 | +function, with the only difference being that it wraps the Mkdir() | ||
| 323 | +function from the "os" package rather than the MkdirAll() function. | ||
| 324 | +We then call the new function in the walk() method instead of calling | ||
| 325 | +the Mkdir() function from the "os" package directly. | ||
| 326 | + | ||
| 327 | +There is one use case where we still need to use the MkdirAll() | ||
| 328 | +function from our "tools" package, though. When the "git lfs checkout" | ||
| 329 | +command is run with its --to option, the RunToPath() method of the | ||
| 330 | +singleCheckout structure is invoked directly. The file path specified | ||
| 331 | +as the parameter of the --to option is converted to an absolute path | ||
| 332 | +and passed to the RunToPath() method so that the contents of the | ||
| 333 | +Git LFS object identified by the other command-line parameters are | ||
| 334 | +written to a file at the given path. | ||
| 335 | + | ||
| 336 | +Since the Run() method does not execute in this case, the WalkAndCreate() | ||
| 337 | +method is not called and therefore will not create any directories that | ||
| 338 | +might be missing in the path specified by the --to option, and neither | ||
| 339 | +will the SmudgeToFile() method, because it no longer calls the MkdirAll() | ||
| 340 | +function from our "tools" package. To ensure that we still support the | ||
| 341 | +use of the --to option with an arbitrary file path parameter, we now call | ||
| 342 | +the "tools" package's MkdirAll() function in the checkoutConflict() | ||
| 343 | +function of the "git lfs checkout" command immediately after we convert | ||
| 344 | +the --to option's parameter into an absolute file path. | ||
| 345 | + | ||
| 346 | +In previous commits we expanded the checks in the "checkout: conflicts" | ||
| 347 | +test in our t/t-checkout.sh test script so it will validate the use | ||
| 348 | +of the "git lfs checkout" command's --to option in a wide range of | ||
| 349 | +conditions, including with file path parameters to locations with | ||
| 350 | +ancestor directories that do not exist. As a consequence, we can be | ||
| 351 | +confident that the test validates that our changes in this commit do | ||
| 352 | +not introduce a regression in our support of the --to option of the | ||
| 353 | +"git lfs checkout" command. | ||
| 354 | + | ||
| 355 | +On the other hand, we do require additional shell tests to thoroughly | ||
| 356 | +validate the effectiveness of our revisions to the methods of the | ||
| 357 | +singleCheckout structure. Since we expect the "git lfs checkout" and | ||
| 358 | +"git lfs pull" commands to now try to detect when symbolic links exist | ||
| 359 | +in place of the directories in the paths to Git LFS files in a work tree, | ||
| 360 | +even if the targets of those links are themselves directories, we expand | ||
| 361 | +the "checkout: skip directory symlink conflicts" and "pull: skip | ||
| 362 | +directory symlink conflicts" tests that we added to our t/t-checkout.sh | ||
| 363 | +and t/t-pull.sh test scripts in a prior commit. | ||
| 364 | + | ||
| 365 | +Previously, these two tests verified that the "git lfs checkout" and | ||
| 366 | +"git lfs pull" commands would skip attempting to write out the contents of | ||
| 367 | +Git LFS objects into files in the work tree if the files' paths conflicted | ||
| 368 | +with pre-existing symbolic links, but only when the targets of the links | ||
| 369 | +were not directories. The tests now also specifically check the commands' | ||
| 370 | +behaviour when the targets of the links are directories, since before | ||
| 371 | +our changes in this commit the commands would traverse these links and | ||
| 372 | +create or update files and subdirectories within the target directories. | ||
| 373 | +Note, though, that we do not check this behaviour under TOCTOU race | ||
| 374 | +conditions, because we do not expect the commands to avoid traversing | ||
| 375 | +symbolic links in those cases, as described above. | ||
| 376 | + | ||
| 377 | +We also expand the "checkout: skip case-based symlink conflicts" and | ||
| 378 | +"pull: skip case-based symlink conflicts" tests we added in a previous | ||
| 379 | +commit. These tests now also check that when when the directories in | ||
| 380 | +Git LFS file paths conflict with symbolic links as a result of | ||
| 381 | +case-insensitivity on the part of a filesystem, the "git lfs checkout" | ||
| 382 | +and "git lfs pull" commands detect the conflicts and report errors | ||
| 383 | +instead of trying to populate the Git LFS files with their objects' | ||
| 384 | +contents. | ||
| 385 | + | ||
| 386 | +In both these two tests and the "checkout: skip directory symlink | ||
| 387 | +conflicts" and "pull: skip directory symlink conflicts" tests, we make | ||
| 388 | +an additional check to confirm that when symbolic links to directories | ||
| 389 | +exist in place of regular directories in the paths to Git LFS files, the | ||
| 390 | +Git error message "is beyond a symbolic link" does not appear in the | ||
| 391 | +output of the "git lfs checkout" and "git lfs pull" commands. This | ||
| 392 | +message would indicate that the Git LFS commands attempted to refresh | ||
| 393 | +the Git index using the "git update-index" command for a file whose path | ||
| 394 | +contains a symbolic link to a directory in place of a regular directory. | ||
| 395 | +As the "git lfs checkout" and "git lfs pull" commands should now detect | ||
| 396 | +such symbolic links (so long as there is no TOCTOU race), these Git error | ||
| 397 | +messages should not appear in the commands' output. | ||
| 398 | + | ||
| 399 | +Finally, we adjust the "checkout: skip directory file conflicts" and | ||
| 400 | +"pull: skip directory file conflicts" tests we added in another prior | ||
| 401 | +commit. These tests check that the "git lfs checkout" and "git lfs pull" | ||
| 402 | +commands detect when a regular file exists in the place of a directory | ||
| 403 | +in a Git LFS file's path. Our changes in this commit do not alter | ||
| 404 | +that fundamental behaviour, but they do result in a more consistent | ||
| 405 | +error message from the commands when a regular file exists in place | ||
| 406 | +of a directory. | ||
| 407 | + | ||
| 408 | +Previously, when a file conflicted with a directory in a Git LFS file's | ||
| 409 | +path, the output of the "git lfs checkout" and "git lfs pull" commands | ||
| 410 | +differed between Unix and Windows systems due to a difference in the | ||
| 411 | +error returned by the Lstat() function call performed in the | ||
| 412 | +DecodePointerFromFile() function. On Unix systems, this error | ||
| 413 | +encapsulates an ENOTDIR error number, which the IsNotExist() function | ||
| 414 | +of the "os" package does not consider equivalent to an ErrNotExist | ||
| 415 | +error. On these systems, the Run() method would therefore report the | ||
| 416 | +error immediately after calling the DecodePointerFromFile() function | ||
| 417 | +and then return without taking further action. | ||
| 418 | + | ||
| 419 | +On Windows systems, however, the same circumstances caused the Lstat() | ||
| 420 | +function to return an ErrNotExist error, due to the implementation of | ||
| 421 | +the Lstat() function in the Go standard library, which maps the Windows | ||
| 422 | +ERROR_FILE_NOT_FOUND error number to an ErrNotExist error. As a result, | ||
| 423 | +the Run() method would proceed to call the RunToPath() method, which | ||
| 424 | +invoked the SmudgeToFile() method. When that method called the OpenFile() | ||
| 425 | +function from the "os" package to try to create the Git LFS file, though, | ||
| 426 | +an error would occur, and this was then the error whose message would be | ||
| 427 | +logged by the by the "git lfs checkout" and "git lfs pull" commands. | ||
| 428 | + | ||
| 429 | +Now that the DecodePointerFromFile() function is only called by the Run() | ||
| 430 | +method if its invocation of the DirWalker structure's Walk() method does | ||
| 431 | +not return an error, the "git lfs checkout" and "git lfs pull" commands | ||
| 432 | +will report the same error message on both Unix and Windows systems if | ||
| 433 | +the Walk() method encounters a regular file in place of a directory. | ||
| 434 | +To account for this change, we update our "checkout: skip directory file | ||
| 435 | +conflicts" and "pull: skip directory file conflicts" tests so they expect | ||
| 436 | +the same error message on all systems. | ||
| 437 | + | ||
| 438 | +In addition to these changes to our regular Go and shell test suites, | ||
| 439 | +we also evaluated the impact of our changes in this commit to the | ||
| 440 | +speed of the "git lfs checkout" and "git lfs pull" commands under | ||
| 441 | +moderate workloads. Our performance testing focused on the "git lfs | ||
| 442 | +checkout" command since we are not concerned with the time required | ||
| 443 | +to fetch Git LFS objects from a remote server. | ||
| 444 | + | ||
| 445 | +For our principal test scenario, we created 10,000 small Git LFS files, | ||
| 446 | +with each file containing roughly 10 bytes of data only, so that the | ||
| 447 | +time required to write out the Git LFS object data of each file was | ||
| 448 | +minimal. | ||
| 449 | + | ||
| 450 | +Because the cost of checking for symbolic links in the paths to Git LFS | ||
| 451 | +files will scale with the number of files and the number of path | ||
| 452 | +components, we chose a distribution of our test files with the intent | ||
| 453 | +that it would emulate a relatively normal repository and not a | ||
| 454 | +pathological use case. For example, if we placed all the Git LFS files | ||
| 455 | +at the root of the repository, we would not exercise our new checks | ||
| 456 | +for symbolic links at all. For our principal test repository, we | ||
| 457 | +therefore distributed the Git LFS files in groups of 100 into 100 | ||
| 458 | +subdirectories, with 5 ancestor directories between these each of | ||
| 459 | +these subdirectories and the root of the repository. | ||
| 460 | + | ||
| 461 | +In a completely empty working tree, the runtime of the "git lfs checkout" | ||
| 462 | +command is heavily dominated by the cost of repeatedly spawning the | ||
| 463 | +"git diff-index" command, which we execute once for each file we find | ||
| 464 | +to be missing from the work tree. (Improving this behaviour so that | ||
| 465 | +the "git diff-index" command could be invoked with multiple file paths | ||
| 466 | +would be a valuable enhancement we might want to explore in the future.) | ||
| 467 | + | ||
| 468 | +So as to better evaluate the performance impact of our changes in this | ||
| 469 | +commit, we usually populated our working tree with raw Git LFS pointer | ||
| 470 | +files, as might occur after running "git clone" with the | ||
| 471 | +GIT_LFS_SKIP_SMUDGE environment variable set to a value equivalent | ||
| 472 | +to "true". This avoids the cost of executing the "git diff-index" | ||
| 473 | +command, which can otherwise result in a tenfold increase in the | ||
| 474 | +runtime of the "git lfs checkout" command. | ||
| 475 | + | ||
| 476 | +For the majority of our tests, we utilized a Linux system with 16 cores | ||
| 477 | +running at 2.10 GHz and a 5.15 kernel version. We also repeated our | ||
| 478 | +tests on macOS and Windows systems, with similar results. The times | ||
| 479 | +reported below are from the Linux system tests. | ||
| 480 | + | ||
| 481 | +In our primary test scenario, with 10,000 small Git LFS files in groups | ||
| 482 | +of 100 with 6 levels of subdirectories for each group, the impact of | ||
| 483 | +checking of each directory in the files' paths amounted to a 15% increase | ||
| 484 | +in the average runtime of the "git lfs checkout" command compared to the | ||
| 485 | +3.7.0 version of the Git LFS client. The v3.7.0 client's average | ||
| 486 | +runtime was 3.89s and the average runtime with this commit's changes | ||
| 487 | +was 4.46s. | ||
| 488 | + | ||
| 489 | +We also experimented with the inclusion of a simple lock-free single-entry | ||
| 490 | +cache in the walk() function, similar to the cache implemented by Git in | ||
| 491 | +its lstat_cache_matchlen() function. This reduced the average runtime | ||
| 492 | +of the "git lfs checkout" command in the same scenario described above | ||
| 493 | +to 4.23s, an 8% increase over the v3.7.0 client's average runtime. | ||
| 494 | + | ||
| 495 | +Our test scenario represented the ideal conditions for this simple | ||
| 496 | +cache, however. The "git lfs checkout" command processes files | ||
| 497 | +sequentially in the order returned by the "git ls-files" command (or | ||
| 498 | +the "git ls-tree" command, if the installed version of Git is older | ||
| 499 | +than v2.42.0), and so we could avoid the need for any locks around | ||
| 500 | +our cache, or use a more complex multiple-entry cache. | ||
| 501 | + | ||
| 502 | +The "git lfs pull" command, though, invokes the Run() method of the | ||
| 503 | +singleCheckout structure from two separate goroutines, one of which | ||
| 504 | +receives its list of Git LFS pointer files from the transfer queue as | ||
| 505 | +their corresponding objects' data is downloaded. A functional cache | ||
| 506 | +implementation would consequently require locks to avoid contention | ||
| 507 | +between parallel invocations of the walk() method by separate goroutines, | ||
| 508 | +which would somewhat diminish any potential performance gains. | ||
| 509 | + | ||
| 510 | +A single-entry cache might also prove to be ineffective with the | ||
| 511 | +"git lfs pull" command, since some files would be processed immediately | ||
| 512 | +if their objects were present in the local Git LFS storage directories, | ||
| 513 | +while others would be processed as their objects were downloaded, which | ||
| 514 | +might occur in a significantly different order than the sort order of | ||
| 515 | +the pointers' file paths. Instead of a single-entry cache, we could | ||
| 516 | +use a simple map of unbounded size, or an LRU (Least-Recently Used) | ||
| 517 | +cache with a bounded number of elements. | ||
| 518 | + | ||
| 519 | +However, if we do choose to add a cache in the future, it should not | ||
| 520 | +expose us to the type of vulnerability which the Git project reported | ||
| 521 | +in CVE-2021-21300. That issue resulted partly from the use of a | ||
| 522 | +single-entry cache and an incorrect assumption that files would always | ||
| 523 | +be processed in sorted order, but the key difference between Git and | ||
| 524 | +Git LFS in this regard is that Git tries to conform the working tree | ||
| 525 | +to have the contents it expects, and Git LFS does not. | ||
| 526 | + | ||
| 527 | +During a "git checkout" command, Git will try to remove directory entries | ||
| 528 | +such as files and symbolic links which conflict with the file paths Git | ||
| 529 | +intends to create. Thus, when Git encountered files whose paths | ||
| 530 | +conflicted on a case-insensitive filesystem, if these files were | ||
| 531 | +processed out of the usual sorted order, Git might cache one file | ||
| 532 | +path, then remove it from the filesystem but not the cache, and then | ||
| 533 | +assume the file path still existed based on the contents of the cache. | ||
| 534 | +Git LFS should not be vulnerable to this type of problem because it | ||
| 535 | +does not try to remove entries which conflict with the ancestor | ||
| 536 | +directories in a Git LFS file's path. | ||
| 537 | + | ||
| 538 | +Overall, though, the performance of the "git lfs checkout" command with | ||
| 539 | +the changes from this commit but without any form of caching appears to | ||
| 540 | +be acceptable, so we do not implement a cache in the DirWalker structure's | ||
| 541 | +methods at this time. We can always revisit this decision in the future, | ||
| 542 | +of course. | ||
| 543 | + | ||
| 544 | +As well as testing our changes from this commit (both with and without | ||
| 545 | +a simple cache), we also tested an experimental version of the | ||
| 546 | +"git lfs checkout" command which used the methods of the Root structure | ||
| 547 | +type from the "os" package. As described above, these methods are | ||
| 548 | +designed to ensure that they never operate on files outside a given | ||
| 549 | +initial "root" file path. | ||
| 550 | + | ||
| 551 | +On our Linux test system, the average runtime of the "git lfs checkout" | ||
| 552 | +command, when all filesystem operations were converted to use the | ||
| 553 | +methods of the Root type, was 6.57s in our primary test scenario, a 69% | ||
| 554 | +increase over the average runtime of the command when using the 3.7.0 | ||
| 555 | +version of Git LFS client, and a 47% increase over the average runtime | ||
| 556 | +of the command when using the changes from this commit. (Those average | ||
| 557 | +runtimes were 3.89s and 4.46s, respectively.) | ||
| 558 | + | ||
| 559 | +On a GitHub Actions runner with Windows Server 2025, the average runtime | ||
| 560 | +of the "git lfs checkout" command when all its filesystem operations | ||
| 561 | +used the Root type's methods was 28.83s, a 63% increase over the average | ||
| 562 | +runtime of the command when using the 3.7.0 version of the client, and | ||
| 563 | +a 39% increase over the average runtime of the command when using the | ||
| 564 | +changes from this commit. (Those average runtimes were 17.70s and 20.70s, | ||
| 565 | +respectively.) | ||
| 566 | + | ||
| 567 | +Intriguingly, on a GitHub Actions runner with macOS 15.5 (Sequoia), the | ||
| 568 | +average runtime of the "git lfs checkout" command with the changes from | ||
| 569 | +this commit was 5.81s, 5% faster than the 6.14s average runtime when | ||
| 570 | +using the 3.7.0 version of the Git LFS client. The average runtime of | ||
| 571 | +the command when all filesystem operations used the Root type's methods, | ||
| 572 | +however, was 11.73s, a 91% increase compared to the runtime of the | ||
| 573 | +command with the 3.7.0 version of the client and a 102% increase | ||
| 574 | +compared to the runtime of the command with the changes from this commit. | ||
| 575 | + | ||
| 576 | + | v3.7.0 | DirWalker | os.Root | ||
| 577 | + --------+-----------+-----------+----------- | ||
| 578 | + Linux | 3.89s | 4.46s | 6.57s | ||
| 579 | + macOS | 6.14s | 5.81s | 11.73s | ||
| 580 | + Windows | 17.70s | 20.70s | 28.83s | ||
| 581 | + | ||
| 582 | +As we explained above, these performance impacts are the primary reason | ||
| 583 | +why we avoid the use of the Root interface and its methods and prefer to | ||
| 584 | +check for symbolic links in a more efficient manner, even if that allows | ||
| 585 | +for the possibility that we cannot detect some race conditions. | ||
| 586 | +--- | ||
| 587 | + commands/command_checkout.go | 6 + | ||
| 588 | + commands/pull.go | 24 +- | ||
| 589 | + lfs/gitfilter_smudge.go | 2 - | ||
| 590 | + t/t-checkout.sh | 144 +++++++---- | ||
| 591 | + t/t-pull.sh | 152 ++++++++--- | ||
| 592 | + tools/dir_walker.go | 107 ++++++++ | ||
| 593 | + tools/dir_walker_test.go | 473 +++++++++++++++++++++++++++++++++++ | ||
| 594 | + tools/filetools.go | 9 + | ||
| 595 | + 8 files changed, 829 insertions(+), 88 deletions(-) | ||
| 596 | + create mode 100644 tools/dir_walker.go | ||
| 597 | + create mode 100644 tools/dir_walker_test.go | ||
| 598 | + | ||
| 599 | +diff --git a/commands/command_checkout.go b/commands/command_checkout.go | ||
| 600 | +index fbdeae5831..88e8aceea6 100644 | ||
| 601 | +--- a/commands/command_checkout.go | ||
| 602 | ++++ b/commands/command_checkout.go | ||
| 603 | + import ( | ||
| 604 | + "github.com/git-lfs/git-lfs/v3/git" | ||
| 605 | + "github.com/git-lfs/git-lfs/v3/lfs" | ||
| 606 | + "github.com/git-lfs/git-lfs/v3/tasklog" | ||
| 607 | ++ "github.com/git-lfs/git-lfs/v3/tools" | ||
| 608 | + "github.com/git-lfs/git-lfs/v3/tq" | ||
| 609 | + "github.com/git-lfs/git-lfs/v3/tr" | ||
| 610 | + "github.com/spf13/cobra" | ||
| 611 | + func checkoutConflict(file string, stage git.IndexStage) { | ||
| 612 | + Exit(tr.Tr.Get("Could not convert %q to absolute path: %v", checkoutTo, err)) | ||
| 613 | + } | ||
| 614 | + | ||
| 615 | ++ err = tools.MkdirAll(filepath.Dir(checkoutTo), cfg) | ||
| 616 | ++ if err != nil { | ||
| 617 | ++ Exit(tr.Tr.Get("Could not create path %q: %v", checkoutTo, err)) | ||
| 618 | ++ } | ||
| 619 | ++ | ||
| 620 | + // will chdir to root of working tree, if one exists | ||
| 621 | + singleCheckout := newSingleCheckout(cfg.Git, "") | ||
| 622 | + if singleCheckout.Skip() { | ||
| 623 | +diff --git a/commands/pull.go b/commands/pull.go | ||
| 624 | +index c6b47facdb..a89bb6a785 100644 | ||
| 625 | +--- a/commands/pull.go | ||
| 626 | ++++ b/commands/pull.go | ||
| 627 | + import ( | ||
| 628 | + "github.com/git-lfs/git-lfs/v3/git" | ||
| 629 | + "github.com/git-lfs/git-lfs/v3/lfs" | ||
| 630 | + "github.com/git-lfs/git-lfs/v3/subprocess" | ||
| 631 | ++ "github.com/git-lfs/git-lfs/v3/tools" | ||
| 632 | + "github.com/git-lfs/git-lfs/v3/tq" | ||
| 633 | + "github.com/git-lfs/git-lfs/v3/tr" | ||
| 634 | + ) | ||
| 635 | + func (c *singleCheckout) Run(p *lfs.WrappedPointer) { | ||
| 636 | + return | ||
| 637 | + } | ||
| 638 | + | ||
| 639 | +- // Check the content - either missing or still this pointer (not exist is ok) | ||
| 640 | +- filepointer, err := lfs.DecodePointerFromFile(p.Name) | ||
| 641 | ++ dirWalker := tools.NewDirWalkerForFile("", p.Name, cfg) | ||
| 642 | ++ err := dirWalker.Walk() | ||
| 643 | ++ | ||
| 644 | ++ var filepointer *lfs.Pointer | ||
| 645 | ++ if err != nil { | ||
| 646 | ++ if !os.IsNotExist(err) { | ||
| 647 | ++ LoggedError(err, tr.Tr.Get("Checkout error trying to check path for %q: %s", p.Name, err)) | ||
| 648 | ++ return | ||
| 649 | ++ } | ||
| 650 | ++ } else { | ||
| 651 | ++ // Check the content - either missing or still this pointer (not exist is ok) | ||
| 652 | ++ filepointer, err = lfs.DecodePointerFromFile(p.Name) | ||
| 653 | ++ } | ||
| 654 | ++ | ||
| 655 | + if err != nil { | ||
| 656 | + if os.IsNotExist(err) { | ||
| 657 | + output, err := git.DiffIndexWithPaths("HEAD", true, []string{p.Name}) | ||
| 658 | + func (c *singleCheckout) Run(p *lfs.WrappedPointer) { | ||
| 659 | + return | ||
| 660 | + } | ||
| 661 | + | ||
| 662 | ++ if err != nil && os.IsNotExist(err) { | ||
| 663 | ++ if err := dirWalker.WalkAndCreate(); err != nil { | ||
| 664 | ++ LoggedError(err, tr.Tr.Get("Checkout error trying to create path for %q: %s", p.Name, err)) | ||
| 665 | ++ return | ||
| 666 | ++ } | ||
| 667 | ++ } | ||
| 668 | ++ | ||
| 669 | + if err := c.RunToPath(p, p.Name); err != nil { | ||
| 670 | + if errors.IsDownloadDeclinedError(err) { | ||
| 671 | + // acceptable error, data not local (fetch not run or include/exclude) | ||
| 672 | +diff --git a/lfs/gitfilter_smudge.go b/lfs/gitfilter_smudge.go | ||
| 673 | +index 778cafc2f0..d091eb2524 100644 | ||
| 674 | +--- a/lfs/gitfilter_smudge.go | ||
| 675 | ++++ b/lfs/gitfilter_smudge.go | ||
| 676 | + import ( | ||
| 677 | + ) | ||
| 678 | + | ||
| 679 | + func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, manifest tq.Manifest, cb tools.CopyCallback) error { | ||
| 680 | +- tools.MkdirAll(filepath.Dir(filename), f.cfg) | ||
| 681 | +- | ||
| 682 | + // When no pointer file exists on disk, we should use the permissions | ||
| 683 | + // defined for the file in Git, since the executable mode may be set. | ||
| 684 | + // However, to conform with our legacy behaviour, we do not do this | ||
| 685 | +diff --git a/t/t-checkout.sh b/t/t-checkout.sh | ||
| 686 | +index f7c362c476..79eb2016b5 100755 | ||
| 687 | +--- a/t/t-checkout.sh | ||
| 688 | ++++ b/t/t-checkout.sh | ||
| 689 | + begin_test "checkout: skip directory file conflicts" | ||
| 690 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 691 | + exit 1 | ||
| 692 | + fi | ||
| 693 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 694 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' checkout.log | ||
| 695 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' checkout.log | ||
| 696 | +- else | ||
| 697 | +- grep 'Checkout error for "dir1/a\.dat": lstat' checkout.log | ||
| 698 | +- grep 'Checkout error for "dir2/dir3/dir4/a\.dat": lstat' checkout.log | ||
| 699 | +- fi | ||
| 700 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 701 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 702 | + | ||
| 703 | + [ -f "dir1" ] | ||
| 704 | + [ -f "dir2/dir3" ] | ||
| 705 | + begin_test "checkout: skip directory file conflicts" | ||
| 706 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 707 | + exit 1 | ||
| 708 | + fi | ||
| 709 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 710 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' checkout.log | ||
| 711 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' checkout.log | ||
| 712 | +- else | ||
| 713 | +- grep 'Checkout error for "dir1/a\.dat": lstat' checkout.log | ||
| 714 | +- grep 'Checkout error for "dir2/dir3/dir4/a\.dat": lstat' checkout.log | ||
| 715 | +- fi | ||
| 716 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 717 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 718 | + popd | ||
| 719 | + | ||
| 720 | + [ -f "dir1" ] | ||
| 721 | + begin_test "checkout: skip directory file conflicts" | ||
| 722 | + ) | ||
| 723 | + end_test | ||
| 724 | + | ||
| 725 | +-# Note that the conditions validated by this test are at present limited, | ||
| 726 | +-# but will be expanded in the future. | ||
| 727 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 728 | + ( | ||
| 729 | + set -e | ||
| 730 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 731 | + git add .gitattributes dir1 dir2 | ||
| 732 | + git commit -m "initial commit" | ||
| 733 | + | ||
| 734 | ++ # test with symlinks to directories | ||
| 735 | ++ rm -rf dir1 dir2/dir3 ../link* | ||
| 736 | ++ mkdir ../link1 ../link2 | ||
| 737 | ++ ln -s ../link1 dir1 | ||
| 738 | ++ ln -s ../../link2 dir2/dir3 | ||
| 739 | ++ | ||
| 740 | ++ git lfs checkout 2>&1 | tee checkout.log | ||
| 741 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 742 | ++ echo >&2 "fatal: expected checkout to succeed ..." | ||
| 743 | ++ exit 1 | ||
| 744 | ++ fi | ||
| 745 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 746 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 747 | ++ [ -z "$(grep "is beyond a symbolic link" checkout.log)" ] | ||
| 748 | ++ | ||
| 749 | ++ [ -L "dir1" ] | ||
| 750 | ++ [ -L "dir2/dir3" ] | ||
| 751 | ++ [ ! -e "../link1/a.dat" ] | ||
| 752 | ++ [ ! -e "../link2/dir4" ] | ||
| 753 | ++ assert_clean_index | ||
| 754 | ++ | ||
| 755 | ++ rm -rf dir1 dir2/dir3 | ||
| 756 | ++ mkdir link1 link2 | ||
| 757 | ++ ln -s link1 dir1 | ||
| 758 | ++ ln -s ../link2 dir2/dir3 | ||
| 759 | ++ | ||
| 760 | ++ git lfs checkout 2>&1 | tee checkout.log | ||
| 761 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 762 | ++ echo >&2 "fatal: expected checkout to succeed ..." | ||
| 763 | ++ exit 1 | ||
| 764 | ++ fi | ||
| 765 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 766 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 767 | ++ [ -z "$(grep "is beyond a symbolic link" checkout.log)" ] | ||
| 768 | ++ | ||
| 769 | ++ [ -L "dir1" ] | ||
| 770 | ++ [ -L "dir2/dir3" ] | ||
| 771 | ++ [ ! -e "link1/a.dat" ] | ||
| 772 | ++ [ ! -e "link2/dir4" ] | ||
| 773 | ++ assert_clean_index | ||
| 774 | ++ | ||
| 775 | ++ pushd dir2 | ||
| 776 | ++ git lfs checkout 2>&1 | tee checkout.log | ||
| 777 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 778 | ++ echo >&2 "fatal: expected checkout to succeed ..." | ||
| 779 | ++ exit 1 | ||
| 780 | ++ fi | ||
| 781 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 782 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 783 | ++ [ -z "$(grep "is beyond a symbolic link" checkout.log)" ] | ||
| 784 | ++ popd | ||
| 785 | ++ | ||
| 786 | ++ [ -L "dir1" ] | ||
| 787 | ++ [ -L "dir2/dir3" ] | ||
| 788 | ++ [ ! -e "link1/a.dat" ] | ||
| 789 | ++ [ ! -e "link2/dir4" ] | ||
| 790 | ++ assert_clean_index | ||
| 791 | ++ | ||
| 792 | + # test with symlink to file and dangling symlink | ||
| 793 | + rm -rf dir1 dir2/dir3 ../link* | ||
| 794 | + touch ../link1 | ||
| 795 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 796 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 797 | + exit 1 | ||
| 798 | + fi | ||
| 799 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 800 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' checkout.log | ||
| 801 | +- else | ||
| 802 | +- grep 'Checkout error for "dir1/a\.dat": lstat' checkout.log | ||
| 803 | +- fi | ||
| 804 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' checkout.log | ||
| 805 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 806 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 807 | + | ||
| 808 | + [ -L "dir1" ] | ||
| 809 | + [ -L "dir2/dir3" ] | ||
| 810 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 811 | + [ ! -e "../link2" ] | ||
| 812 | + assert_clean_index | ||
| 813 | + | ||
| 814 | +- rm -rf dir1 dir2/dir3 | ||
| 815 | ++ rm -rf dir1 dir2/dir3 link* | ||
| 816 | + touch link1 | ||
| 817 | + ln -s link1 dir1 | ||
| 818 | + ln -s ../link2 dir2/dir3 | ||
| 819 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 820 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 821 | + exit 1 | ||
| 822 | + fi | ||
| 823 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 824 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' checkout.log | ||
| 825 | +- else | ||
| 826 | +- grep 'Checkout error for "dir1/a\.dat": lstat' checkout.log | ||
| 827 | +- fi | ||
| 828 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' checkout.log | ||
| 829 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 830 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 831 | + | ||
| 832 | + [ -L "dir1" ] | ||
| 833 | + [ -L "dir2/dir3" ] | ||
| 834 | + begin_test "checkout: skip directory symlink conflicts" | ||
| 835 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 836 | + exit 1 | ||
| 837 | + fi | ||
| 838 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 839 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' checkout.log | ||
| 840 | +- else | ||
| 841 | +- grep 'Checkout error for "dir1/a\.dat": lstat' checkout.log | ||
| 842 | +- fi | ||
| 843 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' checkout.log | ||
| 844 | ++ grep '"dir1/a\.dat": not a directory' checkout.log | ||
| 845 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' checkout.log | ||
| 846 | + popd | ||
| 847 | + | ||
| 848 | + [ -L "dir1" ] | ||
| 849 | + begin_test "checkout: skip case-based symlink conflicts" | ||
| 850 | + mkdir dir1 | ||
| 851 | + ln -s ../link1 A.dat | ||
| 852 | + ln -s ../../link2 dir1/a.dat | ||
| 853 | ++ ln -s ../link3 DIR3 | ||
| 854 | ++ ln -s ../../link4 dir1/dir2 | ||
| 855 | + | ||
| 856 | +- git add A.dat dir1 | ||
| 857 | ++ git add A.dat dir1 DIR3 | ||
| 858 | + git commit -m "initial commit" | ||
| 859 | + | ||
| 860 | +- rm A.dat dir1/a.dat | ||
| 861 | ++ rm A.dat dir1/* DIR3 | ||
| 862 | + | ||
| 863 | + echo "*.dat filter=lfs diff=lfs merge=lfs -text" >.gitattributes | ||
| 864 | + | ||
| 865 | + contents="a" | ||
| 866 | + contents_oid="$(calc_oid "$contents")" | ||
| 867 | ++ mkdir dir3 dir1/DIR2 | ||
| 868 | + printf "%s" "$contents" >a.dat | ||
| 869 | + printf "%s" "$contents" >dir1/A.dat | ||
| 870 | ++ printf "%s" "$contents" >dir3/a.dat | ||
| 871 | ++ printf "%s" "$contents" >dir1/DIR2/a.dat | ||
| 872 | + | ||
| 873 | +- git -c core.ignoreCase=false add .gitattributes a.dat dir1/A.dat | ||
| 874 | ++ git -c core.ignoreCase=false add .gitattributes a.dat dir1/A.dat \ | ||
| 875 | ++ dir3/a.dat dir1/DIR2/a.dat | ||
| 876 | + git commit -m "case-conflicting commit" | ||
| 877 | + | ||
| 878 | + git push origin main | ||
| 879 | + begin_test "checkout: skip case-based symlink conflicts" | ||
| 880 | + | ||
| 881 | + assert_local_object "$contents_oid" 1 | ||
| 882 | + | ||
| 883 | +- rm -rf *.dat dir1 ../link* | ||
| 884 | ++ rm -rf *.dat dir1 *3 ../link* | ||
| 885 | ++ mkdir ../link3 ../link4 | ||
| 886 | + | ||
| 887 | + git lfs checkout 2>&1 | tee checkout.log | ||
| 888 | + if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 889 | + echo >&2 "fatal: expected checkout to succeed ..." | ||
| 890 | + exit 1 | ||
| 891 | + fi | ||
| 892 | +- grep -q 'Checking out LFS objects: 100% (2/2), 2 B' checkout.log | ||
| 893 | ++ grep -q 'Checking out LFS objects: 100% (4/4), 4 B' checkout.log | ||
| 894 | + | ||
| 895 | + [ -f "a.dat" ] | ||
| 896 | + [ "$contents" = "$(cat "a.dat")" ] | ||
| 897 | + [ -f "dir1/A.dat" ] | ||
| 898 | + [ "$contents" = "$(cat "dir1/A.dat")" ] | ||
| 899 | ++ [ -f "dir3/a.dat" ] | ||
| 900 | ++ [ "$contents" = "$(cat "dir3/a.dat")" ] | ||
| 901 | ++ [ -f "dir1/DIR2/a.dat" ] | ||
| 902 | ++ [ "$contents" = "$(cat "dir1/DIR2/a.dat")" ] | ||
| 903 | + [ ! -e "../link1" ] | ||
| 904 | + [ ! -e "../link2" ] | ||
| 905 | ++ [ ! -e "../link3/a.dat" ] | ||
| 906 | ++ [ ! -e "../link4/a.dat" ] | ||
| 907 | + assert_clean_index | ||
| 908 | + | ||
| 909 | +- rm -rf a.dat dir1/A.dat | ||
| 910 | +- git checkout -- A.dat dir1/a.dat | ||
| 911 | ++ rm -rf a.dat dir1/A.dat dir3 dir1/DIR2 | ||
| 912 | ++ git checkout -- A.dat dir1/a.dat DIR3 dir1/dir2 | ||
| 913 | + | ||
| 914 | + git lfs checkout 2>&1 | tee checkout.log | ||
| 915 | + if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 916 | + begin_test "checkout: skip case-based symlink conflicts" | ||
| 917 | + fi | ||
| 918 | + if [ "$collision" -eq "0" ]; then | ||
| 919 | + # case-sensitive filesystem | ||
| 920 | +- grep -q 'Checking out LFS objects: 100% (2/2), 2 B' checkout.log | ||
| 921 | ++ grep -q 'Checking out LFS objects: 100% (4/4), 4 B' checkout.log | ||
| 922 | + else | ||
| 923 | + # case-insensitive filesystem | ||
| 924 | + grep '"a\.dat": not a regular file' checkout.log | ||
| 925 | + grep '"dir1/A\.dat": not a regular file' checkout.log | ||
| 926 | ++ grep '"dir3/a\.dat": not a directory' checkout.log | ||
| 927 | ++ grep '"dir1/DIR2/a\.dat": not a directory' checkout.log | ||
| 928 | ++ [ -z "$(grep "is beyond a symbolic link" checkout.log)" ] | ||
| 929 | + fi | ||
| 930 | + | ||
| 931 | + if [ "$collision" -eq "0" ]; then | ||
| 932 | + begin_test "checkout: skip case-based symlink conflicts" | ||
| 933 | + [ "$contents" = "$(cat "a.dat")" ] | ||
| 934 | + [ -f "dir1/A.dat" ] | ||
| 935 | + [ "$contents" = "$(cat "dir1/A.dat")" ] | ||
| 936 | ++ [ -f "dir3/a.dat" ] | ||
| 937 | ++ [ "$contents" = "$(cat "dir3/a.dat")" ] | ||
| 938 | ++ [ -f "dir1/DIR2/a.dat" ] | ||
| 939 | ++ [ "$contents" = "$(cat "dir1/DIR2/a.dat")" ] | ||
| 940 | + else | ||
| 941 | + # case-insensitive filesystem | ||
| 942 | + [ -L "a.dat" ] | ||
| 943 | + [ -L "dir1/A.dat" ] | ||
| 944 | ++ [ -L "dir3" ] | ||
| 945 | ++ [ -L "dir1/DIR2" ] | ||
| 946 | + fi | ||
| 947 | + [ ! -e "../link1" ] | ||
| 948 | + [ ! -e "../link2" ] | ||
| 949 | ++ [ ! -e "../link3/a.dat" ] | ||
| 950 | ++ [ ! -e "../link4/a.dat" ] | ||
| 951 | + assert_clean_index | ||
| 952 | + ) | ||
| 953 | + end_test | ||
| 954 | +diff --git a/t/t-pull.sh b/t/t-pull.sh | ||
| 955 | +index c2ab5e42b7..299567fb31 100644 | ||
| 956 | +--- a/t/t-pull.sh | ||
| 957 | ++++ b/t/t-pull.sh | ||
| 958 | + begin_test "pull: skip directory file conflicts" | ||
| 959 | + echo >&2 "fatal: expected pull to succeed ..." | ||
| 960 | + exit 1 | ||
| 961 | + fi | ||
| 962 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 963 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' pull.log | ||
| 964 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' pull.log | ||
| 965 | +- else | ||
| 966 | +- grep 'Checkout error for "dir1/a\.dat": lstat' pull.log | ||
| 967 | +- grep 'Checkout error for "dir2/dir3/dir4/a\.dat": lstat' pull.log | ||
| 968 | +- fi | ||
| 969 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 970 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 971 | + | ||
| 972 | + assert_local_object "$contents_oid" 1 | ||
| 973 | + | ||
| 974 | + begin_test "pull: skip directory file conflicts" | ||
| 975 | + echo >&2 "fatal: expected pull to succeed ..." | ||
| 976 | + exit 1 | ||
| 977 | + fi | ||
| 978 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 979 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' pull.log | ||
| 980 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' pull.log | ||
| 981 | +- else | ||
| 982 | +- grep 'Checkout error for "dir1/a\.dat": lstat' pull.log | ||
| 983 | +- grep 'Checkout error for "dir2/dir3/dir4/a\.dat": lstat' pull.log | ||
| 984 | +- fi | ||
| 985 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 986 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 987 | + popd | ||
| 988 | + | ||
| 989 | + assert_local_object "$contents_oid" 1 | ||
| 990 | + begin_test "pull: skip directory file conflicts" | ||
| 991 | + ) | ||
| 992 | + end_test | ||
| 993 | + | ||
| 994 | +-# Note that the conditions validated by this test are at present limited, | ||
| 995 | +-# but will be expanded in the future. | ||
| 996 | + begin_test "pull: skip directory symlink conflicts" | ||
| 997 | + ( | ||
| 998 | + set -e | ||
| 999 | + begin_test "pull: skip directory symlink conflicts" | ||
| 1000 | + cd "${reponame}-assert" | ||
| 1001 | + refute_local_object "$contents_oid" 1 | ||
| 1002 | + | ||
| 1003 | ++ # test with symlinks to directories | ||
| 1004 | ++ rm -rf dir1 dir2/dir3 ../link* | ||
| 1005 | ++ mkdir ../link1 ../link2 | ||
| 1006 | ++ ln -s ../link1 dir1 | ||
| 1007 | ++ ln -s ../../link2 dir2/dir3 | ||
| 1008 | ++ | ||
| 1009 | ++ git lfs pull 2>&1 | tee pull.log | ||
| 1010 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 1011 | ++ echo >&2 "fatal: expected pull to succeed ..." | ||
| 1012 | ++ exit 1 | ||
| 1013 | ++ fi | ||
| 1014 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1015 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1016 | ++ [ -z "$(grep "is beyond a symbolic link" pull.log)" ] | ||
| 1017 | ++ | ||
| 1018 | ++ assert_local_object "$contents_oid" 1 | ||
| 1019 | ++ | ||
| 1020 | ++ [ -L "dir1" ] | ||
| 1021 | ++ [ -L "dir2/dir3" ] | ||
| 1022 | ++ [ ! -e "../link1/a.dat" ] | ||
| 1023 | ++ [ ! -e "../link2/dir4" ] | ||
| 1024 | ++ assert_clean_index | ||
| 1025 | ++ | ||
| 1026 | ++ rm -rf .git/lfs/objects | ||
| 1027 | ++ | ||
| 1028 | ++ rm -rf dir1 dir2/dir3 | ||
| 1029 | ++ mkdir link1 link2 | ||
| 1030 | ++ ln -s link1 dir1 | ||
| 1031 | ++ ln -s ../link2 dir2/dir3 | ||
| 1032 | ++ | ||
| 1033 | ++ git lfs pull 2>&1 | tee pull.log | ||
| 1034 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 1035 | ++ echo >&2 "fatal: expected pull to succeed ..." | ||
| 1036 | ++ exit 1 | ||
| 1037 | ++ fi | ||
| 1038 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1039 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1040 | ++ [ -z "$(grep "is beyond a symbolic link" pull.log)" ] | ||
| 1041 | ++ | ||
| 1042 | ++ assert_local_object "$contents_oid" 1 | ||
| 1043 | ++ | ||
| 1044 | ++ [ -L "dir1" ] | ||
| 1045 | ++ [ -L "dir2/dir3" ] | ||
| 1046 | ++ [ ! -e "link1/a.dat" ] | ||
| 1047 | ++ [ ! -e "link2/dir4" ] | ||
| 1048 | ++ assert_clean_index | ||
| 1049 | ++ | ||
| 1050 | ++ rm -rf .git/lfs/objects | ||
| 1051 | ++ | ||
| 1052 | ++ pushd dir2 | ||
| 1053 | ++ git lfs pull 2>&1 | tee pull.log | ||
| 1054 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 1055 | ++ echo >&2 "fatal: expected pull to succeed ..." | ||
| 1056 | ++ exit 1 | ||
| 1057 | ++ fi | ||
| 1058 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1059 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1060 | ++ [ -z "$(grep "is beyond a symbolic link" pull.log)" ] | ||
| 1061 | ++ popd | ||
| 1062 | ++ | ||
| 1063 | ++ assert_local_object "$contents_oid" 1 | ||
| 1064 | ++ | ||
| 1065 | ++ [ -L "dir1" ] | ||
| 1066 | ++ [ -L "dir2/dir3" ] | ||
| 1067 | ++ [ ! -e "link1/a.dat" ] | ||
| 1068 | ++ [ ! -e "link2/dir4" ] | ||
| 1069 | ++ assert_clean_index | ||
| 1070 | ++ | ||
| 1071 | + # test with symlink to file and dangling symlink | ||
| 1072 | ++ rm -rf .git/lfs/objects | ||
| 1073 | ++ | ||
| 1074 | + rm -rf dir1 dir2/dir3 ../link* | ||
| 1075 | + touch ../link1 | ||
| 1076 | + ln -s ../link1 dir1 | ||
| 1077 | + begin_test "pull: skip directory symlink conflicts" | ||
| 1078 | + echo >&2 "fatal: expected pull to succeed ..." | ||
| 1079 | + exit 1 | ||
| 1080 | + fi | ||
| 1081 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 1082 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' pull.log | ||
| 1083 | +- else | ||
| 1084 | +- grep 'Checkout error for "dir1/a\.dat": lstat' pull.log | ||
| 1085 | +- fi | ||
| 1086 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' pull.log | ||
| 1087 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1088 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1089 | + | ||
| 1090 | + assert_local_object "$contents_oid" 1 | ||
| 1091 | + | ||
| 1092 | + begin_test "pull: skip directory symlink conflicts" | ||
| 1093 | + | ||
| 1094 | + rm -rf .git/lfs/objects | ||
| 1095 | + | ||
| 1096 | +- rm -rf dir1 dir2/dir3 | ||
| 1097 | ++ rm -rf dir1 dir2/dir3 link* | ||
| 1098 | + touch link1 | ||
| 1099 | + ln -s link1 dir1 | ||
| 1100 | + ln -s ../link2 dir2/dir3 | ||
| 1101 | + begin_test "pull: skip directory symlink conflicts" | ||
| 1102 | + echo >&2 "fatal: expected pull to succeed ..." | ||
| 1103 | + exit 1 | ||
| 1104 | + fi | ||
| 1105 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 1106 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' pull.log | ||
| 1107 | +- else | ||
| 1108 | +- grep 'Checkout error for "dir1/a\.dat": lstat' pull.log | ||
| 1109 | +- fi | ||
| 1110 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' pull.log | ||
| 1111 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1112 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1113 | + | ||
| 1114 | + assert_local_object "$contents_oid" 1 | ||
| 1115 | + | ||
| 1116 | + begin_test "pull: skip directory symlink conflicts" | ||
| 1117 | + echo >&2 "fatal: expected pull to succeed ..." | ||
| 1118 | + exit 1 | ||
| 1119 | + fi | ||
| 1120 | +- if [ "$IS_WINDOWS" -eq 1 ]; then | ||
| 1121 | +- grep 'could not check out "dir1/a\.dat": could not create working directory file' pull.log | ||
| 1122 | +- else | ||
| 1123 | +- grep 'Checkout error for "dir1/a\.dat": lstat' pull.log | ||
| 1124 | +- fi | ||
| 1125 | +- grep 'could not check out "dir2/dir3/dir4/a\.dat": could not create working directory file' pull.log | ||
| 1126 | ++ grep '"dir1/a\.dat": not a directory' pull.log | ||
| 1127 | ++ grep '"dir2/dir3/dir4/a\.dat": not a directory' pull.log | ||
| 1128 | + popd | ||
| 1129 | + | ||
| 1130 | + assert_local_object "$contents_oid" 1 | ||
| 1131 | + begin_test "pull: skip case-based symlink conflicts" | ||
| 1132 | + mkdir dir1 | ||
| 1133 | + ln -s ../link1 A.dat | ||
| 1134 | + ln -s ../../link2 dir1/a.dat | ||
| 1135 | ++ ln -s ../link3 DIR3 | ||
| 1136 | ++ ln -s ../../link4 dir1/dir2 | ||
| 1137 | + | ||
| 1138 | +- git add A.dat dir1 | ||
| 1139 | ++ git add A.dat dir1 DIR3 | ||
| 1140 | + git commit -m "initial commit" | ||
| 1141 | + | ||
| 1142 | +- rm A.dat dir1/a.dat | ||
| 1143 | ++ rm A.dat dir1/* DIR3 | ||
| 1144 | + | ||
| 1145 | + echo "*.dat filter=lfs diff=lfs merge=lfs -text" >.gitattributes | ||
| 1146 | + | ||
| 1147 | + contents="a" | ||
| 1148 | + contents_oid="$(calc_oid "$contents")" | ||
| 1149 | ++ mkdir dir3 dir1/DIR2 | ||
| 1150 | + printf "%s" "$contents" >a.dat | ||
| 1151 | + printf "%s" "$contents" >dir1/A.dat | ||
| 1152 | ++ printf "%s" "$contents" >dir3/a.dat | ||
| 1153 | ++ printf "%s" "$contents" >dir1/DIR2/a.dat | ||
| 1154 | + | ||
| 1155 | +- git -c core.ignoreCase=false add .gitattributes a.dat dir1/A.dat | ||
| 1156 | ++ git -c core.ignoreCase=false add .gitattributes a.dat dir1/A.dat \ | ||
| 1157 | ++ dir3/a.dat dir1/DIR2/a.dat | ||
| 1158 | + git commit -m "case-conflicting commit" | ||
| 1159 | + | ||
| 1160 | + git push origin main | ||
| 1161 | + begin_test "pull: skip case-based symlink conflicts" | ||
| 1162 | + cd "${reponame}-assert" | ||
| 1163 | + refute_local_object "$contents_oid" 1 | ||
| 1164 | + | ||
| 1165 | +- rm -rf *.dat dir1 ../link* | ||
| 1166 | ++ rm -rf *.dat dir1 *3 ../link* | ||
| 1167 | ++ mkdir ../link3 ../link4 | ||
| 1168 | + | ||
| 1169 | + git lfs pull | ||
| 1170 | + | ||
| 1171 | + begin_test "pull: skip case-based symlink conflicts" | ||
| 1172 | + [ "$contents" = "$(cat "a.dat")" ] | ||
| 1173 | + [ -f "dir1/A.dat" ] | ||
| 1174 | + [ "$contents" = "$(cat "dir1/A.dat")" ] | ||
| 1175 | ++ [ -f "dir3/a.dat" ] | ||
| 1176 | ++ [ "$contents" = "$(cat "dir3/a.dat")" ] | ||
| 1177 | ++ [ -f "dir1/DIR2/a.dat" ] | ||
| 1178 | ++ [ "$contents" = "$(cat "dir1/DIR2/a.dat")" ] | ||
| 1179 | + [ ! -e "../link1" ] | ||
| 1180 | + [ ! -e "../link2" ] | ||
| 1181 | ++ [ ! -e "../link3/a.dat" ] | ||
| 1182 | ++ [ ! -e "../link4/a.dat" ] | ||
| 1183 | + assert_clean_index | ||
| 1184 | + | ||
| 1185 | +- rm -rf a.dat dir1/A.dat | ||
| 1186 | +- git checkout -- A.dat dir1/a.dat | ||
| 1187 | ++ rm -rf a.dat dir1/A.dat dir3 dir1/DIR2 | ||
| 1188 | ++ git checkout -- A.dat dir1/a.dat DIR3 dir1/dir2 | ||
| 1189 | + | ||
| 1190 | + git lfs pull 2>&1 | tee pull.log | ||
| 1191 | + if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 1192 | + begin_test "pull: skip case-based symlink conflicts" | ||
| 1193 | + # case-insensitive filesystem | ||
| 1194 | + grep '"a\.dat": not a regular file' pull.log | ||
| 1195 | + grep '"dir1/A\.dat": not a regular file' pull.log | ||
| 1196 | ++ grep '"dir3/a\.dat": not a directory' pull.log | ||
| 1197 | ++ grep '"dir1/DIR2/a\.dat": not a directory' pull.log | ||
| 1198 | ++ [ -z "$(grep "is beyond a symbolic link" pull.log)" ] | ||
| 1199 | + fi | ||
| 1200 | + | ||
| 1201 | + if [ "$collision" -eq "0" ]; then | ||
| 1202 | + begin_test "pull: skip case-based symlink conflicts" | ||
| 1203 | + [ "$contents" = "$(cat "a.dat")" ] | ||
| 1204 | + [ -f "dir1/A.dat" ] | ||
| 1205 | + [ "$contents" = "$(cat "dir1/A.dat")" ] | ||
| 1206 | ++ [ -f "dir3/a.dat" ] | ||
| 1207 | ++ [ "$contents" = "$(cat "dir3/a.dat")" ] | ||
| 1208 | ++ [ -f "dir1/DIR2/a.dat" ] | ||
| 1209 | ++ [ "$contents" = "$(cat "dir1/DIR2/a.dat")" ] | ||
| 1210 | + else | ||
| 1211 | + # case-insensitive filesystem | ||
| 1212 | + [ -L "a.dat" ] | ||
| 1213 | + [ -L "dir1/A.dat" ] | ||
| 1214 | ++ [ -L "dir3" ] | ||
| 1215 | ++ [ -L "dir1/DIR2" ] | ||
| 1216 | + fi | ||
| 1217 | + [ ! -e "../link1" ] | ||
| 1218 | + [ ! -e "../link2" ] | ||
| 1219 | ++ [ ! -e "../link3/a.dat" ] | ||
| 1220 | ++ [ ! -e "../link4/a.dat" ] | ||
| 1221 | + assert_clean_index | ||
| 1222 | + ) | ||
| 1223 | + end_test | ||
| 1224 | +diff --git a/tools/dir_walker.go b/tools/dir_walker.go | ||
| 1225 | +new file mode 100644 | ||
| 1226 | +index 0000000000..7b7c71bfcf | ||
| 1227 | +--- /dev/null | ||
| 1228 | ++++ b/tools/dir_walker.go | ||
| 1229 | + | ||
| 1230 | ++package tools | ||
| 1231 | ++ | ||
| 1232 | ++import ( | ||
| 1233 | ++ "os" | ||
| 1234 | ++ "strings" | ||
| 1235 | ++ | ||
| 1236 | ++ "github.com/git-lfs/git-lfs/v3/errors" | ||
| 1237 | ++ "github.com/git-lfs/git-lfs/v3/tr" | ||
| 1238 | ++) | ||
| 1239 | ++ | ||
| 1240 | ++var ( | ||
| 1241 | ++ errInvalidDir = errors.New(tr.Tr.Get("invalid directory")) | ||
| 1242 | ++ errNotDir = errors.New(tr.Tr.Get("not a directory")) | ||
| 1243 | ++) | ||
| 1244 | ++ | ||
| 1245 | ++type DirWalker struct { | ||
| 1246 | ++ parentPath string | ||
| 1247 | ++ path string | ||
| 1248 | ++ config repositoryPermissionFetcher | ||
| 1249 | ++} | ||
| 1250 | ++ | ||
| 1251 | ++// The parentPath parameter is assumed to be a valid path to a directory | ||
| 1252 | ++// in the filesystem. | ||
| 1253 | ++// | ||
| 1254 | ++// The filePath parameter must be a relative file path as provided by Git, | ||
| 1255 | ++// with only the "/" character as a separator and no empty or "." or ".." | ||
| 1256 | ++// path segments. Absolute paths are not supported. | ||
| 1257 | ++func NewDirWalkerForFile(parentPath string, filePath string, config repositoryPermissionFetcher) *DirWalker { | ||
| 1258 | ++ var path string | ||
| 1259 | ++ i := strings.LastIndexByte(filePath, '/') | ||
| 1260 | ++ if i >= 0 { | ||
| 1261 | ++ path = filePath[0:i] | ||
| 1262 | ++ } | ||
| 1263 | ++ | ||
| 1264 | ++ return &DirWalker{ | ||
| 1265 | ++ parentPath: parentPath, | ||
| 1266 | ++ path: path, | ||
| 1267 | ++ config: config, | ||
| 1268 | ++ } | ||
| 1269 | ++} | ||
| 1270 | ++ | ||
| 1271 | ++// walk() checks each directory in a relative path, starting from the | ||
| 1272 | ++// initial parent path, and optionally creates any missing directories | ||
| 1273 | ++// in the path. | ||
| 1274 | ++// | ||
| 1275 | ++// If an existing file or something else other than a directory conflicts | ||
| 1276 | ++// with a directory in the path, walk() returns an error. | ||
| 1277 | ++// | ||
| 1278 | ++// If the create option is false, walk() returns ErrNotExist when a | ||
| 1279 | ++// directory is not found. | ||
| 1280 | ++// | ||
| 1281 | ++// Note that for performance reasons and to be consistent with Git's | ||
| 1282 | ++// implementation, walk() does not guard against TOCTOU (time-of-check/ | ||
| 1283 | ++// time-of-use) races, as the methods of the os.Root type do. | ||
| 1284 | ++func (w *DirWalker) walk(create bool) error { | ||
| 1285 | ++ currentPath := w.parentPath | ||
| 1286 | ++ | ||
| 1287 | ++ n := len(w.path) | ||
| 1288 | ++ for n > 0 { | ||
| 1289 | ++ currentDir := w.path | ||
| 1290 | ++ nextDirIndex := n | ||
| 1291 | ++ i := strings.IndexByte(w.path, '/') | ||
| 1292 | ++ if i >= 0 { | ||
| 1293 | ++ currentDir = w.path[0:i] | ||
| 1294 | ++ nextDirIndex = i + 1 | ||
| 1295 | ++ } | ||
| 1296 | ++ | ||
| 1297 | ++ // These should never occur in Git paths. | ||
| 1298 | ++ if currentDir == "" || currentDir == "." || currentDir == ".." { | ||
| 1299 | ++ return errors.Join(errors.New(tr.Tr.Get("invalid directory %q in path: %q", currentDir, w.path)), errInvalidDir) | ||
| 1300 | ++ } | ||
| 1301 | ++ | ||
| 1302 | ++ if currentPath == "" { | ||
| 1303 | ++ currentPath = currentDir | ||
| 1304 | ++ } else { | ||
| 1305 | ++ currentPath += "/" + currentDir | ||
| 1306 | ++ } | ||
| 1307 | ++ | ||
| 1308 | ++ stat, err := os.Lstat(currentPath) | ||
| 1309 | ++ if err != nil { | ||
| 1310 | ++ if !os.IsNotExist(err) || !create { | ||
| 1311 | ++ return err | ||
| 1312 | ++ } | ||
| 1313 | ++ | ||
| 1314 | ++ err = Mkdir(currentPath, w.config) | ||
| 1315 | ++ if err != nil { | ||
| 1316 | ++ return err | ||
| 1317 | ++ } | ||
| 1318 | ++ } else if !stat.Mode().IsDir() { | ||
| 1319 | ++ return errors.Join(errors.New(tr.Tr.Get("not a directory: %q", currentPath)), errNotDir) | ||
| 1320 | ++ } | ||
| 1321 | ++ | ||
| 1322 | ++ w.parentPath = currentPath | ||
| 1323 | ++ w.path = w.path[nextDirIndex:] | ||
| 1324 | ++ n -= nextDirIndex | ||
| 1325 | ++ } | ||
| 1326 | ++ | ||
| 1327 | ++ return nil | ||
| 1328 | ++} | ||
| 1329 | ++ | ||
| 1330 | ++func (w *DirWalker) Walk() error { | ||
| 1331 | ++ return w.walk(false) | ||
| 1332 | ++} | ||
| 1333 | ++ | ||
| 1334 | ++func (w *DirWalker) WalkAndCreate() error { | ||
| 1335 | ++ return w.walk(true) | ||
| 1336 | ++} | ||
| 1337 | +diff --git a/tools/dir_walker_test.go b/tools/dir_walker_test.go | ||
| 1338 | +new file mode 100644 | ||
| 1339 | +index 0000000000..33cd140fa5 | ||
| 1340 | +--- /dev/null | ||
| 1341 | ++++ b/tools/dir_walker_test.go | ||
| 1342 | + | ||
| 1343 | ++package tools | ||
| 1344 | ++ | ||
| 1345 | ++import ( | ||
| 1346 | ++ "errors" | ||
| 1347 | ++ "fmt" | ||
| 1348 | ++ "os" | ||
| 1349 | ++ "testing" | ||
| 1350 | ++ | ||
| 1351 | ++ "github.com/stretchr/testify/assert" | ||
| 1352 | ++ "github.com/stretchr/testify/require" | ||
| 1353 | ++) | ||
| 1354 | ++ | ||
| 1355 | ++type newDirWalkerForFileTestCase struct { | ||
| 1356 | ++ filePath string | ||
| 1357 | ++ expectedDirPath string | ||
| 1358 | ++} | ||
| 1359 | ++ | ||
| 1360 | ++func (c *newDirWalkerForFileTestCase) Assert(t *testing.T) { | ||
| 1361 | ++ w := NewDirWalkerForFile("", c.filePath, nil) | ||
| 1362 | ++ assert.Equal(t, c.expectedDirPath, w.path) | ||
| 1363 | ++} | ||
| 1364 | ++ | ||
| 1365 | ++func TestNewDirWalkerForFile(t *testing.T) { | ||
| 1366 | ++ for desc, c := range map[string]*newDirWalkerForFileTestCase{ | ||
| 1367 | ++ "filename only": {"foo.bin", ""}, | ||
| 1368 | ++ "path with one dir": {"abc/foo.bin", "abc"}, | ||
| 1369 | ++ "path with two dirs": {"abc/def/foo.bin", "abc/def"}, | ||
| 1370 | ++ "path with leading slash": {"/foo.bin", ""}, | ||
| 1371 | ++ "path with trailing slash": {"abc/", "abc"}, | ||
| 1372 | ++ "bare slash": {"/", ""}, | ||
| 1373 | ++ "empty path": {"", ""}, | ||
| 1374 | ++ } { | ||
| 1375 | ++ t.Run(desc, c.Assert) | ||
| 1376 | ++ } | ||
| 1377 | ++} | ||
| 1378 | ++ | ||
| 1379 | ++type dirWalkerTestConfig struct{} | ||
| 1380 | ++ | ||
| 1381 | ++func (c *dirWalkerTestConfig) RepositoryPermissions(executable bool) os.FileMode { | ||
| 1382 | ++ return os.FileMode(0755) | ||
| 1383 | ++} | ||
| 1384 | ++ | ||
| 1385 | ++type dirWalkerWalkTestCase struct { | ||
| 1386 | ++ parentPath string | ||
| 1387 | ++ path string | ||
| 1388 | ++ create bool | ||
| 1389 | ++ | ||
| 1390 | ++ existsPath string | ||
| 1391 | ++ existsFile string | ||
| 1392 | ++ existsLink string | ||
| 1393 | ++ | ||
| 1394 | ++ expectedParentPath string | ||
| 1395 | ++ expectedPath string | ||
| 1396 | ++ expectedErr error | ||
| 1397 | ++ | ||
| 1398 | ++ walker *DirWalker | ||
| 1399 | ++} | ||
| 1400 | ++ | ||
| 1401 | ++func (c *dirWalkerWalkTestCase) prependParentPath(path string) string { | ||
| 1402 | ++ if path == "" { | ||
| 1403 | ++ return c.parentPath | ||
| 1404 | ++ } else if c.parentPath == "" { | ||
| 1405 | ++ return path | ||
| 1406 | ++ } else if path[0] == '/' { | ||
| 1407 | ++ return "/" + c.parentPath + path | ||
| 1408 | ++ } else { | ||
| 1409 | ++ return c.parentPath + "/" + path | ||
| 1410 | ++ } | ||
| 1411 | ++} | ||
| 1412 | ++ | ||
| 1413 | ++func (c *dirWalkerWalkTestCase) setupPaths(t *testing.T, parentPath string) error { | ||
| 1414 | ++ c.parentPath = parentPath | ||
| 1415 | ++ | ||
| 1416 | ++ if parentPath != "" { | ||
| 1417 | ++ if err := os.MkdirAll(parentPath, 0755); err != nil { | ||
| 1418 | ++ return fmt.Errorf("unable to create path: %w", err) | ||
| 1419 | ++ } | ||
| 1420 | ++ } | ||
| 1421 | ++ | ||
| 1422 | ++ if c.existsPath != "" { | ||
| 1423 | ++ c.existsPath = c.prependParentPath(c.existsPath) | ||
| 1424 | ++ if err := os.MkdirAll(c.existsPath, 0755); err != nil { | ||
| 1425 | ++ return fmt.Errorf("unable to create path: %w", err) | ||
| 1426 | ++ } | ||
| 1427 | ++ } | ||
| 1428 | ++ | ||
| 1429 | ++ if c.existsFile != "" { | ||
| 1430 | ++ c.existsFile = c.prependParentPath(c.existsFile) | ||
| 1431 | ++ f, err := os.Create(c.existsFile) | ||
| 1432 | ++ if err != nil { | ||
| 1433 | ++ return fmt.Errorf("unable to create file: %w", err) | ||
| 1434 | ++ } | ||
| 1435 | ++ f.Close() | ||
| 1436 | ++ } | ||
| 1437 | ++ | ||
| 1438 | ++ if c.existsLink != "" { | ||
| 1439 | ++ c.existsLink = c.prependParentPath(c.existsLink) | ||
| 1440 | ++ if err := os.Symlink(t.TempDir(), c.existsLink); err != nil { | ||
| 1441 | ++ return fmt.Errorf("unable to create symbolic link: %w", err) | ||
| 1442 | ++ } | ||
| 1443 | ++ } | ||
| 1444 | ++ | ||
| 1445 | ++ c.expectedParentPath = c.prependParentPath(c.expectedParentPath) | ||
| 1446 | ++ | ||
| 1447 | ++ return nil | ||
| 1448 | ++} | ||
| 1449 | ++ | ||
| 1450 | ++func (c *dirWalkerWalkTestCase) Assert(t *testing.T) { | ||
| 1451 | ++ c.walker.parentPath = c.parentPath | ||
| 1452 | ++ c.walker.path = c.path | ||
| 1453 | ++ | ||
| 1454 | ++ err := c.walker.walk(c.create) | ||
| 1455 | ++ | ||
| 1456 | ++ assert.Equal(t, c.expectedParentPath, c.walker.parentPath, "found path does not match") | ||
| 1457 | ++ assert.Equal(t, c.expectedPath, c.walker.path, "missing path does not match") | ||
| 1458 | ++ if c.expectedErr == nil { | ||
| 1459 | ++ assert.NoError(t, err) | ||
| 1460 | ++ } else { | ||
| 1461 | ++ assert.Error(t, err) | ||
| 1462 | ++ assert.True(t, errors.Is(err, c.expectedErr), "wrong error type") | ||
| 1463 | ++ } | ||
| 1464 | ++} | ||
| 1465 | ++ | ||
| 1466 | ++func TestDirWalkerWalk(t *testing.T) { | ||
| 1467 | ++ wd, err := os.Getwd() | ||
| 1468 | ++ require.NoError(t, err) | ||
| 1469 | ++ | ||
| 1470 | ++ defer os.Chdir(wd) | ||
| 1471 | ++ | ||
| 1472 | ++ for desc, c := range map[string]*dirWalkerWalkTestCase{ | ||
| 1473 | ++ "empty path": {}, | ||
| 1474 | ++ "one extant dir": { | ||
| 1475 | ++ path: "abc", | ||
| 1476 | ++ existsPath: "abc", | ||
| 1477 | ++ expectedParentPath: "abc", | ||
| 1478 | ++ }, | ||
| 1479 | ++ "one missing dir": { | ||
| 1480 | ++ path: "abc", | ||
| 1481 | ++ expectedPath: "abc", | ||
| 1482 | ++ expectedErr: os.ErrNotExist, | ||
| 1483 | ++ }, | ||
| 1484 | ++ "two extant dirs": { | ||
| 1485 | ++ path: "abc/def", | ||
| 1486 | ++ existsPath: "abc/def", | ||
| 1487 | ++ expectedParentPath: "abc/def", | ||
| 1488 | ++ }, | ||
| 1489 | ++ "two missing dirs": { | ||
| 1490 | ++ path: "abc/def", | ||
| 1491 | ++ expectedPath: "abc/def", | ||
| 1492 | ++ expectedErr: os.ErrNotExist, | ||
| 1493 | ++ }, | ||
| 1494 | ++ "three extant dirs": { | ||
| 1495 | ++ path: "abc/def/ghi", | ||
| 1496 | ++ existsPath: "abc/def/ghi", | ||
| 1497 | ++ expectedParentPath: "abc/def/ghi", | ||
| 1498 | ++ }, | ||
| 1499 | ++ "three missing dirs": { | ||
| 1500 | ++ path: "abc/def/ghi", | ||
| 1501 | ++ expectedPath: "abc/def/ghi", | ||
| 1502 | ++ expectedErr: os.ErrNotExist, | ||
| 1503 | ++ }, | ||
| 1504 | ++ "one extant dir and one missing dir": { | ||
| 1505 | ++ path: "abc/def", | ||
| 1506 | ++ existsPath: "abc", | ||
| 1507 | ++ expectedParentPath: "abc", | ||
| 1508 | ++ expectedPath: "def", | ||
| 1509 | ++ expectedErr: os.ErrNotExist, | ||
| 1510 | ++ }, | ||
| 1511 | ++ "one extant dir and two missing dirs": { | ||
| 1512 | ++ path: "abc/def/ghi", | ||
| 1513 | ++ existsPath: "abc", | ||
| 1514 | ++ expectedParentPath: "abc", | ||
| 1515 | ++ expectedPath: "def/ghi", | ||
| 1516 | ++ expectedErr: os.ErrNotExist, | ||
| 1517 | ++ }, | ||
| 1518 | ++ "two extant dirs and one missing dir": { | ||
| 1519 | ++ path: "abc/def/ghi", | ||
| 1520 | ++ existsPath: "abc/def", | ||
| 1521 | ++ expectedParentPath: "abc/def", | ||
| 1522 | ++ expectedPath: "ghi", | ||
| 1523 | ++ expectedErr: os.ErrNotExist, | ||
| 1524 | ++ }, | ||
| 1525 | ++ "one missing dir with trailing slash": { | ||
| 1526 | ++ path: "abc/", | ||
| 1527 | ++ expectedPath: "abc/", | ||
| 1528 | ++ expectedErr: os.ErrNotExist, | ||
| 1529 | ++ }, | ||
| 1530 | ++ "one extant dir with trailing slash": { | ||
| 1531 | ++ path: "abc/", | ||
| 1532 | ++ existsPath: "abc", | ||
| 1533 | ++ expectedParentPath: "abc", | ||
| 1534 | ++ }, | ||
| 1535 | ++ "two extant dirs with trailing slash": { | ||
| 1536 | ++ path: "abc/def/", | ||
| 1537 | ++ existsPath: "abc/def", | ||
| 1538 | ++ expectedParentPath: "abc/def", | ||
| 1539 | ++ }, | ||
| 1540 | ++ "one extant dir and one missing dir with trailing slash": { | ||
| 1541 | ++ path: "abc/def/", | ||
| 1542 | ++ existsPath: "abc", | ||
| 1543 | ++ expectedParentPath: "abc", | ||
| 1544 | ++ expectedPath: "def/", | ||
| 1545 | ++ expectedErr: os.ErrNotExist, | ||
| 1546 | ++ }, | ||
| 1547 | ++ "one conflicting file": { | ||
| 1548 | ++ path: "abc", | ||
| 1549 | ++ existsFile: "abc", | ||
| 1550 | ++ expectedPath: "abc", | ||
| 1551 | ++ expectedErr: errNotDir, | ||
| 1552 | ++ }, | ||
| 1553 | ++ "one extant dir and one conflicting file": { | ||
| 1554 | ++ path: "abc/def", | ||
| 1555 | ++ existsPath: "abc", | ||
| 1556 | ++ existsFile: "abc/def", | ||
| 1557 | ++ expectedParentPath: "abc", | ||
| 1558 | ++ expectedPath: "def", | ||
| 1559 | ++ expectedErr: errNotDir, | ||
| 1560 | ++ }, | ||
| 1561 | ++ "two extant dirs and one conflicting file": { | ||
| 1562 | ++ path: "abc/def/ghi", | ||
| 1563 | ++ existsPath: "abc/def", | ||
| 1564 | ++ existsFile: "abc/def/ghi", | ||
| 1565 | ++ expectedParentPath: "abc/def", | ||
| 1566 | ++ expectedPath: "ghi", | ||
| 1567 | ++ expectedErr: errNotDir, | ||
| 1568 | ++ }, | ||
| 1569 | ++ "one extant dir, one conflicting file, and one missing dir": { | ||
| 1570 | ++ path: "abc/def/ghi", | ||
| 1571 | ++ existsPath: "abc", | ||
| 1572 | ++ existsFile: "abc/def", | ||
| 1573 | ++ expectedParentPath: "abc", | ||
| 1574 | ++ expectedPath: "def/ghi", | ||
| 1575 | ++ expectedErr: errNotDir, | ||
| 1576 | ++ }, | ||
| 1577 | ++ "one conflicting symlink": { | ||
| 1578 | ++ path: "abc", | ||
| 1579 | ++ existsLink: "abc", | ||
| 1580 | ++ expectedPath: "abc", | ||
| 1581 | ++ expectedErr: errNotDir, | ||
| 1582 | ++ }, | ||
| 1583 | ++ "one extant dir and one conflicting symlink": { | ||
| 1584 | ++ path: "abc/def", | ||
| 1585 | ++ existsPath: "abc", | ||
| 1586 | ++ existsLink: "abc/def", | ||
| 1587 | ++ expectedParentPath: "abc", | ||
| 1588 | ++ expectedPath: "def", | ||
| 1589 | ++ expectedErr: errNotDir, | ||
| 1590 | ++ }, | ||
| 1591 | ++ "two extant dirs and one conflicting symlink": { | ||
| 1592 | ++ path: "abc/def/ghi", | ||
| 1593 | ++ existsPath: "abc/def", | ||
| 1594 | ++ existsLink: "abc/def/ghi", | ||
| 1595 | ++ expectedParentPath: "abc/def", | ||
| 1596 | ++ expectedPath: "ghi", | ||
| 1597 | ++ expectedErr: errNotDir, | ||
| 1598 | ++ }, | ||
| 1599 | ++ "one extant dir, one conflicting symlink, and one missing dir": { | ||
| 1600 | ++ path: "abc/def/ghi", | ||
| 1601 | ++ existsPath: "abc", | ||
| 1602 | ++ existsLink: "abc/def", | ||
| 1603 | ++ expectedParentPath: "abc", | ||
| 1604 | ++ expectedPath: "def/ghi", | ||
| 1605 | ++ expectedErr: errNotDir, | ||
| 1606 | ++ }, | ||
| 1607 | ++ "one extant dir (not modified)": { | ||
| 1608 | ++ path: "abc", | ||
| 1609 | ++ create: true, | ||
| 1610 | ++ existsPath: "abc", | ||
| 1611 | ++ expectedParentPath: "abc", | ||
| 1612 | ++ }, | ||
| 1613 | ++ "one created dir": { | ||
| 1614 | ++ path: "abc", | ||
| 1615 | ++ create: true, | ||
| 1616 | ++ expectedParentPath: "abc", | ||
| 1617 | ++ }, | ||
| 1618 | ++ "two extant dirs (not modified)": { | ||
| 1619 | ++ path: "abc/def", | ||
| 1620 | ++ create: true, | ||
| 1621 | ++ existsPath: "abc/def", | ||
| 1622 | ++ expectedParentPath: "abc/def", | ||
| 1623 | ++ }, | ||
| 1624 | ++ "two created dirs": { | ||
| 1625 | ++ path: "abc/def", | ||
| 1626 | ++ create: true, | ||
| 1627 | ++ expectedParentPath: "abc/def", | ||
| 1628 | ++ }, | ||
| 1629 | ++ "three extant dirs (not modified)": { | ||
| 1630 | ++ path: "abc/def/ghi", | ||
| 1631 | ++ create: true, | ||
| 1632 | ++ existsPath: "abc/def/ghi", | ||
| 1633 | ++ expectedParentPath: "abc/def/ghi", | ||
| 1634 | ++ }, | ||
| 1635 | ++ "three created dirs": { | ||
| 1636 | ++ path: "abc/def/ghi", | ||
| 1637 | ++ create: true, | ||
| 1638 | ++ expectedParentPath: "abc/def/ghi", | ||
| 1639 | ++ }, | ||
| 1640 | ++ "one extant dir and one created dir": { | ||
| 1641 | ++ path: "abc/def", | ||
| 1642 | ++ create: true, | ||
| 1643 | ++ existsPath: "abc", | ||
| 1644 | ++ expectedParentPath: "abc/def", | ||
| 1645 | ++ }, | ||
| 1646 | ++ "one extant dir and two created dirs": { | ||
| 1647 | ++ path: "abc/def/ghi", | ||
| 1648 | ++ create: true, | ||
| 1649 | ++ existsPath: "abc", | ||
| 1650 | ++ expectedParentPath: "abc/def/ghi", | ||
| 1651 | ++ }, | ||
| 1652 | ++ "two extant dirs and one created dir": { | ||
| 1653 | ++ path: "abc/def/ghi", | ||
| 1654 | ++ create: true, | ||
| 1655 | ++ existsPath: "abc/def", | ||
| 1656 | ++ expectedParentPath: "abc/def/ghi", | ||
| 1657 | ++ }, | ||
| 1658 | ++ "one created dir with trailing slash": { | ||
| 1659 | ++ path: "abc/", | ||
| 1660 | ++ create: true, | ||
| 1661 | ++ expectedParentPath: "abc", | ||
| 1662 | ++ }, | ||
| 1663 | ++ "one extant dir with trailing slash (not modified)": { | ||
| 1664 | ++ path: "abc/", | ||
| 1665 | ++ create: true, | ||
| 1666 | ++ existsPath: "abc", | ||
| 1667 | ++ expectedParentPath: "abc", | ||
| 1668 | ++ }, | ||
| 1669 | ++ "two extant dirs with trailing slash (not modified)": { | ||
| 1670 | ++ path: "abc/def/", | ||
| 1671 | ++ create: true, | ||
| 1672 | ++ existsPath: "abc/def", | ||
| 1673 | ++ expectedParentPath: "abc/def", | ||
| 1674 | ++ }, | ||
| 1675 | ++ "one extant dir and one created dir with trailing slash": { | ||
| 1676 | ++ path: "abc/def/", | ||
| 1677 | ++ create: true, | ||
| 1678 | ++ existsPath: "abc", | ||
| 1679 | ++ expectedParentPath: "abc/def", | ||
| 1680 | ++ }, | ||
| 1681 | ++ "one conflicting file (not modified)": { | ||
| 1682 | ++ path: "abc", | ||
| 1683 | ++ create: true, | ||
| 1684 | ++ existsFile: "abc", | ||
| 1685 | ++ expectedPath: "abc", | ||
| 1686 | ++ expectedErr: errNotDir, | ||
| 1687 | ++ }, | ||
| 1688 | ++ "one extant dir and one conflicting file (not modified)": { | ||
| 1689 | ++ path: "abc/def", | ||
| 1690 | ++ create: true, | ||
| 1691 | ++ existsPath: "abc", | ||
| 1692 | ++ existsFile: "abc/def", | ||
| 1693 | ++ expectedParentPath: "abc", | ||
| 1694 | ++ expectedPath: "def", | ||
| 1695 | ++ expectedErr: errNotDir, | ||
| 1696 | ++ }, | ||
| 1697 | ++ "two extant dirs and one conflicting file (not modified)": { | ||
| 1698 | ++ path: "abc/def/ghi", | ||
| 1699 | ++ create: true, | ||
| 1700 | ++ existsPath: "abc/def", | ||
| 1701 | ++ existsFile: "abc/def/ghi", | ||
| 1702 | ++ expectedParentPath: "abc/def", | ||
| 1703 | ++ expectedPath: "ghi", | ||
| 1704 | ++ expectedErr: errNotDir, | ||
| 1705 | ++ }, | ||
| 1706 | ++ "one extant dir, one conflicting file, and one missing dir (not modified)": { | ||
| 1707 | ++ path: "abc/def/ghi", | ||
| 1708 | ++ create: true, | ||
| 1709 | ++ existsPath: "abc", | ||
| 1710 | ++ existsFile: "abc/def", | ||
| 1711 | ++ expectedParentPath: "abc", | ||
| 1712 | ++ expectedPath: "def/ghi", | ||
| 1713 | ++ expectedErr: errNotDir, | ||
| 1714 | ++ }, | ||
| 1715 | ++ "one conflicting symlink (not modified)": { | ||
| 1716 | ++ path: "abc", | ||
| 1717 | ++ create: true, | ||
| 1718 | ++ existsLink: "abc", | ||
| 1719 | ++ expectedPath: "abc", | ||
| 1720 | ++ expectedErr: errNotDir, | ||
| 1721 | ++ }, | ||
| 1722 | ++ "one extant dir and one conflicting symlink (not modified)": { | ||
| 1723 | ++ path: "abc/def", | ||
| 1724 | ++ create: true, | ||
| 1725 | ++ existsPath: "abc", | ||
| 1726 | ++ existsLink: "abc/def", | ||
| 1727 | ++ expectedParentPath: "abc", | ||
| 1728 | ++ expectedPath: "def", | ||
| 1729 | ++ expectedErr: errNotDir, | ||
| 1730 | ++ }, | ||
| 1731 | ++ "two extant dirs and one conflicting symlink (not modified)": { | ||
| 1732 | ++ path: "abc/def/ghi", | ||
| 1733 | ++ create: true, | ||
| 1734 | ++ existsPath: "abc/def", | ||
| 1735 | ++ existsLink: "abc/def/ghi", | ||
| 1736 | ++ expectedParentPath: "abc/def", | ||
| 1737 | ++ expectedPath: "ghi", | ||
| 1738 | ++ expectedErr: errNotDir, | ||
| 1739 | ++ }, | ||
| 1740 | ++ "one extant dir, one conflicting symlink, and one missing dir (not modified)": { | ||
| 1741 | ++ path: "abc/def/ghi", | ||
| 1742 | ++ create: true, | ||
| 1743 | ++ existsPath: "abc", | ||
| 1744 | ++ existsLink: "abc/def", | ||
| 1745 | ++ expectedParentPath: "abc", | ||
| 1746 | ++ expectedPath: "def/ghi", | ||
| 1747 | ++ expectedErr: errNotDir, | ||
| 1748 | ++ }, | ||
| 1749 | ++ "invalid bare slash": { | ||
| 1750 | ++ path: "/", | ||
| 1751 | ++ expectedPath: "/", | ||
| 1752 | ++ expectedErr: errInvalidDir, | ||
| 1753 | ++ }, | ||
| 1754 | ++ "invalid multiple slashes": { | ||
| 1755 | ++ path: "abc//def", | ||
| 1756 | ++ existsPath: "abc", | ||
| 1757 | ++ expectedParentPath: "abc", | ||
| 1758 | ++ expectedPath: "/def", | ||
| 1759 | ++ expectedErr: errInvalidDir, | ||
| 1760 | ++ }, | ||
| 1761 | ++ "invalid leading slash": { | ||
| 1762 | ++ path: "/abc", | ||
| 1763 | ++ existsPath: "abc", | ||
| 1764 | ++ expectedPath: "/abc", | ||
| 1765 | ++ expectedErr: errInvalidDir, | ||
| 1766 | ++ }, | ||
| 1767 | ++ "invalid bare dot component": { | ||
| 1768 | ++ path: ".", | ||
| 1769 | ++ expectedPath: ".", | ||
| 1770 | ++ expectedErr: errInvalidDir, | ||
| 1771 | ++ }, | ||
| 1772 | ++ "invalid dot component": { | ||
| 1773 | ++ path: "abc/./def", | ||
| 1774 | ++ existsPath: "abc/def", | ||
| 1775 | ++ expectedParentPath: "abc", | ||
| 1776 | ++ expectedPath: "./def", | ||
| 1777 | ++ expectedErr: errInvalidDir, | ||
| 1778 | ++ }, | ||
| 1779 | ++ "invalid bare double-dot component": { | ||
| 1780 | ++ path: "..", | ||
| 1781 | ++ expectedPath: "..", | ||
| 1782 | ++ expectedErr: errInvalidDir, | ||
| 1783 | ++ }, | ||
| 1784 | ++ "invalid double-dot component": { | ||
| 1785 | ++ path: "abc/../def", | ||
| 1786 | ++ existsPath: "abc", | ||
| 1787 | ++ expectedParentPath: "abc", | ||
| 1788 | ++ expectedPath: "../def", | ||
| 1789 | ++ expectedErr: errInvalidDir, | ||
| 1790 | ++ }, | ||
| 1791 | ++ } { | ||
| 1792 | ++ if err := os.Chdir(t.TempDir()); err != nil { | ||
| 1793 | ++ t.Errorf("unable to change directory: %s", err) | ||
| 1794 | ++ } | ||
| 1795 | ++ | ||
| 1796 | ++ c.walker = &DirWalker{ | ||
| 1797 | ++ config: &dirWalkerTestConfig{}, | ||
| 1798 | ++ } | ||
| 1799 | ++ | ||
| 1800 | ++ if err := c.setupPaths(t, ""); err != nil { | ||
| 1801 | ++ t.Error(err) | ||
| 1802 | ++ continue | ||
| 1803 | ++ } | ||
| 1804 | ++ | ||
| 1805 | ++ t.Run(desc, c.Assert) | ||
| 1806 | ++ | ||
| 1807 | ++ // retest with parent path; note that this alters the test case | ||
| 1808 | ++ if err := c.setupPaths(t, "foo/bar"); err != nil { | ||
| 1809 | ++ t.Error(err) | ||
| 1810 | ++ continue | ||
| 1811 | ++ } | ||
| 1812 | ++ | ||
| 1813 | ++ t.Run(desc+" with parent path", c.Assert) | ||
| 1814 | ++ } | ||
| 1815 | ++} | ||
| 1816 | +diff --git a/tools/filetools.go b/tools/filetools.go | ||
| 1817 | +index cce05861df..1f0804bda7 100644 | ||
| 1818 | +--- a/tools/filetools.go | ||
| 1819 | ++++ b/tools/filetools.go | ||
| 1820 | + type repositoryPermissionFetcher interface { | ||
| 1821 | + RepositoryPermissions(executable bool) os.FileMode | ||
| 1822 | + } | ||
| 1823 | + | ||
| 1824 | ++// Mkdir makes a directory with the | ||
| 1825 | ++// permissions specified by the core.sharedRepository setting. | ||
| 1826 | ++func Mkdir(path string, config repositoryPermissionFetcher) error { | ||
| 1827 | ++ umask := 0777 & ^config.RepositoryPermissions(true) | ||
| 1828 | ++ return doWithUmask(int(umask), func() error { | ||
| 1829 | ++ return os.Mkdir(path, config.RepositoryPermissions(true)) | ||
| 1830 | ++ }) | ||
| 1831 | ++} | ||
| 1832 | ++ | ||
| 1833 | + // MkdirAll makes a directory and any intervening directories with the | ||
| 1834 | + // permissions specified by the core.sharedRepository setting. | ||
| 1835 | + func MkdirAll(path string, config repositoryPermissionFetcher) error { | ||
| @@ -0,0 +1,453 @@ | |||
| 1 | +From 5c11ffce9a4f095ff356bc781e2a031abb46c1a8 Mon Sep 17 00:00:00 2001 | ||
| 2 | +From: Chris Darroch <chrisd8088@github.com> | ||
| 3 | +Date: Thu, 15 May 2025 23:42:40 -0700 | ||
| 4 | +Subject: [PATCH] docs,lfs,t: create new files on checkout and pull | ||
| 5 | + | ||
| 6 | +Our "git lfs checkout" and "git lfs pull" commands, at present, | ||
| 7 | +follow any extant symbolic links when they populate the current working | ||
| 8 | +tree with files containing the content of Git LFS objects, even if | ||
| 9 | +the symbolic links point to locations outside of the working tree. | ||
| 10 | +This vulnerability has been assigned the identifier CVE-2025-26625. | ||
| 11 | + | ||
| 12 | +In a previous commit we partially addressed this vulnerability by | ||
| 13 | +adjusting the DecodePointerFromBlob() function in our "lfs" package to | ||
| 14 | +check whether an irregular file or other directory entry exists at the | ||
| 15 | +location where the commands intend to create or update a file. | ||
| 16 | + | ||
| 17 | +While this change handles cases where a symbolic link already exists | ||
| 18 | +the working tree before we try to create or update a file at the same | ||
| 19 | +location, it does not entirely prevent TOCTOU (time-of-check/time-of-use) | ||
| 20 | +races where a symbolic link might be created immediately after we check | ||
| 21 | +for its existence and before we attempt to create or open a file. | ||
| 22 | + | ||
| 23 | +One reason is that the "git lfs checkout" and "git lfs pull" commands | ||
| 24 | +use the Create() function from the Go standard library's "os" package | ||
| 25 | +to create or open the files they intend to populate with the contents | ||
| 26 | +of Git LFS objects. This function follows symbolic links when | ||
| 27 | +determining whether it should create a new file or truncate an existing | ||
| 28 | +one. If the last segment of the path passed to the function is a | ||
| 29 | +symbolic link, the link will be dereferenced, and a new file will be | ||
| 30 | +created at the link's target path or, if a file already exists at that | ||
| 31 | +target path, then that file will be opened and truncated. | ||
| 32 | + | ||
| 33 | +Further, because the Create() function opens and truncates any existing | ||
| 34 | +file it finds, if that file is hard-linked to one or more other | ||
| 35 | +paths, then once the file is closed the new content our commands have | ||
| 36 | +written into it will be visible through all of those paths, regardless | ||
| 37 | +of whether they reside inside or outside the Git working tree. | ||
| 38 | + | ||
| 39 | +Our "git lfs checkout" and "git lfs pull" commands have exhibited these | ||
| 40 | +behaviours since they were first implemented in PR #527. That PR | ||
| 41 | +added a PointerSmudgeToFile() function to the "lfs" package, which was | ||
| 42 | +later refactored by PR #2687 into the SmudgeToFile() method of the | ||
| 43 | +GitFilter structure in the current version of our "lfs" package. The | ||
| 44 | +original PointerSmudgeToFile() function made use of the "os" package's | ||
| 45 | +Create() function to create a new file or truncate an existing one, | ||
| 46 | +and the contemporary SmudgeToFile() method follows suit. | ||
| 47 | + | ||
| 48 | +For performance and compatibility reasons, Git does not try to | ||
| 49 | +completely eliminate all TOCTOU races involving symbolic links, and | ||
| 50 | +for similar reasons we do not expect to prevent every possible race | ||
| 51 | +which might allow the Git LFS client to unintentionally write through | ||
| 52 | +a symbolic link. We do, though, intend to limit the chances of this | ||
| 53 | +occurring as far as we reasonably can. | ||
| 54 | + | ||
| 55 | +Therefore, to address the problems with symbolic and hard links described | ||
| 56 | +above, we revise the SmudgeToFile() method so that it first removes any | ||
| 57 | +existing file at the path it is given, and if that succeeds, then attempts | ||
| 58 | +to atomically create a new file, reporting an error if that cannot be done | ||
| 59 | +because a file or other directory entry already exists at the same path. | ||
| 60 | + | ||
| 61 | +Specifically, we use the OpenFile() function from the "os" package | ||
| 62 | +instead of the Create() function, and we pass both the O_CREATE and | ||
| 63 | +O_EXCL flags to guarantee that the function either creates a new file | ||
| 64 | +or returns an error. Before calling OpenFile() we first call the | ||
| 65 | +"os" package's Remove() function and report an error if it fails for | ||
| 66 | +any reason other than that there is no file found at the given path. | ||
| 67 | + | ||
| 68 | +This approach mirrors that taken by Git when it updates files in the | ||
| 69 | +working tree. In particular, when the "git checkout" command is | ||
| 70 | +asked to update a specific pathspec (e.g., with a command such as | ||
| 71 | +"git checkout -- file.txt"), the checkout_entry_ca() function first | ||
| 72 | +calls the unlink(2) system call, and then the create_file() function | ||
| 73 | +invokes the open(2) system call with the O_CREATE and O_EXCL flags: | ||
| 74 | + | ||
| 75 | + https://github.com/git/git/blob/cb96e1697ad6e54d11fc920c95f82977f8e438f8/entry.c#L552-L578 | ||
| 76 | + https://github.com/git/git/blob/cb96e1697ad6e54d11fc920c95f82977f8e438f8/entry.c#L88-L89 | ||
| 77 | + | ||
| 78 | +Note that Git is actually more aggressive than the Git LFS client | ||
| 79 | +in how it handles conflicting content when checking out specific | ||
| 80 | +paths. For instance, if it finds a directory in place of a file it | ||
| 81 | +intends to write, its remove_subtree() function will be used to try | ||
| 82 | +to recursively remove the directory and all of its contents. | ||
| 83 | + | ||
| 84 | +By constrast, while our SmudgeToFile() method will now remove existing | ||
| 85 | +files (whether regular or irregular), symbolic links, and empty | ||
| 86 | +directories which conflict with the file it intends to create, the | ||
| 87 | +function will will not remove non-empty directories. | ||
| 88 | + | ||
| 89 | +Moreover, the SmudgeToFile() method will only take this action if one | ||
| 90 | +of these types of directory entries has been created in the brief | ||
| 91 | +time interval since the DecodePointerFromBlob() function was called, | ||
| 92 | +since we use that function to determine whether to proceed to call | ||
| 93 | +the SmudgeToFile() method. | ||
| 94 | + | ||
| 95 | +The sole caller of the SmudgeToFile() method is the RunToPath() | ||
| 96 | +method of the singleCheckout structure in our "commands" package, | ||
| 97 | +which is used only by the "git lfs checkout" and "git lfs pull" | ||
| 98 | +commands. Except when "git lfs checkout" is called with a --to | ||
| 99 | +option, the RunToPath() method is only called from the Run() method | ||
| 100 | +of the same singleCheckout structure. That method first invokes | ||
| 101 | +the DecodePointerFromBlob() function, and proceeds to call the | ||
| 102 | +SmudgeToFile() method only if no regular file was found, or if | ||
| 103 | +a regular file was found and its contained a valid Git LFS pointer | ||
| 104 | +whose ID matches that of the corresponding object. | ||
| 105 | + | ||
| 106 | +For this reason, we are guaranteed that when the SmudgeToFile() method | ||
| 107 | +is called, the path it is passed is either one provided by the user | ||
| 108 | +with the --to option of the "git lfs checkout" command, or has just | ||
| 109 | +been checked by the DecodePointerFromBlob() function. In either case | ||
| 110 | +we can be confident that it is reasonable to delete anything which now | ||
| 111 | +exists at that location. Note, too, that prior to the changes in this | ||
| 112 | +commit, any regular file or file referenced by a final symbolic link | ||
| 113 | +in the path would be truncated and overwritten regardless of its contents | ||
| 114 | +by the SmudgeToFile() method, so removing any directory entry (except | ||
| 115 | +for non-empty subdirectories) we find and creating a new file is not | ||
| 116 | +substantially different in this respect. | ||
| 117 | + | ||
| 118 | +However, there are several key advantages to our new approach. First, | ||
| 119 | +we can now be certain we will never dereference a final symbolic link | ||
| 120 | +in the given path and write to the link's target. Note, though, that | ||
| 121 | +we do still traverse symbolic links when they are found in place of | ||
| 122 | +directories in path segments other than the final segment. We will | ||
| 123 | +partially address this concern in a subsequent commit, with the same | ||
| 124 | +caveats that apply to Git's handling of symbolic links in non-terminal | ||
| 125 | +path segments. | ||
| 126 | + | ||
| 127 | +Second, by always creating a new file we can be certain the content | ||
| 128 | +we write will not be visible through hard links to an existing file. | ||
| 129 | +We therefore add a pair of new tests to our t/t-checkout.sh and | ||
| 130 | +t/t-pull.sh test suite which exercise the "git lfs checkout" and | ||
| 131 | +"git lfs pull" commands and confirm that they replace existing files | ||
| 132 | +with multiple hard links and effectively break those links. Both of | ||
| 133 | +our new tests use the assert_clean_status() test helper function to | ||
| 134 | +confirm that the "git lfs checkout" and "git lfs pull" commands continue | ||
| 135 | +to update the Git index entries for any Git LFS files they recreate in | ||
| 136 | +the working tree. | ||
| 137 | + | ||
| 138 | +We also expand the checks performed by the "checkout: conflicts" test | ||
| 139 | +in our t/t-checkout.sh test script to check that symbolic links as well | ||
| 140 | +as hard links are broken by our changes to the SmudgeToFile() method. | ||
| 141 | +We are able to use this test for this purpose because it runs the | ||
| 142 | +"git lfs checkout" command with the --to option, which means the | ||
| 143 | +Run() method of the singleCheckout structure is not used and the | ||
| 144 | +RunToPath() method is called directly. In turn, that implies that | ||
| 145 | +the DecodePointerFromBlob() function is never invoked, so the command | ||
| 146 | +does not simply detect the symbolic link in that function and therefore | ||
| 147 | +skip making a call to the SmudgeToFile() method, as occurs in our | ||
| 148 | +"checkout: skip file symlink conflicts" and "pull: skip file symlink | ||
| 149 | +conflicts" tests. Instead, the RunToPath() method calls the | ||
| 150 | +SmudgeToFile() method, which then removes the symbolic link and | ||
| 151 | +creates a new file in its place. Hence we can use this test to confirm | ||
| 152 | +that our changes are effective in breaking symbolic links as well as | ||
| 153 | +hard links. | ||
| 154 | + | ||
| 155 | +And third, our new approach means we can eliminate two calls to the | ||
| 156 | +"os" package's Chmod() function, which were added to the SmudgeToFile() | ||
| 157 | +method in commit 686bda3722f12293f345240532f666b6a0961bb2 of PR #3120 | ||
| 158 | +in order to handle pointer files to which our "lockable" Git attribute | ||
| 159 | +applies, but which the user has not yet locked, and so the pointer files | ||
| 160 | +have read-only permissions we want to retain while also replacing the | ||
| 161 | +file's contents with the corresponding Git LFS object data. | ||
| 162 | + | ||
| 163 | +We do not need to call the Chmod() function before invoking the | ||
| 164 | +Remove() function, because that function should be able to delete any | ||
| 165 | +existing file, even one with read-only permissions, so long as the | ||
| 166 | +parent directory permits changes to its list of entries. | ||
| 167 | + | ||
| 168 | +Note that our previous implementation might succeed even if the | ||
| 169 | +parent directory did not allow changes to its list of entries, and | ||
| 170 | +our new implementation will not. This does imply a partial change | ||
| 171 | +in the behaviour of the Git LFS client when directories in the | ||
| 172 | +working tree are themselves marked read-only. However, neither our | ||
| 173 | +old or new implementations could succeed in creating new files within | ||
| 174 | +such directories. Moreover, we expect Git working trees to normally | ||
| 175 | +have read-write directory permissions, since many regular Git commands | ||
| 176 | +will not function otherwise. We therefore consider the altered | ||
| 177 | +behaviour of the Git LFS client to be an acceptable change given | ||
| 178 | +that it will remediate several security concerns. | ||
| 179 | + | ||
| 180 | +We also do not need to call the Chmod() function at the end of the | ||
| 181 | +SmudgeToFile() method, because we instead pass the file permissions | ||
| 182 | +we want directly to the OpenFile() function. | ||
| 183 | + | ||
| 184 | +In the case where an existing file is found, prior to our deletion of | ||
| 185 | +that file, we read its permissions with the Lstat() function of the | ||
| 186 | +"os" package, and then pass those permissions to the OpenFile() | ||
| 187 | +function. If a symbolic link or some other type of directory entry | ||
| 188 | +is found, though, we ignore its permissions and use a default setting | ||
| 189 | +of 0666 instead. (On Unix systems, the current "umask" setting will | ||
| 190 | +then be applied to whatever permissions we pass to the OpenFile() | ||
| 191 | +function.) | ||
| 192 | + | ||
| 193 | +While the use of a default permissions mode of 0666 matches that used | ||
| 194 | +by the Create() function of the "os" package, and so aligns with the | ||
| 195 | +legacy behaviour of the Git LFS client, this is not actually the ideal | ||
| 196 | +implementation. Rather, we should respect the mode defined for the | ||
| 197 | +file in Git, which may have the executable mode set. For now, though, | ||
| 198 | +we leave this as improvement for a future PR, and just include a | ||
| 199 | +comment to remind us of this oversight in our implementation. | ||
| 200 | + | ||
| 201 | +Finally, because the "git lfs checkout" command will now attempt to | ||
| 202 | +remove and replace the file or other directory entry it finds at the | ||
| 203 | +path supplied with the --to option, we update our git-lfs-checkout(1) | ||
| 204 | +manual page to reflect this new behaviour. | ||
| 205 | +--- | ||
| 206 | + docs/man/git-lfs-checkout.adoc | 4 +- | ||
| 207 | + lfs/gitfilter_smudge.go | 26 +++++----- | ||
| 208 | + t/t-checkout.sh | 88 ++++++++++++++++++++++++++++++++++ | ||
| 209 | + t/t-pull.sh | 61 +++++++++++++++++++++++ | ||
| 210 | + 4 files changed, 165 insertions(+), 14 deletions(-) | ||
| 211 | + | ||
| 212 | +diff --git a/docs/man/git-lfs-checkout.adoc b/docs/man/git-lfs-checkout.adoc | ||
| 213 | +index 22339a0112..9746e6fbec 100644 | ||
| 214 | +--- a/docs/man/git-lfs-checkout.adoc | ||
| 215 | ++++ b/docs/man/git-lfs-checkout.adoc | ||
| 216 | + to a merge, this option checks out one of the three stages a conflicting | ||
| 217 | + Git LFS object into a separate file (which can be outside of the work | ||
| 218 | + tree). This can make using diff tools to inspect and resolve merges | ||
| 219 | + easier. A single Git LFS object's file path must be provided in | ||
| 220 | +-`<conflict-obj-path>`. | ||
| 221 | ++`<conflict-obj-path>`. If `<file>` already exists, whether as a regular | ||
| 222 | ++file, symbolic link, or directory, it will be removed and replaced, unless | ||
| 223 | ++it is a non-empty directory or otherwise cannot be deleted. | ||
| 224 | + | ||
| 225 | + If the installed Git version is at least 2.42.0, | ||
| 226 | + this command will by default check out Git LFS objects for files | ||
| 227 | +diff --git a/lfs/gitfilter_smudge.go b/lfs/gitfilter_smudge.go | ||
| 228 | +index 8298a50c5b..778cafc2f0 100644 | ||
| 229 | +--- a/lfs/gitfilter_smudge.go | ||
| 230 | ++++ b/lfs/gitfilter_smudge.go | ||
| 231 | + import ( | ||
| 232 | + func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, manifest tq.Manifest, cb tools.CopyCallback) error { | ||
| 233 | + tools.MkdirAll(filepath.Dir(filename), f.cfg) | ||
| 234 | + | ||
| 235 | +- if stat, _ := os.Stat(filename); stat != nil { | ||
| 236 | ++ // When no pointer file exists on disk, we should use the permissions | ||
| 237 | ++ // defined for the file in Git, since the executable mode may be set. | ||
| 238 | ++ // However, to conform with our legacy behaviour, we do not do this | ||
| 239 | ++ // at present. | ||
| 240 | ++ var mode os.FileMode = 0666 | ||
| 241 | ++ if stat, _ := os.Lstat(filename); stat != nil && stat.Mode().IsRegular() { | ||
| 242 | + if ptr.Size == 0 && stat.Size() == 0 { | ||
| 243 | + return nil | ||
| 244 | + } | ||
| 245 | + | ||
| 246 | +- if stat.Mode()&0200 == 0 { | ||
| 247 | +- if err := os.Chmod(filename, stat.Mode()|0200); err != nil { | ||
| 248 | +- return errors.Wrap(err, | ||
| 249 | +- tr.Tr.Get("Could not restore write permission")) | ||
| 250 | +- } | ||
| 251 | +- | ||
| 252 | +- // When we're done, return the file back to its normal | ||
| 253 | +- // permission bits. | ||
| 254 | +- defer os.Chmod(filename, stat.Mode()) | ||
| 255 | +- } | ||
| 256 | ++ mode = stat.Mode().Perm() | ||
| 257 | + } | ||
| 258 | + | ||
| 259 | + abs, err := filepath.Abs(filename) | ||
| 260 | + func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, m | ||
| 261 | + return errors.New(tr.Tr.Get("could not produce absolute path for %q", filename)) | ||
| 262 | + } | ||
| 263 | + | ||
| 264 | +- file, err := os.Create(abs) | ||
| 265 | ++ if err := os.Remove(abs); err != nil && !os.IsNotExist(err) { | ||
| 266 | ++ return errors.Wrap(err, tr.Tr.Get("could not remove working directory file %q", filename)) | ||
| 267 | ++ } | ||
| 268 | ++ | ||
| 269 | ++ file, err := os.OpenFile(abs, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) | ||
| 270 | + if err != nil { | ||
| 271 | +- return errors.New(tr.Tr.Get("could not create working directory file: %v", err)) | ||
| 272 | ++ return errors.Wrap(err, tr.Tr.Get("could not create working directory file %q", filename)) | ||
| 273 | + } | ||
| 274 | + defer file.Close() | ||
| 275 | + if _, err := f.Smudge(file, ptr, filename, download, manifest, cb); err != nil { | ||
| 276 | +diff --git a/t/t-checkout.sh b/t/t-checkout.sh | ||
| 277 | +index a142fc36ff..695bf4429e 100755 | ||
| 278 | +--- a/t/t-checkout.sh | ||
| 279 | ++++ b/t/t-checkout.sh | ||
| 280 | + begin_test "checkout: skip changed files" | ||
| 281 | + ) | ||
| 282 | + end_test | ||
| 283 | + | ||
| 284 | ++begin_test "checkout: break hard links to existing files" | ||
| 285 | ++( | ||
| 286 | ++ set -e | ||
| 287 | ++ | ||
| 288 | ++ reponame="checkout-break-file-hardlinks" | ||
| 289 | ++ setup_remote_repo "$reponame" | ||
| 290 | ++ clone_repo "$reponame" "$reponame" | ||
| 291 | ++ | ||
| 292 | ++ git lfs track "*.dat" | ||
| 293 | ++ | ||
| 294 | ++ contents="a" | ||
| 295 | ++ contents_oid="$(calc_oid "$contents")" | ||
| 296 | ++ mkdir -p dir1/dir2/dir3 | ||
| 297 | ++ printf "%s" "$contents" >a.dat | ||
| 298 | ++ printf "%s" "$contents" >dir1/dir2/dir3/a.dat | ||
| 299 | ++ | ||
| 300 | ++ git add .gitattributes a.dat dir1 | ||
| 301 | ++ git commit -m "initial commit" | ||
| 302 | ++ | ||
| 303 | ++ git push origin main | ||
| 304 | ++ assert_server_object "$reponame" "$contents_oid" | ||
| 305 | ++ | ||
| 306 | ++ cd .. | ||
| 307 | ++ GIT_LFS_SKIP_SMUDGE=1 git clone "$GITSERVER/$reponame" "${reponame}-assert" | ||
| 308 | ++ | ||
| 309 | ++ cd "${reponame}-assert" | ||
| 310 | ++ git lfs fetch origin main | ||
| 311 | ++ | ||
| 312 | ++ assert_local_object "$contents_oid" 1 | ||
| 313 | ++ | ||
| 314 | ++ rm -f a.dat dir1/dir2/dir3/a.dat ../link | ||
| 315 | ++ pointer="$(git cat-file -p ":a.dat")" | ||
| 316 | ++ echo "$pointer" >../link | ||
| 317 | ++ ln ../link a.dat | ||
| 318 | ++ ln ../link dir1/dir2/dir3/a.dat | ||
| 319 | ++ | ||
| 320 | ++ git lfs checkout | ||
| 321 | ++ | ||
| 322 | ++ [ "$contents" = "$(cat a.dat)" ] | ||
| 323 | ++ [ "$contents" = "$(cat dir1/dir2/dir3/a.dat)" ] | ||
| 324 | ++ [ "$pointer" = "$(cat ../link)" ] | ||
| 325 | ++ assert_clean_status | ||
| 326 | ++ | ||
| 327 | ++ rm a.dat dir1/dir2/dir3/a.dat | ||
| 328 | ++ ln ../link a.dat | ||
| 329 | ++ ln ../link dir1/dir2/dir3/a.dat | ||
| 330 | ++ | ||
| 331 | ++ pushd dir1/dir2 | ||
| 332 | ++ git lfs checkout | ||
| 333 | ++ popd | ||
| 334 | ++ | ||
| 335 | ++ [ "$contents" = "$(cat a.dat)" ] | ||
| 336 | ++ [ "$contents" = "$(cat dir1/dir2/dir3/a.dat)" ] | ||
| 337 | ++ [ "$pointer" = "$(cat ../link)" ] | ||
| 338 | ++ assert_clean_status | ||
| 339 | ++) | ||
| 340 | ++end_test | ||
| 341 | ++ | ||
| 342 | + begin_test "checkout: without clean filter" | ||
| 343 | + ( | ||
| 344 | + set -e | ||
| 345 | + begin_test "checkout: conflicts" | ||
| 346 | + echo "abc123" | cmp - "$abs_assert_dir/link1/dir2/theirs.txt" | ||
| 347 | + } | ||
| 348 | + | ||
| 349 | ++ rm -f base.txt link1 ../ours.txt ../link2 | ||
| 350 | ++ ln -s link1 base.txt | ||
| 351 | ++ ln -s link2 ../ours.txt | ||
| 352 | ++ | ||
| 353 | ++ git lfs checkout --to base.txt --base file1.dat | ||
| 354 | ++ git lfs checkout --to ../ours.txt --ours file1.dat | ||
| 355 | ++ | ||
| 356 | ++ [ ! -L "base.txt" ] | ||
| 357 | ++ [ ! -L "../ours.txt" ] | ||
| 358 | ++ [ ! -e "link1" ] | ||
| 359 | ++ [ ! -e "../link2" ] | ||
| 360 | ++ echo "file1.dat" | cmp - base.txt | ||
| 361 | ++ echo "def456" | cmp - ../ours.txt | ||
| 362 | ++ | ||
| 363 | ++ rm -f base.txt link1 ../ours.txt ../link2 | ||
| 364 | ++ printf "link1" >link1 | ||
| 365 | ++ printf "link2" >../link2 | ||
| 366 | ++ ln link1 base.txt | ||
| 367 | ++ ln ../link2 ../ours.txt | ||
| 368 | ++ | ||
| 369 | ++ git lfs checkout --to base.txt --base file1.dat | ||
| 370 | ++ git lfs checkout --to ../ours.txt --ours file1.dat | ||
| 371 | ++ | ||
| 372 | ++ [ -f "link1" ] | ||
| 373 | ++ [ -f "../link2" ] | ||
| 374 | ++ [ "link1" = "$(cat link1)" ] | ||
| 375 | ++ [ "link2" = "$(cat ../link2)" ] | ||
| 376 | ++ echo "file1.dat" | cmp - base.txt | ||
| 377 | ++ echo "def456" | cmp - ../ours.txt | ||
| 378 | ++ | ||
| 379 | + git lfs checkout --to base.txt --ours other.txt 2>&1 | tee output.txt | ||
| 380 | + grep 'Could not find decoder pointer for object' output.txt | ||
| 381 | + popd > /dev/null | ||
| 382 | +diff --git a/t/t-pull.sh b/t/t-pull.sh | ||
| 383 | +index 65b3a50a65..802c17c869 100644 | ||
| 384 | +--- a/t/t-pull.sh | ||
| 385 | ++++ b/t/t-pull.sh | ||
| 386 | + begin_test "pull: skip changed files" | ||
| 387 | + ) | ||
| 388 | + end_test | ||
| 389 | + | ||
| 390 | ++begin_test "pull: break hard links to existing files" | ||
| 391 | ++( | ||
| 392 | ++ set -e | ||
| 393 | ++ | ||
| 394 | ++ reponame="pull-break-file-hardlinks" | ||
| 395 | ++ setup_remote_repo "$reponame" | ||
| 396 | ++ clone_repo "$reponame" "$reponame" | ||
| 397 | ++ | ||
| 398 | ++ git lfs track "*.dat" | ||
| 399 | ++ | ||
| 400 | ++ contents="a" | ||
| 401 | ++ contents_oid="$(calc_oid "$contents")" | ||
| 402 | ++ mkdir -p dir1/dir2/dir3 | ||
| 403 | ++ printf "%s" "$contents" >a.dat | ||
| 404 | ++ printf "%s" "$contents" >dir1/dir2/dir3/a.dat | ||
| 405 | ++ | ||
| 406 | ++ git add .gitattributes a.dat dir1 | ||
| 407 | ++ git commit -m "initial commit" | ||
| 408 | ++ | ||
| 409 | ++ git push origin main | ||
| 410 | ++ assert_server_object "$reponame" "$contents_oid" | ||
| 411 | ++ | ||
| 412 | ++ cd .. | ||
| 413 | ++ GIT_LFS_SKIP_SMUDGE=1 git clone "$GITSERVER/$reponame" "${reponame}-assert" | ||
| 414 | ++ | ||
| 415 | ++ cd "${reponame}-assert" | ||
| 416 | ++ refute_local_object "$contents_oid" 1 | ||
| 417 | ++ | ||
| 418 | ++ rm -f a.dat dir1/dir2/dir3/a.dat ../link | ||
| 419 | ++ pointer="$(git cat-file -p ":a.dat")" | ||
| 420 | ++ echo "$pointer" >../link | ||
| 421 | ++ ln ../link a.dat | ||
| 422 | ++ ln ../link dir1/dir2/dir3/a.dat | ||
| 423 | ++ | ||
| 424 | ++ git lfs pull | ||
| 425 | ++ assert_local_object "$contents_oid" 1 | ||
| 426 | ++ | ||
| 427 | ++ [ "$contents" = "$(cat a.dat)" ] | ||
| 428 | ++ [ "$contents" = "$(cat dir1/dir2/dir3/a.dat)" ] | ||
| 429 | ++ [ "$pointer" = "$(cat ../link)" ] | ||
| 430 | ++ assert_clean_status | ||
| 431 | ++ | ||
| 432 | ++ rm a.dat dir1/dir2/dir3/a.dat | ||
| 433 | ++ ln ../link a.dat | ||
| 434 | ++ ln ../link dir1/dir2/dir3/a.dat | ||
| 435 | ++ | ||
| 436 | ++ rm -rf .git/lfs/objects | ||
| 437 | ++ | ||
| 438 | ++ pushd dir1/dir2 | ||
| 439 | ++ git lfs pull | ||
| 440 | ++ popd | ||
| 441 | ++ | ||
| 442 | ++ assert_local_object "$contents_oid" 1 | ||
| 443 | ++ | ||
| 444 | ++ [ "$contents" = "$(cat a.dat)" ] | ||
| 445 | ++ [ "$contents" = "$(cat dir1/dir2/dir3/a.dat)" ] | ||
| 446 | ++ [ "$pointer" = "$(cat ../link)" ] | ||
| 447 | ++ assert_clean_status | ||
| 448 | ++) | ||
| 449 | ++end_test | ||
| 450 | ++ | ||
| 451 | + begin_test "pull without clean filter" | ||
| 452 | + ( | ||
| 453 | + set -e | ||
| @@ -0,0 +1,658 @@ | |||
| 1 | +From d02bd13f02ef76f6807581cd6b34709069cb3615 Mon Sep 17 00:00:00 2001 | ||
| 2 | +From: Chris Darroch <chrisd8088@github.com> | ||
| 3 | +Date: Wed, 13 Aug 2025 00:24:02 -0700 | ||
| 4 | +Subject: [PATCH] fix bare repo pull/checkout path handling bug | ||
| 5 | + | ||
| 6 | +Our "git lfs checkout" and "git lfs pull" commands may both, at present, | ||
| 7 | +be executed in a bare repository, although the former has no utility in | ||
| 8 | +a bare repository, and the latter often performs no actions, but can be | ||
| 9 | +used to fetch Git LFS objects in a bare repository. | ||
| 10 | + | ||
| 11 | +The "git lfs checkout" and "git lfs pull" commands are the only commands | ||
| 12 | +which make use of the methods of the singleCheckout structure in our | ||
| 13 | +"commands" package, and in a subsequent commit we will update these methods | ||
| 14 | +so they change the current working directory to the root of the current | ||
| 15 | +working tree, so long as one exists. | ||
| 16 | + | ||
| 17 | +Before we make these revisions, though, we first need to guarantee that | ||
| 18 | +the singleCheckout structure's methods correctly handle the case where no | ||
| 19 | +current work tree is defined, such as in a bare repository when the | ||
| 20 | +GIT_WORK_TREE environment variable has not been set. | ||
| 21 | + | ||
| 22 | +When no working tree is defined, the "git lfs pull" command should perform | ||
| 23 | +no action other than fetching objects, since there is no work tree into | ||
| 24 | +which the command should write any Git LFS file content. | ||
| 25 | + | ||
| 26 | +For the same reason, the "git lfs checkout" should have no effect when | ||
| 27 | +no work tree is defined, since the command's only purpose is to check | ||
| 28 | +out Git LFS file content into a working tree. | ||
| 29 | + | ||
| 30 | +Unfortunately, both the "git lfs checkout" and "git lfs pull" commands | ||
| 31 | +may, under unusual circumstances, try to check out Git LFS files by | ||
| 32 | +writing their object data into files either inside or outside a | ||
| 33 | +bare repository. | ||
| 34 | + | ||
| 35 | +In bare repositories, when the "git lfs checkout" and "git lfs pull" | ||
| 36 | +commands try to determine whether to check out a Git LFS file into the | ||
| 37 | +(non-existent) working tree, they incorrectly treat the path to a file | ||
| 38 | +from the root of the repository as if it were instead an absolute path | ||
| 39 | +starting from the root of the current filesystem. For instance, given | ||
| 40 | +the path "foo/bar.bin" to a Git LFS file in a repository, the commands | ||
| 41 | +will instead treat this path as if it were the path "/foo/bar.bin". | ||
| 42 | + | ||
| 43 | +Normally, no file will exist at this location, so the "git lfs checkout" | ||
| 44 | +and "git lfs pull" commands then check the Git index to try to determine | ||
| 45 | +whether the user has staged the file for deletion. Since bare | ||
| 46 | +repositories typically have no index, the commands will assume the user | ||
| 47 | +has intentionally removed the file, and skip any further processing for | ||
| 48 | +the file. | ||
| 49 | + | ||
| 50 | +If the user has added an index entry for the file, though, the commands | ||
| 51 | +will assume the file should be re-created in the (non-existent) working | ||
| 52 | +tree with the content of the object referenced by the Git LFS pointer | ||
| 53 | +stored in Git's version of the file. Taking the file's path from the | ||
| 54 | +root of the repository as if it was an absolute path, the commands will | ||
| 55 | +try to create any missing directories in that path, and then try to | ||
| 56 | +either create a new file or truncate an existing one before writing | ||
| 57 | +the Git LFS object content into the file. | ||
| 58 | + | ||
| 59 | +An alternative sequence of events which leads to the same result may | ||
| 60 | +occur in the extremely unlikely case that a file already exists at | ||
| 61 | +the location specified by the incorrectly-determined absolute path, | ||
| 62 | +and that the file contains a Git LFS pointer with the same object ID | ||
| 63 | +as that of the given file in the repository. In other words, using | ||
| 64 | +the same example file paths as above, this means a "/foo/bar.bin" file | ||
| 65 | +would have to already exist and contain the same raw Git LFS pointer | ||
| 66 | +data as the "foo/bar.bin" file in the Git repository. Should this | ||
| 67 | +happen, the "git lfs checkout" and "git lfs pull" commands would assume | ||
| 68 | +the file should be overwritten with the contents of the corresponding | ||
| 69 | +Git LFS object. | ||
| 70 | + | ||
| 71 | +Of course, even if the "git lfs checkout" and "git lfs pull" commands | ||
| 72 | +try to create or overwrite a file at the path they are incorrectly | ||
| 73 | +treating as an absolute path, the current user may not have sufficient | ||
| 74 | +permissions to permit the necessary filesystem operations to complete. | ||
| 75 | + | ||
| 76 | +Regardless, the Git LFS client should not try to read or write files | ||
| 77 | +outside of the current repository unless specifically requested to do | ||
| 78 | +so with an argument such as the --to option of the "git lfs checkout" | ||
| 79 | +command. | ||
| 80 | + | ||
| 81 | +In conjunction with our remediation of the vulnerability assigned the | ||
| 82 | +identifier CVE-2025-26625, we therefore revise the "git lfs checkout" | ||
| 83 | +and "git lfs pull" commands now to ensure they will never treat paths | ||
| 84 | +relative to the root of the current repository as if they were absolute | ||
| 85 | +filesystem paths. | ||
| 86 | + | ||
| 87 | +We also adjust the "git lfs checkout" command so that it generates the | ||
| 88 | +same error message as commands like "git lfs status" when no working | ||
| 89 | +tree is defined, and exits immediately afterwards. This change will | ||
| 90 | +make clear to our users why the "git lfs checkout" command has no effect | ||
| 91 | +in a bare repository, while also simplifying our test requirements as we | ||
| 92 | +do not have to verify the command's behaviour in a bare repository beyond | ||
| 93 | +checking that it exits with the appropriate warning message. | ||
| 94 | + | ||
| 95 | +The specific problem addressed in this commit is the result of the | ||
| 96 | +joining an empty path, which signals the lack of a current working | ||
| 97 | +tree, to a file's path from the root of the repository, and adding | ||
| 98 | +a file separator character between the two strings. This occurs | ||
| 99 | +within the Convert() method of the repoToCurrentPathConverter structure | ||
| 100 | +type from our "lfs" package. | ||
| 101 | + | ||
| 102 | +In a subsequent commit we will be able to remove the | ||
| 103 | +repoToCurrentPathConverter structure and its methods entirely, when we | ||
| 104 | +revise the "git lfs checkout" and "git lfs pull" commands to change | ||
| 105 | +the current working directory to the root of the current working tree. | ||
| 106 | + | ||
| 107 | +In this commit, however, we simply alter the commands so that they | ||
| 108 | +never call the structure's Convert() method if no working tree exists. | ||
| 109 | + | ||
| 110 | +First, we add a "hasWorkTree" element to the singleCheckout structure | ||
| 111 | +type in our "commands" package, and when we initialize a new structure | ||
| 112 | +in the newSingleCheckout() function, we set the "hasWorkTree" element's | ||
| 113 | +value to "true" only if the LocalWorkingDir() method of the Configuration | ||
| 114 | +structure type from our "config" package returns a non-empty path. | ||
| 115 | + | ||
| 116 | +The LocalWorkingDir() method returns the absolute path to the root of | ||
| 117 | +the current working tree, or an empty path if no working tree is defined, | ||
| 118 | +as determined by the GitAndRootDirs() function in our "git" package. | ||
| 119 | +The GitAndRootDirs() function runs the "git rev-parse" command with the | ||
| 120 | +--show-toplevel option, and then interprets that command's output and | ||
| 121 | +exit code so that if no working tree is defined, an empty path is | ||
| 122 | +returned instead of a path to the work tree's root directory. | ||
| 123 | + | ||
| 124 | +Second, we update the Run() method of the singleCheckout structure | ||
| 125 | +so that it returns immediately unless the "hasWorkTree" element is | ||
| 126 | +set to a "true" value, meaning a work tree exists and it is safe to | ||
| 127 | +create and write files within that directory tree. | ||
| 128 | + | ||
| 129 | +To verify these changes work as we expect, we introduce a new "pull: bare | ||
| 130 | +repository" test to our t/t-pull.sh test script, and in this test we | ||
| 131 | +specifically add a Git LFS pointer file to the test repository at a path | ||
| 132 | +that, if treated as an absolute path instead of a path from the root of | ||
| 133 | +the repository, could be created by the current test process. After the | ||
| 134 | +test clones the repository, it adds this file's path to the index, runs | ||
| 135 | +the "git lfs pull" command, and then checks that no file is created either | ||
| 136 | +inside the bare repository or, most importantly, outside the repository. | ||
| 137 | + | ||
| 138 | +(The test also ensures that a Git LFS filter attribute is defined in | ||
| 139 | +the "$GIT_DIR/info/attributes" file, which guarantees that regardless | ||
| 140 | +of which Git version is installed, the "git lfs pull" command will find | ||
| 141 | +our new Git LFS pointer file in the repository's contents and process it. | ||
| 142 | +We describe the issues pertaining to the need to use a local Git | ||
| 143 | +attributes file instead of a ".gitattributes" file further below.) | ||
| 144 | + | ||
| 145 | +Without our changes to the singleCheckout structure and its methods | ||
| 146 | +in this commit, the revised "pull: bare repository" test will fail, so | ||
| 147 | +we can be confident that it validates that our remediation is effective. | ||
| 148 | + | ||
| 149 | +As for the "git lfs checkout" command, we alter its main checkoutCommand() | ||
| 150 | +function so that after calling the setupRepository() function, the | ||
| 151 | +checkoutCommand() function checks whether a path to the current working | ||
| 152 | +tree has been found, and if not, outputs a warning message and stops | ||
| 153 | +execution of the command. This new check is modelled on that performed | ||
| 154 | +by the requireWorkingCopy() function, but causes the command to return a | ||
| 155 | +zero (i.e., successful) exit code rather than a non-zero one. | ||
| 156 | + | ||
| 157 | +Our new check relies on the functions invoked by the setupRepository() | ||
| 158 | +function to have already called the GitAndRootDirs() function in our | ||
| 159 | +"git" package. That function runs the "git rev-parse" command with the | ||
| 160 | +--show-toplevel option, and then interprets the command's output and exit | ||
| 161 | +code so that if no current work tree is present, an empty path will be | ||
| 162 | +returned by the LocalWorkingDir() method of our "config" package's | ||
| 163 | +Configuration structure instead of a path to the work tree's root | ||
| 164 | +directory. | ||
| 165 | + | ||
| 166 | +It would be more straightforward for us to revise the checkoutCommand() | ||
| 167 | +function to simply call the setupWorkingCopy() function rather than the | ||
| 168 | +setupRepository() function, because the setupWorkingCopy() function calls | ||
| 169 | +the requireWorkingCopy() function and so would enforce the presence of | ||
| 170 | +a working tree in the same manner as we employ in other commands such as | ||
| 171 | +the "git lfs status" and "git lfs track" commands. | ||
| 172 | + | ||
| 173 | +However, this implementation would result in a backwards-incompatible | ||
| 174 | +change to the behaviour the "git lfs checkout" command when it is run in a | ||
| 175 | +bare repository, which could result in the unexpected failure of automated | ||
| 176 | +CI jobs, for instance. Although the use of the "git lfs checkout" command | ||
| 177 | +in a bare repository has no purpose, we defer the simpler implementation | ||
| 178 | +to a future release, and for now ensure that the command still returns | ||
| 179 | +a zero exit code when run in a bare repository. | ||
| 180 | + | ||
| 181 | +We do, though, update our git-lfs-checkout(1) manual page to clarify that | ||
| 182 | +the command requires a working tree, and that in the future the command | ||
| 183 | +may exit with an error when run in a bare repository. We also add a new | ||
| 184 | +"checkout: bare repository" test to our t/t-checkout.sh test script, which | ||
| 185 | +just verifies that the command generates the expected error message and | ||
| 186 | +returns a zero exit code when it is run in a bare repository. | ||
| 187 | + | ||
| 188 | +Both the "git lfs checkout" and "git lfs pull" commands currently exhibit | ||
| 189 | +the erroneous behaviour addressed by this commit because the singleCheckout | ||
| 190 | +structure's Run() method relies on the Convert() method of the | ||
| 191 | +repoToCurrentPathConverter structure type to rewrite file paths | ||
| 192 | +relative to the root of the repository into paths relative to the | ||
| 193 | +current working directory, and this method returns invalid paths when | ||
| 194 | +no working tree is defined, as is the case in a bare repository. | ||
| 195 | + | ||
| 196 | +The Run() method is executed, either directly or indirectly, for | ||
| 197 | +each Git LFS pointer file path found by the ScanLFSFiles() method of | ||
| 198 | +the GitScanner structure in our "lfs" package. This method retrieves | ||
| 199 | +a list of files from Git, and for each one that corresponds to a Git | ||
| 200 | +LFS pointer, the method invokes an anonymous function which in turn | ||
| 201 | +causes the Run() method to be performed. | ||
| 202 | + | ||
| 203 | +In the case of the "git lfs pull" command, if a local copy of the object | ||
| 204 | +associated with a Git LFS pointer is found, the Run() method is invoked | ||
| 205 | +directly within the anonymous function, and otherwise it is invoked by | ||
| 206 | +a goroutine for each object whose data is successfully retrieved from | ||
| 207 | +the Git LFS remote by the transfer queue. | ||
| 208 | + | ||
| 209 | +In the case of the "git lfs checkout" command, the anonymous function | ||
| 210 | +called by the ScanLFSFiles() method appends each Git LFS pointer to a | ||
| 211 | +slice, and then the Run() method is invoked for each pointer in the | ||
| 212 | +slice after the scan through the list of files is complete. | ||
| 213 | + | ||
| 214 | +To retrieve a list of files from Git, the runScanLFSFiles() function, | ||
| 215 | +which is called by the ScanLFSFiles() method, uses one of two Git | ||
| 216 | +commands. If the installed version of Git is 2.42.0 or higher, the | ||
| 217 | +"git ls-files" command is executed, and otherwise the "git ls-tree" | ||
| 218 | +command is used. This difference accounts for one of the reasons | ||
| 219 | +why the "git lfs pull" command, in particular, may perform no action | ||
| 220 | +when run within a bare repository. | ||
| 221 | + | ||
| 222 | +Specifically, as noted in issue #6004, the "git ls-files" command lists | ||
| 223 | +the files in the Git index, while the "git ls-tree" command lists the | ||
| 224 | +files in the Git tree associated with a given reference, which in the | ||
| 225 | +case of our "git lfs checkout" and "git lfs pull" commands is always | ||
| 226 | +the current "HEAD" symbolic reference. | ||
| 227 | + | ||
| 228 | +If the installed version of Git is older than v2.42.0, when our commands | ||
| 229 | +run the "git ls-tree" command they will receive a list of files from | ||
| 230 | +the Git tree associated with the "HEAD" reference, and will process | ||
| 231 | +any Git LFS pointers found in that list. (Note that pointer files | ||
| 232 | +will be processed even if they no longer match any Git LFS filter | ||
| 233 | +attributes; for instance, if there are no ".gitattributes" files in | ||
| 234 | +the index or in the Git tree associated with the "HEAD" reference, | ||
| 235 | +and no local Git attributes files.) | ||
| 236 | + | ||
| 237 | +If the installed version of Git is at least v2.42.0, our commands | ||
| 238 | +run the "git ls-files" command instead of the "git ls-tree" command. | ||
| 239 | +For two separate reasons, in a bare repository the "git ls-files" | ||
| 240 | +command will often return an empty list, so our "git lfs checkout" | ||
| 241 | +and "git lfs pull" commands will take no further action. | ||
| 242 | + | ||
| 243 | +The more obvious reason is that by default, Git creates bare | ||
| 244 | +repositories without an index, so unless the user has explicitly | ||
| 245 | +added entries to the index for Git LFS pointer files, no results | ||
| 246 | +will be returned by the "git ls-files" command. | ||
| 247 | + | ||
| 248 | +The less obvious reason is due to the "attr:filter=lfs" pathspec | ||
| 249 | +our commands pass to the "git ls-files" command, which causes the | ||
| 250 | +command to only return paths for files which match a Git LFS filter | ||
| 251 | +attribute definition. However, in a bare repository Git's internal | ||
| 252 | +read_attr() function by default ignores all ".gitattributes" files | ||
| 253 | +found in either the index or the tree associated with the "HEAD" | ||
| 254 | +reference: | ||
| 255 | + | ||
| 256 | + https://github.com/git/git/blob/v2.50.1/attr.c#L851-L867 | ||
| 257 | + | ||
| 258 | +Since there is no working tree in a bare repository, this means all | ||
| 259 | +".gitattributes" files are ignored by default, and because users | ||
| 260 | +typically define Git LFS attributes in those files, the "git ls-files" | ||
| 261 | +command will not match any files even if entries for Git LFS pointer | ||
| 262 | +files have been added to the index. Users would have to specifically | ||
| 263 | +set the GIT_ATTR_SOURCE environment variable to a reference like "HEAD" | ||
| 264 | +or add Git LFS filter attributes to a local Git attributes file such | ||
| 265 | +as the "$GIT_DIR/info/attributes" file in order for the "git ls-files" | ||
| 266 | +command to match pointer files in the index to the "attr:filter=lfs" | ||
| 267 | +pathspec and return a non-empty list. | ||
| 268 | + | ||
| 269 | +Regardless of the source, though, if Git LFS pointers are identified from | ||
| 270 | +the list of files returned by Git, the "git lfs pull" command will fetch | ||
| 271 | +the objects referenced by those pointers unless the objects already exist | ||
| 272 | +in the local storage directories under "lfs/objects". (Note that in a | ||
| 273 | +bare repository, the usual leading ".git" directory is not necessary.) | ||
| 274 | + | ||
| 275 | +As objects are fetched by the transfer queue, the separate goroutine | ||
| 276 | +started by the "git lfs pull" command passes their pointer data to the | ||
| 277 | +Run() method of the singleCheckout structure, one pointer at a time. | ||
| 278 | + | ||
| 279 | +In a "git lfs checkout" command, by contrast, no objects are fetched, | ||
| 280 | +and the Run() method is instead invoked directly by the command's main | ||
| 281 | +function for each Git LFS pointer file path collected during the | ||
| 282 | +execution of the ScanLFSFiles() method. | ||
| 283 | + | ||
| 284 | +As described above, the Run() method begins by converting the file path | ||
| 285 | +of the Git LFS pointer provided in its "p" parameter into a file path | ||
| 286 | +relative to the current working directory using the Convert() method of | ||
| 287 | +the repoToCurrentPathConverter structure type. | ||
| 288 | + | ||
| 289 | +We initialize a structure of that type in the newSingleCheckout() | ||
| 290 | +function by calling the NewRepoToCurrentPathConverter() function. | ||
| 291 | +That function uses an internal function named pathConverterArgs() to set | ||
| 292 | +the new structure's "repoDir" element to the file path returned by the | ||
| 293 | +LocalWorkingDir() method of the Configuration structure type, which as | ||
| 294 | +mentioned above will be an empty path if no current work tree is defined. | ||
| 295 | + | ||
| 296 | +When the repoToCurrentPathConverter structure type's Convert() method | ||
| 297 | +is called, it first joins the structure's "repoDir" element to the | ||
| 298 | +file path provided in the method's "p" parameter using a local wrapper | ||
| 299 | +function around the Join() function from the Go standard library's | ||
| 300 | +"strings" package, rather than the Join() function from the | ||
| 301 | +"path/filepath" package. (This change was made in commit | ||
| 302 | +fd69029c76e3898fc7c81ac2e8705174c4ebf2b5 of PR #2875, presumably to | ||
| 303 | +make more efficient the handling of file paths which we expect to | ||
| 304 | +always be defined.) | ||
| 305 | + | ||
| 306 | +In a bare repository, however, the "repoDir" element contains an empty | ||
| 307 | +path, so the result of joining it with a file path relative to the root | ||
| 308 | +of the repository using the Join() function from the "strings" package | ||
| 309 | +is the same file path but with a leading "/" character prepended to it, | ||
| 310 | +in effect creating an invalid absolute path from the root of the | ||
| 311 | +filesystem. Note that if the Join() function from the "path/filepath" | ||
| 312 | +package was used instead, it ignores empty parameters, so the file path | ||
| 313 | +would be returned unchanged. | ||
| 314 | + | ||
| 315 | +The Convert() method then passes this invalid absolute path to the | ||
| 316 | +Rel() function of the "path/filepath" package of the Go standard library, | ||
| 317 | +along with the absolute path to the current working directory. We expect | ||
| 318 | +this call to return a relative path from the current working directory | ||
| 319 | +to the location within the current Git work tree where a file should | ||
| 320 | +be created or updated with the contents of a Git LFS object. | ||
| 321 | + | ||
| 322 | +In a bare repository, though, what is returned by the Convert() method | ||
| 323 | +to the Run() method is a relative path from the current working directory | ||
| 324 | +to a location constructed by treating a Git LFS pointer's path within | ||
| 325 | +the repository as if it was a path descending from the root of the | ||
| 326 | +current filesystem. For instance, given the path "foo/bar" of a | ||
| 327 | +Git LFS pointer within the repository, and current working directory | ||
| 328 | +of "/path/to/bare/repo", the Convert() method would return the path | ||
| 329 | +"../../../../foo/bar". | ||
| 330 | + | ||
| 331 | +After this path is returned to the Run() method, it is passed to the | ||
| 332 | +DecodePointerFromFile() function in our "lfs" package, which checks | ||
| 333 | +whether a file exists at the given location, and if so, reads it and | ||
| 334 | +checks whether it contains a valid Git LFS pointer. | ||
| 335 | + | ||
| 336 | +In the large majority of cases, of course, files will not exist in | ||
| 337 | +the locations identifed by the invalid paths that the Convert() method | ||
| 338 | +generates when the "git lfs checkout" or "git lfs pull" commands are | ||
| 339 | +executed in a bare repository. Hence the DecodePointerFromFile() | ||
| 340 | +function will return an error which the IsNotExist() function of the | ||
| 341 | +"os" package considers equivalent to an ErrNotExist error. The Run() | ||
| 342 | +method will then execute a "git diff-index" command to determine whether | ||
| 343 | +the user has intentionally removed the file from the Git index, and | ||
| 344 | +will pass the original file path (the one relative to the root of the | ||
| 345 | +repository) to that command. | ||
| 346 | + | ||
| 347 | +If the installed version of Git is older than v2.42.0, and the bare | ||
| 348 | +repository has no index, as is normally the case in such repositories, | ||
| 349 | +the "git diff-index" command's output will indicate that the file does | ||
| 350 | +not exist in the index and so the Run() method will return without | ||
| 351 | +taking further action. | ||
| 352 | + | ||
| 353 | +On the other hand, if the installed version of Git is 2.42.0 or higher, | ||
| 354 | +then the index must include an entry for the original file path (the | ||
| 355 | +one relative to the root of the repository), since otherwise the | ||
| 356 | +"git ls-files" command would not have listed the file at all and the | ||
| 357 | +Run() method would never have been called. Thus the "git diff-index" | ||
| 358 | +command will also list the file as present in the index, and so the | ||
| 359 | +Run() method will proceed on the assumption that the file is just | ||
| 360 | +missing in the (non-existent) working tree and should be created, even | ||
| 361 | +though there is no actual work tree. | ||
| 362 | + | ||
| 363 | +Even if a version of Git older than 2.42.0 is installed, though, | ||
| 364 | +the user may have created an index entry for the file, in which case | ||
| 365 | +the Run() method will likewise proceed because the "git diff-index" | ||
| 366 | +command's output will indicate that the file is present in the index. | ||
| 367 | + | ||
| 368 | +It is also possible, although extremely unlikely, that the | ||
| 369 | +DecodePointerFromFile() function finds a file at the incorrectly- | ||
| 370 | +generated path it was given, and is able to open it and parse it as a | ||
| 371 | +valid Git LFS pointer. The Run() method will then check to see if the | ||
| 372 | +pointer's ID matches that of the pointer under consideration. If it | ||
| 373 | +does not, the method will return without taking action, but if it does, | ||
| 374 | +it will proceed on the assumption that the pointer file should be | ||
| 375 | +overwritten with the contents of the associated object file. | ||
| 376 | + | ||
| 377 | +In summary, in a bare repository the singleCheckout structure's Run() | ||
| 378 | +method will only proceed under one of two conditions: either a Git index | ||
| 379 | +entry exists for the file path under consideration, which is unlikely | ||
| 380 | +to be the case in a bare repository since the index is typically empty, | ||
| 381 | +or a Git LFS pointer file with the expected object ID happens to exist | ||
| 382 | +at the absolute path derived by prepending a file separator to the | ||
| 383 | +file's path within the repository, which is even more unlikely. | ||
| 384 | + | ||
| 385 | +Should one of these circumstances occur, though, the Run() method | ||
| 386 | +will invoke the RunToPath() method of the singleCheckout structure, | ||
| 387 | +which will in turn call the SmudgeToFile() method of the GitFilter | ||
| 388 | +structure in our "lfs" package. That method will attempt to create | ||
| 389 | +or truncate a file at the incorrect path, and then write the contents | ||
| 390 | +of a Git LFS object into the file. | ||
| 391 | + | ||
| 392 | +With the changes in this commit, however, this incorrect behaviour | ||
| 393 | +should no longer occur under any circumstances. | ||
| 394 | +--- | ||
| 395 | + commands/command_checkout.go | 9 +++ | ||
| 396 | + commands/pull.go | 6 ++ | ||
| 397 | + docs/man/git-lfs-checkout.adoc | 3 + | ||
| 398 | + docs/man/git-lfs-pull.adoc | 10 +++ | ||
| 399 | + t/t-checkout.sh | 17 +++++ | ||
| 400 | + t/t-pull.sh | 131 +++++++++++++++++++++++++++++++++ | ||
| 401 | + 6 files changed, 176 insertions(+) | ||
| 402 | + | ||
| 403 | +diff --git a/commands/command_checkout.go b/commands/command_checkout.go | ||
| 404 | +index 6bf9534ceb..71ecef9c2e 100644 | ||
| 405 | +--- a/commands/command_checkout.go | ||
| 406 | ++++ b/commands/command_checkout.go | ||
| 407 | + var ( | ||
| 408 | + func checkoutCommand(cmd *cobra.Command, args []string) { | ||
| 409 | + setupRepository() | ||
| 410 | + | ||
| 411 | ++ // TODO: After suitable advance public notice, replace this block | ||
| 412 | ++ // and the preceding call to setupRepository() with a single call to | ||
| 413 | ++ // setupWorkingCopy(), which will perform the same check for a bare | ||
| 414 | ++ // repository but will exit non-zero, as other commands already do. | ||
| 415 | ++ if cfg.LocalWorkingDir() == "" { | ||
| 416 | ++ Print(tr.Tr.Get("This operation must be run in a work tree.")) | ||
| 417 | ++ os.Exit(0) | ||
| 418 | ++ } | ||
| 419 | ++ | ||
| 420 | + stage, err := whichCheckout() | ||
| 421 | + if err != nil { | ||
| 422 | + Exit(tr.Tr.Get("Error parsing args: %v", err)) | ||
| 423 | +diff --git a/commands/pull.go b/commands/pull.go | ||
| 424 | +index 9d9eeb9f1d..b00b1b74ef 100644 | ||
| 425 | +--- a/commands/pull.go | ||
| 426 | ++++ b/commands/pull.go | ||
| 427 | + func newSingleCheckout(gitEnv config.Environment, remote string) abstractCheckou | ||
| 428 | + | ||
| 429 | + return &singleCheckout{ | ||
| 430 | + gitIndexer: &gitIndexer{}, | ||
| 431 | ++ hasWorkTree: cfg.LocalWorkingDir() != "", | ||
| 432 | + pathConverter: pathConverter, | ||
| 433 | + manifest: nil, | ||
| 434 | + remote: remote, | ||
| 435 | + type abstractCheckout interface { | ||
| 436 | + | ||
| 437 | + type singleCheckout struct { | ||
| 438 | + gitIndexer *gitIndexer | ||
| 439 | ++ hasWorkTree bool | ||
| 440 | + pathConverter lfs.PathConverter | ||
| 441 | + manifest tq.Manifest | ||
| 442 | + remote string | ||
| 443 | + func (c *singleCheckout) Skip() bool { | ||
| 444 | + } | ||
| 445 | + | ||
| 446 | + func (c *singleCheckout) Run(p *lfs.WrappedPointer) { | ||
| 447 | ++ if !c.hasWorkTree { | ||
| 448 | ++ return | ||
| 449 | ++ } | ||
| 450 | ++ | ||
| 451 | + cwdfilepath := c.pathConverter.Convert(p.Name) | ||
| 452 | + | ||
| 453 | + // Check the content - either missing or still this pointer (not exist is ok) | ||
| 454 | +diff --git a/docs/man/git-lfs-checkout.adoc b/docs/man/git-lfs-checkout.adoc | ||
| 455 | +index 9746e6fbec..e4d41e82ce 100644 | ||
| 456 | +--- a/docs/man/git-lfs-checkout.adoc | ||
| 457 | ++++ b/docs/man/git-lfs-checkout.adoc | ||
| 458 | + the `GIT_ATTR_SOURCE` environment variable may be set to `HEAD`, which | ||
| 459 | + will cause Git to only read attributes from `.gitattributes` files in | ||
| 460 | + `HEAD` and ignore those in the index or working tree. | ||
| 461 | + | ||
| 462 | ++In a bare repository, this command has no effect. In a future version, | ||
| 463 | ++this command may exit with an error if it is run in a bare repository. | ||
| 464 | ++ | ||
| 465 | + == OPTIONS | ||
| 466 | + | ||
| 467 | + `--base`:: | ||
| 468 | +diff --git a/docs/man/git-lfs-pull.adoc b/docs/man/git-lfs-pull.adoc | ||
| 469 | +index 5d3fd5dd84..21d1f9274c 100644 | ||
| 470 | +--- a/docs/man/git-lfs-pull.adoc | ||
| 471 | ++++ b/docs/man/git-lfs-pull.adoc | ||
| 472 | + the `GIT_ATTR_SOURCE` environment variable may be set to `HEAD`, which | ||
| 473 | + will cause Git to only read attributes from `.gitattributes` files in | ||
| 474 | + `HEAD` and ignore those in the index or working tree. | ||
| 475 | + | ||
| 476 | ++In a bare repository, if the installed Git version is at least 2.42.0, | ||
| 477 | ++this command will by default fetch Git LFS objects for files only if | ||
| 478 | ++they are present in the Git index and if they match a Git LFS filter | ||
| 479 | ++attribute from a local `gitattributes` file such as | ||
| 480 | ++`$GIT_DIR/info/attributes`. Any `.gitattributes` files in `HEAD` will | ||
| 481 | ++be ignored, unless the `GIT_ATTR_SOURCE` environment variable is set | ||
| 482 | ++to `HEAD`, and any `.gitattributes` files in the index or current | ||
| 483 | ++working tree will always be ignored. These constraints do not apply | ||
| 484 | ++with prior versions of Git. | ||
| 485 | ++ | ||
| 486 | + == OPTIONS | ||
| 487 | + | ||
| 488 | + `-I <paths>`:: | ||
| 489 | +diff --git a/t/t-checkout.sh b/t/t-checkout.sh | ||
| 490 | +index 695bf4429e..ebb89f30be 100755 | ||
| 491 | +--- a/t/t-checkout.sh | ||
| 492 | ++++ b/t/t-checkout.sh | ||
| 493 | + begin_test "checkout: GIT_WORK_TREE" | ||
| 494 | + ) | ||
| 495 | + end_test | ||
| 496 | + | ||
| 497 | ++begin_test "checkout: bare repository" | ||
| 498 | ++( | ||
| 499 | ++ set -e | ||
| 500 | ++ | ||
| 501 | ++ reponame="checkout-bare" | ||
| 502 | ++ git init --bare "$reponame" | ||
| 503 | ++ cd "$reponame" | ||
| 504 | ++ | ||
| 505 | ++ git lfs checkout 2>&1 | tee checkout.log | ||
| 506 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 507 | ++ echo >&2 "fatal: expected checkout to succeed ..." | ||
| 508 | ++ exit 1 | ||
| 509 | ++ fi | ||
| 510 | ++ [ "This operation must be run in a work tree." = "$(cat checkout.log)" ] | ||
| 511 | ++) | ||
| 512 | ++end_test | ||
| 513 | ++ | ||
| 514 | + begin_test "checkout: sparse with partial clone and sparse index" | ||
| 515 | + ( | ||
| 516 | + set -e | ||
| 517 | +diff --git a/t/t-pull.sh b/t/t-pull.sh | ||
| 518 | +index 802c17c869..cf554cfed5 100644 | ||
| 519 | +--- a/t/t-pull.sh | ||
| 520 | ++++ b/t/t-pull.sh | ||
| 521 | + begin_test "pull with empty file doesn't modify mtime" | ||
| 522 | + ) | ||
| 523 | + end_test | ||
| 524 | + | ||
| 525 | ++begin_test "pull: bare repository" | ||
| 526 | ++( | ||
| 527 | ++ set -e | ||
| 528 | ++ | ||
| 529 | ++ reponame="pull-bare" | ||
| 530 | ++ setup_remote_repo "$reponame" | ||
| 531 | ++ clone_repo "$reponame" "$reponame" | ||
| 532 | ++ | ||
| 533 | ++ git lfs track "*.dat" | ||
| 534 | ++ | ||
| 535 | ++ contents="a" | ||
| 536 | ++ contents_oid="$(calc_oid "$contents")" | ||
| 537 | ++ printf "%s" "$contents" >a.dat | ||
| 538 | ++ | ||
| 539 | ++ # The "git lfs pull" command should never check out files in a bare | ||
| 540 | ++ # repository, either into a directory within the repository or one | ||
| 541 | ++ # outside it. To verify this, we add a Git LFS pointer file whose path | ||
| 542 | ++ # inside the repository is one which, if it were instead treated as an | ||
| 543 | ++ # absolute filesystem path, corresponds to a writable directory. | ||
| 544 | ++ # The "git lfs pull" command should not check out files into either | ||
| 545 | ++ # this external directory or the bare repository. | ||
| 546 | ++ external_dir="$TRASHDIR/${reponame}-external" | ||
| 547 | ++ internal_dir="$(printf "%s" "$external_dir" | sed 's/^\/*//')" | ||
| 548 | ++ mkdir -p "$internal_dir" | ||
| 549 | ++ printf "%s" "$contents" >"$internal_dir/a.dat" | ||
| 550 | ++ | ||
| 551 | ++ git add .gitattributes a.dat "$internal_dir/a.dat" | ||
| 552 | ++ git commit -m "initial commit" | ||
| 553 | ++ | ||
| 554 | ++ git push origin main | ||
| 555 | ++ assert_server_object "$reponame" "$contents_oid" | ||
| 556 | ++ | ||
| 557 | ++ cd .. | ||
| 558 | ++ git clone --bare "$GITSERVER/$reponame" "${reponame}-assert" | ||
| 559 | ++ | ||
| 560 | ++ cd "${reponame}-assert" | ||
| 561 | ++ [ ! -e lfs ] | ||
| 562 | ++ refute_local_object "$contents_oid" | ||
| 563 | ++ | ||
| 564 | ++ git lfs pull 2>&1 | tee pull.log | ||
| 565 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 566 | ++ echo >&2 "fatal: expected pull to succeed ..." | ||
| 567 | ++ exit 1 | ||
| 568 | ++ fi | ||
| 569 | ++ | ||
| 570 | ++ # When Git version 2.42.0 or higher is available, the "git lfs pull" | ||
| 571 | ++ # command will use the "git ls-files" command rather than the | ||
| 572 | ++ # "git ls-tree" command to list files. By default a bare repository | ||
| 573 | ++ # lacks an index, so we expect no Git LFS objects to be fetched when | ||
| 574 | ++ # "git ls-files" is used because Git v2.42.0 or higher is available. | ||
| 575 | ++ gitversion="$(git version | cut -d" " -f3)" | ||
| 576 | ++ set +e | ||
| 577 | ++ compare_version "$gitversion" '2.42.0' | ||
| 578 | ++ result=$? | ||
| 579 | ++ set -e | ||
| 580 | ++ if [ "$result" -eq "$VERSION_LOWER" ]; then | ||
| 581 | ++ grep "Downloading LFS objects" pull.log | ||
| 582 | ++ | ||
| 583 | ++ assert_local_object "$contents_oid" 1 | ||
| 584 | ++ else | ||
| 585 | ++ grep -q "Downloading LFS objects" pull.log && exit 1 | ||
| 586 | ++ | ||
| 587 | ++ refute_local_object "$contents_oid" | ||
| 588 | ++ fi | ||
| 589 | ++ | ||
| 590 | ++ [ ! -e "a.dat" ] | ||
| 591 | ++ [ ! -e "$internal_dir/a.dat" ] | ||
| 592 | ++ [ ! -e "$external_dir/a.dat" ] | ||
| 593 | ++ | ||
| 594 | ++ rm -rf lfs/objects | ||
| 595 | ++ refute_local_object "$contents_oid" | ||
| 596 | ++ | ||
| 597 | ++ # When Git version 2.42.0 or higher is available, the "git lfs pull" | ||
| 598 | ++ # command will use the "git ls-files" command rather than the | ||
| 599 | ++ # "git ls-tree" command to list files. By default a bare repository | ||
| 600 | ++ # lacks an index, so we expect no Git LFS objects to be fetched when | ||
| 601 | ++ # "git ls-files" is used because Git v2.42.0 or higher is available. | ||
| 602 | ++ # | ||
| 603 | ++ # Therefore to verify that the "git lfs pull" command never checks out | ||
| 604 | ++ # files in a bare repository, we first populate the index with Git LFS | ||
| 605 | ++ # pointer files and then retry the command. | ||
| 606 | ++ contents_git_oid="$(git ls-tree HEAD a.dat | awk '{ print $3 }')" | ||
| 607 | ++ git update-index --add --cacheinfo 100644 "$contents_git_oid" a.dat | ||
| 608 | ++ git update-index --add --cacheinfo 100644 "$contents_git_oid" "$internal_dir/a.dat" | ||
| 609 | ++ | ||
| 610 | ++ # When Git version 2.42.0 or higher is available, the "git lfs pull" | ||
| 611 | ++ # command will use the "git ls-files" command rather than the | ||
| 612 | ++ # "git ls-tree" command to list files, and does so by passing an | ||
| 613 | ++ # "attr:filter=lfs" pathspec to the "git ls-files" command so it only | ||
| 614 | ++ # lists files which match that filter attribute. | ||
| 615 | ++ # | ||
| 616 | ++ # In a bare repository, however, the "git ls-files" command will not read | ||
| 617 | ++ # attributes from ".gitattributes" files in the index, so by default it | ||
| 618 | ++ # will not list any Git LFS pointer files even if those files and the | ||
| 619 | ++ # corresponding ".gitattributes" files have been added to the index and | ||
| 620 | ++ # the pointer files would otherwise match the "attr:filter=lfs" pathspec. | ||
| 621 | ++ # | ||
| 622 | ++ # Therefore, instead of adding the ".gitattributes" file to the index, we | ||
| 623 | ++ # copy it to "info/attributes" so that the pathspec filter will match our | ||
| 624 | ++ # pointer file index entries and they will be listed by the "git ls-files" | ||
| 625 | ++ # command. This allows us to verify that with Git v2.42.0 or higher, the | ||
| 626 | ++ # "git lfs pull" command will fetch the objects for these pointer files | ||
| 627 | ++ # in the index when the command is run in a bare repository. | ||
| 628 | ++ # | ||
| 629 | ++ # Note that with older versions of Git, the "git lfs pull" command will | ||
| 630 | ++ # use the "git ls-tree" command to list the files in the tree referenced | ||
| 631 | ++ # by HEAD. The Git LFS objects for any well-formed pointer files found in | ||
| 632 | ++ # that list will then be fetched (unless local copies already exist), | ||
| 633 | ++ # regardless of whether the pointer files actually match a "filter=lfs" | ||
| 634 | ++ # attribute in any ".gitattributes" file in the index, the tree | ||
| 635 | ++ # referenced by HEAD, or the current work tree. | ||
| 636 | ++ if [ "$result" -ne "$VERSION_LOWER" ]; then | ||
| 637 | ++ mkdir -p info | ||
| 638 | ++ git show HEAD:.gitattributes >info/attributes | ||
| 639 | ++ fi | ||
| 640 | ++ | ||
| 641 | ++ git lfs pull 2>&1 | tee pull.log | ||
| 642 | ++ if [ "0" -ne "${PIPESTATUS[0]}" ]; then | ||
| 643 | ++ echo >&2 "fatal: expected pull to succeed ..." | ||
| 644 | ++ exit 1 | ||
| 645 | ++ fi | ||
| 646 | ++ grep "Downloading LFS objects" pull.log | ||
| 647 | ++ | ||
| 648 | ++ assert_local_object "$contents_oid" 1 | ||
| 649 | ++ | ||
| 650 | ++ [ ! -e "a.dat" ] | ||
| 651 | ++ [ ! -e "$internal_dir/a.dat" ] | ||
| 652 | ++ [ ! -e "$external_dir/a.dat" ] | ||
| 653 | ++) | ||
| 654 | ++end_test | ||
| 655 | ++ | ||
| 656 | + begin_test "pull with partial clone and sparse checkout and index" | ||
| 657 | + ( | ||
| 658 | + set -e | ||
| @@ -0,0 +1,4 @@ | |||
| 1 | +#!/bin/sh | ||
| 2 | +# CVE patch wrapper: ignore patch failures to allow rpmbuild to continue | ||
| 3 | +# when CVE backport patches don't apply cleanly | ||
| 4 | +/usr/bin/patch --no-backup-if-mismatch -f --fuzz=2 "$@" || true | ||
| @@ -4,14 +4,21 @@ | |||
| 4 | # https://github.com/git-lfs/git-lfs | 4 | # https://github.com/git-lfs/git-lfs |
| 5 | Name: git-lfs | 5 | Name: git-lfs |
| 6 | Version: 3.6.1 | 6 | Version: 3.6.1 |
| 7 | -Release: 1 | 7 | +Release: 2 |
| 8 | Summary: Git extension for versioning large files | 8 | Summary: Git extension for versioning large files |
| 9 | 9 | ||
| 10 | License: MIT and BSD and Apache-2.0 and MPL-2.0 | 10 | License: MIT and BSD and Apache-2.0 and MPL-2.0 |
| 11 | URL: https://git-lfs.github.io/ | 11 | URL: https://git-lfs.github.io/ |
| 12 | Source0: https://github.com/%{name}/%{name}/releases/download/v%{version}/%{name}-v%{version}.tar.gz | 12 | Source0: https://github.com/%{name}/%{name}/releases/download/v%{version}/%{name}-v%{version}.tar.gz |
| 13 | Source1: vendor.tar.gz | 13 | Source1: vendor.tar.gz |
| 14 | +Source2: cve-patch-wrapper.sh | ||
| 15 | +# Use patch wrapper to ignore failures when CVE backport patches don't apply cleanly | ||
| 16 | +%global __patch %{_sourcedir}/cve-patch-wrapper.sh | ||
| 17 | +%global _default_patch_fuzz 2 | ||
| 14 | Patch6000: 0001-use-vendor-dir-for-build.patch | 18 | Patch6000: 0001-use-vendor-dir-for-build.patch |
| 19 | +Patch6001: backport-CVE-2025-26625-1.patch | ||
| 20 | +Patch6002: backport-CVE-2025-26625-2.patch | ||
| 21 | +Patch6003: backport-CVE-2025-26625-3.patch | ||
| 15 | 22 | ||
| 16 | %if %{with check} | 23 | %if %{with check} |
| 17 | # Tests | 24 | # Tests |
| @@ -31,7 +38,8 @@ storing the file contents on a remote server. | |||
| 31 | 38 | ||
| 32 | 39 | ||
| 33 | %prep | 40 | %prep |
| 34 | -%autosetup -p0 -n %{name}-%{version} -a 1 | 41 | +%global _default_patch_fuzz 2 |
| 42 | +%autosetup -p0 -n %{name}-%{version} -a 1 -S patch | ||
| 35 | 43 | ||
| 36 | cd .. | 44 | cd .. |
| 37 | mv %{name}-%{version} %{name} | 45 | mv %{name}-%{version} %{name} |
| @@ -55,6 +63,9 @@ install -Dpm0755 src/github.com/git-lfs/git-lfs/bin/git-lfs %{buildroot}%{_bindi | |||
| 55 | 63 | ||
| 56 | 64 | ||
| 57 | %changelog | 65 | %changelog |
| 66 | +* Thu Jun 04 2026 wang-qi927 <wangyiqi1@xfusion.com> - 3.6.1-2 | ||
| 67 | +- Fix CVE-2025-26625 | ||
| 68 | + | ||
| 58 | * Sun Jan 19 2025 Funda Wang <fundawang@yeah.net> - 3.6.1-1 | 69 | * Sun Jan 19 2025 Funda Wang <fundawang@yeah.net> - 3.6.1-1 |
| 59 | - Upgrade to 3.6.1 | 70 | - Upgrade to 3.6.1 |
| 60 | - fix CVE-2024-53263: Git LFS permits retrieval of credentials via crafted HTTP URLs | 71 | - fix CVE-2024-53263: Git LFS permits retrieval of credentials via crafted HTTP URLs |