summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2023-01-27Merge branch 'cw/fetch-remote-group-with-duplication'Junio C Hamano2-0/+10
"git fetch <group>", when "<group>" of remotes lists the same remote twice, unnecessarily failed when parallel fetching was enabled, which has been corrected. * cw/fetch-remote-group-with-duplication: fetch: fix duplicate remote parallel fetch bug
2023-01-27Merge branch 'jc/doc-branch-update-checked-out-branch'Junio C Hamano1-0/+4
Document that "branch -f <branch>" disables only the safety to avoid recreating an existing branch. * jc/doc-branch-update-checked-out-branch: branch: document `-f` and linked worktree behaviour
2023-01-27Merge branch 'jk/hash-object-literally-fd-leak'Junio C Hamano1-0/+1
Leakfix. * jk/hash-object-literally-fd-leak: hash-object: fix descriptor leak with --literally
2023-01-27Merge branch 'pb/branch-advice-recurse-submodules'Junio C Hamano1-1/+1
Improve advice message given when "git branch --recurse-submodules" fails. * pb/branch-advice-recurse-submodules: branch: improve advice when --recurse-submodules fails
2023-01-27Merge branch 'km/send-email-with-v-reroll-count'Junio C Hamano2-1/+14
"git send-email -v 3" used to be expanded to "git send-email --validate 3" when the user meant to pass them down to "format-patch", which has been corrected. * km/send-email-with-v-reroll-count: send-email: relay '-v N' to format-patch
2023-01-27Merge branch 'cb/grep-pcre-ucp'Junio C Hamano2-1/+43
"grep -P" learned to use Unicode Character Property to grok character classes when processing \b and \w etc. * cb/grep-pcre-ucp: grep: correctly identify utf-8 characters with \{b,w} in -P
2023-01-27Merge branch 'sa/cat-file-mailmap--batch-check'Junio C Hamano1-7/+12
Docfix. * sa/cat-file-mailmap--batch-check: git-cat-file.txt: fix list continuations rendering literally
2023-01-27Merge branch 'ab/cache-api-cleanup-users'Junio C Hamano15-58/+30
Updates the users of the cache API. * ab/cache-api-cleanup-users: treewide: always have a valid "index_state.repo" member
2023-01-26attr: fix instructions on how to check attrsJohn Cai1-1/+1
The instructions in attr.h describing what functions to call to check attributes is missing the index as the first argument to git_check_attr(), as well as tree_oid as the second argument. When 7a400a2c (attr: remove an implicit dependency on the_index, 2018-08-13) started passing an index_state instance to git_check_attr(), it forgot to update the API documentation in Documentation/technical/api-gitattributes.txt. Later, 3a1b3415 (attr: move doc to attr.h, 2019-11-17) moved the API documentation to attr.h as a comment, but still left out the index_state as an argument. In 47cfc9b (attr: add flag `--source` to work with tree-ish 2023-01-14) added tree_oid as an optional parameter but was not added to the docs in attr.h Fix this to make the documentation in the comment consistent with the actual function signature. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25request-pull: filter out SSH/X.509 tag signaturesGwyneth Morgan1-1/+1
git request-pull filters PGP signatures out of the tag message, but not SSH or X.509 signatures. Signed-off-by: Gwyneth Morgan <gwymor@tilde.club> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: provide better error message for apply options vs. merge configElijah Newren3-7/+46
When config which selects the merge backend (currently, rebase.autosquash=true or rebase.updateRefs=true) conflicts with other options on the command line (such as --whitespace=fix), make the error message specifically call out the config option and specify how to override that config option on the command line. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: put rebase_options initialization in single placeElijah Newren1-2/+2
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: fix formatting of rebase --reapply-cherry-picks option in docsElijah Newren1-2/+0
Commit ce5238a690 ("rebase --keep-base: imply --reapply-cherry-picks", 2022-10-17) accidentally added some blank lines that cause extra paragraphs about --reapply-cherry-picks to be considered not part of the documentation of that option. Remove the blank lines to make it clear we are still discussing --reapply-cherry-picks. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: clarify the OPT_CMDMODE incompatibilitiesElijah Newren1-28/+33
--edit-todo was documented as being incompatible with any of the options for the apply backend. However, it is also incompatible with any of the options for the merge backend, and is incompatible with any options that are not backend specific as well. The same can be said for --continue, --skip, --abort, --quit, etc. This is already somewhat implicitly covered by the synopsis, but since "[<options>]" in the first two variants are vague it might be easy to miss this. That might not be a big deal, but since the rebase manpage has to spend so much verbiage about incompatibility of options, making a separate section for these options that are incompatible with everything else seems clearer. Do that, and remove the needless inclusion of --edit-todo in the explicit incompatibility list. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: add coverage of other incompatible optionsElijah Newren2-0/+18
The git-rebase manual noted several sets of incompatible options, but we were missing tests for a few of these. Further, we were missing code checks for one of these, which could result in command line options being silently ignored. Also, note that adding a check for autosquash means that using --whitespace=fix together with the config setting rebase.autosquash=true will trigger an error. A subsequent commit will improve the error message. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: fix incompatiblity checks for --[no-]reapply-cherry-picksElijah Newren3-14/+32
--[no-]reapply-cherry-picks was traditionally only supported by the sequencer. Support was added for the apply backend, when --keep-base is also specified, in commit ce5238a690 ("rebase --keep-base: imply --reapply-cherry-picks", 2022-10-17). Make the code error out when --[no-]reapply-cherry-picks is specified AND the apply backend is used AND --keep-base is not specified. Also, clarify a number of comments surrounding the interaction of these flags. Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: fix docs about incompatibilities with --rootElijah Newren2-5/+6
In commit 5dacd4abdd ("git-rebase.txt: document incompatible options", 2018-06-25), I added notes about incompatibilities between options for the apply and merge backends. Unfortunately, I inverted the condition when --root was incompatible with the apply backend. Fix the documentation, and add a testcase that verifies the documentation matches the code. While at it, the documentation for --root also tried to cover some of the backend differences between the apply and merge backends in relation to reapplying cherry picks. The information: * assumed that the apply backend was the default (it isn't anymore) * was written before --reapply-cherry-picks became an option * was written before the detailed information on backend differences All of these factors make the sentence under --root about reapplying cherry picks contradict information that is now available elsewhere in the manual, and the other references are correct. So just strike this sentence. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: remove --allow-empty-message from incompatible optsElijah Newren1-1/+0
--allow-empty-message was turned into a no-op and even documented as such; the flag is simply ignored. Since the flag is ignored, it shouldn't be documented as being incompatible with other flags. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: flag --apply and --merge as incompatibleElijah Newren2-2/+13
Previously, we flagged options which implied --apply as being incompatible with options which implied --merge. But if both options were given explicitly, then we didn't flag the incompatibility. The same is true with --apply and --interactive. Add the check, and add some testcases to verify these are also caught. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25rebase: mark --update-refs as requiring the merge backendElijah Newren3-5/+15
--update-refs is built in terms of the sequencer, which requires the merge backend. It was already marked as incompatible with the apply backend in the git-rebase manual, but the code didn't check for this incompatibility and warn the user. Check and error now. While at it, fix a typo in t3422...and fix some misleading wording (most options which used to be am-specific have since been implemented in the merge backend as well). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25ssh signing: better error message when key not in agentAdam Szkoda1-1/+5
When signing a commit with a SSH key, with the private key missing from ssh-agent, a confusing error message is produced: error: Load key "/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7": invalid format? fatal: failed to write commit object The temporary file .git_signing_key_tmpkArSj7 created by git contains a valid *public* key. The error message comes from `ssh-keygen -Y sign' and is caused by a fallback mechanism in ssh-keygen whereby it tries to interpret .git_signing_key_tmpkArSj7 as a *private* key if it can't find in the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to 'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent. As a result, when the private key is missing from the agent, a more accurate error message gets produced: error: Couldn't find key in agent [1] https://bugzilla.mindrot.org/show_bug.cgi?id=3429 Signed-off-by: Adam Szkoda <adaszko@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-25Merge branch 'ab/makeflags'Pratyush Yadav1-1/+1
Backport a Makefile fix from upstream git. * ab/makeflags: Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4
2023-01-25Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4Ævar Arnfjörð Bjarmason1-1/+1
Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has changed in a backward-incompatible way, as its "NEWS" file notes: Previously only simple (one-letter) options were added to the MAKEFLAGS variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return the set of one-letter options which can be examined via findstring, etc. This means that $(MAKEFLAGS) now contains long options like "--jobserver-auth=fifo:<path>" and we have to adapt to that. Note that the "-" in "-$(MAKEFLAGS)" is critical here, as the variable will always contain leading whitespace if there are no short options, but long options are present. This is a partial backport of 67b36879fc0 (Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4, 2022-11-30), which had been applied directly to git/git. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-24dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKSTaylor Blau4-5/+56
When using the dir_iterator API, we first stat(2) the base path, and then use that as a starting point to enumerate the directory's contents. If the directory contains symbolic links, we will immediately die() upon encountering them without the `FOLLOW_SYMLINKS` flag. The same is not true when resolving the top-level directory, though. As explained in a previous commit, this oversight in 6f054f9fb3 (builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28) can be used as an attack vector to include arbitrary files on a victim's filesystem from outside of the repository. Prevent resolving top-level symlinks unless the FOLLOW_SYMLINKS flag is given, which will cause clones of a repository with a symlink'd "$GIT_DIR/objects" directory to fail. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-24clone: delay picking a transport until after get_repo_path()Taylor Blau2-7/+14
In the previous commit, t5619 demonstrates an issue where two calls to `get_repo_path()` could trick Git into using its local clone mechanism in conjunction with a non-local transport. That sequence is: - the starting state is that the local path https:/example.com/foo is a symlink that points to ../../../.git/modules/foo. So it's dangling. - get_repo_path() sees that no such path exists (because it's dangling), and thus we do not canonicalize it into an absolute path - because we're using --separate-git-dir, we create .git/modules/foo. Now our symlink is no longer dangling! - we pass the url to transport_get(), which sees it as an https URL. - we call get_repo_path() again, on the url. This second call was introduced by f38aa83f9a (use local cloning if insteadOf makes a local URL, 2014-07-17). The idea is that we want to pull the url fresh from the remote.c API, because it will apply any aliases. And of course now it sees that there is a local file, which is a mismatch with the transport we already selected. The issue in the above sequence is calling `transport_get()` before deciding whether or not the repository is indeed local, and not passing in an absolute path if it is local. This is reminiscent of a similar bug report in [1], where it was suggested to perform the `insteadOf` lookup earlier. Taking that approach may not be as straightforward, since the intent is to store the original URL in the config, but to actually fetch from the insteadOf one, so conflating the two early on is a non-starter. Note: we pass the path returned by `get_repo_path(remote->url[0])`, which should be the same as `repo_name` (aside from any `insteadOf` rewrites). We *could* pass `absolute_pathdup()` of the same argument, which 86521acaca (Bring local clone's origin URL in line with that of a remote clone, 2008-09-01) indicates may differ depending on the presence of ".git/" for a non-bare repo. That matters for forming relative submodule paths, but doesn't matter for the second call, since we're just feeding it to the transport code, which is fine either way. [1]: https://lore.kernel.org/git/CAMoD=Bi41mB3QRn3JdZL-FGHs4w3C2jGpnJB-CqSndO7FMtfzA@mail.gmail.com/ Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-24t5619: demonstrate clone_local() with ambiguous transportTaylor Blau1-0/+63
When cloning a repository, Git must determine (a) what transport mechanism to use, and (b) whether or not the clone is local. Since f38aa83f9a (use local cloning if insteadOf makes a local URL, 2014-07-17), the latter check happens after the remote has been initialized, and references the remote's URL instead of the local path. This is done to make it possible for a `url.<base>.insteadOf` rule to convert a remote URL into a local one, in which case the `clone_local()` mechanism should be used. However, with a specially crafted repository, Git can be tricked into using a non-local transport while still setting `is_local` to "1" and using the `clone_local()` optimization. The below test case demonstrates such an instance, and shows that it can be used to include arbitrary (known) paths in the working copy of a cloned repository on a victim's machine[^1], even if local file clones are forbidden by `protocol.file.allow`. This happens in a few parts: 1. We first call `get_repo_path()` to see if the remote is a local path. If it is, we replace the repo name with its absolute path. 2. We then call `transport_get()` on the repo name and decide how to access it. If it was turned into an absolute path in the previous step, then we should always treat it like a file. 3. We use `get_repo_path()` again, and set `is_local` as appropriate. But it's already too late to rewrite the repo name as an absolute path, since we've already fed it to the transport code. The attack works by including a submodule whose URL corresponds to a path on disk. In the below example, the repository "sub" is reachable via the dumb HTTP protocol at (something like): http://127.0.0.1:NNNN/dumb/sub.git However, the path "http:/127.0.0.1:NNNN/dumb" (that is, a top-level directory called "http:", then nested directories "127.0.0.1:NNNN", and "dumb") exists within the repository, too. To determine this, it first picks the appropriate transport, which is dumb HTTP. It then uses the remote's URL in order to determine whether the repository exists locally on disk. However, the malicious repository also contains an embedded stub repository which is the target of a symbolic link at the local path corresponding to the "sub" repository on disk (i.e., there is a symbolic link at "http:/127.0.0.1/dumb/sub.git", pointing to the stub repository via ".git/modules/sub/../../../repo"). This stub repository fools Git into thinking that a local repository exists at that URL and thus can be cloned locally. The affected call is in `get_repo_path()`, which in turn calls `get_repo_path_1()`, which locates a valid repository at that target. This then causes Git to set the `is_local` variable to "1", and in turn instructs Git to clone the repository using its local clone optimization via the `clone_local()` function. The exploit comes into play because the stub repository's top-level "$GIT_DIR/objects" directory is a symbolic link which can point to an arbitrary path on the victim's machine. `clone_local()` resolves the top-level "objects" directory through a `stat(2)` call, meaning that we read through the symbolic link and copy or hardlink the directory contents at the destination of the link. In other words, we can get steps (1) and (3) to disagree by leveraging the dangling symlink to pick a non-local transport in the first step, and then set is_local to "1" in the third step when cloning with `--separate-git-dir`, which makes the symlink non-dangling. This can result in data-exfiltration on the victim's machine when sensitive data is at a known path (e.g., "/home/$USER/.ssh"). The appropriate fix is two-fold: - Resolve the transport later on (to avoid using the local clone optimization with a non-local transport). - Avoid reading through the top-level "objects" directory when (correctly) using the clone_local() optimization. This patch merely demonstrates the issue. The following two patches will implement each part of the above fix, respectively. [^1]: Provided that any target directory does not contain symbolic links, in which case the changes from 6f054f9fb3 (builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28) will abort the clone. Reported-by: yvvdwf <yvvdwf@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-24Merge branch 'js/windows-rce'Pratyush Yadav1-74/+133
Fix a Remote Code Execution vulnerability on Windows. This is caused by the fact that Tcl on Windows always includes the current directory when looking for an executable. Therefore malicious repositories can ship with an aspell.exe in their top-level directory which is executed by Git GUI without giving the user a chance to inspect it first, i.e. running untrusted code. This merge fixes CVE-2022-41953. * js/windows-rce: Work around Tcl's default `PATH` lookup Move the `_which` function (almost) to the top Move is_<platform> functions to the beginning is_Cygwin: avoid `exec`ing anything windows: ignore empty `PATH` elements
2023-01-24Work around Tcl's default `PATH` lookupJohannes Schindelin1-0/+56
As per https://www.tcl.tk/man/tcl8.6/TclCmd/exec.html#M23, Tcl's `exec` function goes out of its way to imitate the highly dangerous path lookup of `cmd.exe`, but _of course_ only on Windows: If a directory name was not specified as part of the application name, the following directories are automatically searched in order when attempting to locate the application: The directory from which the Tcl executable was loaded. The current directory. The Windows 32-bit system directory. The Windows home directory. The directories listed in the path. The dangerous part is the second item, of course: `exec` _prefers_ executables in the current directory to those that are actually in the `PATH`. It is almost as if people wanted to Windows users vulnerable, specifically. To avoid that, Git GUI already has the `_which` function that does not imitate that dangerous practice when looking up executables in the search path. However, Git GUI currently fails to use that function e.g. when trying to execute `aspell` for spell checking. That is not only dangerous but combined with Tcl's unfortunate default behavior and with the fact that Git GUI tries to spell-check a repository just after cloning, leads to a critical Remote Code Execution vulnerability. Let's override both `exec` and `open` to always use `_which` instead of letting Tcl perform the path lookup, to prevent this attack vector. This addresses CVE-2022-41953. For more details, see https://github.com/git-for-windows/git/security/advisories/GHSA-v4px-mx59-w99c Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-24Move the `_which` function (almost) to the topJohannes Schindelin1-42/+46
We are about to make use of the `_which` function to address CVE-2022-41953 by overriding Tcl/Tk's unsafe PATH lookup on Windows. In preparation for that, let's move it close to the top of the file to make sure that even early `exec` calls that happen during the start-up of Git GUI benefit from the fix. This commit is best viewed with `--color-moved`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-24Move is_<platform> functions to the beginningJohannes Schindelin1-27/+31
We need these in `_which` and they should be defined before that function's definition. This commit is best viewed with `--color-moved`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-24is_Cygwin: avoid `exec`ing anythingJohannes Schindelin1-10/+2
The `is_Cygwin` function is used, among other things, to determine how executables are discovered in the `PATH` list by the `_which` function. We are about to change the behavior of the `_which` function on Windows (but not Cygwin): On Windows, we want it to ignore empty elements of the `PATH` instead of treating them as referring to the current directory (which is a "legacy feature" according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03, but apparently not explicitly deprecated, the POSIX documentation is quite unclear on that even if the Cygwin project itself considers it to be deprecated: https://github.com/cygwin/cygwin/commit/fc74dbf22f5c). This is important because on Windows, `exec` does something very unsafe by default (unless we're running a Cygwin version of Tcl, which follows Unix semantics). However, we try to `exec` something _inside_ `is_Cygwin` to determine whether we're running within Cygwin or not, i.e. before we determined whether we need to handle `PATH` specially or not. That's a Catch-22. Therefore, and because it is much cleaner anyway, use the `$::tcl_platform(os)` value which is guaranteed to start with `CYGWIN_` when running a Cygwin variant of Tcl/Tk, instead of executing `cygpath --windir`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-24windows: ignore empty `PATH` elementsJohannes Schindelin1-0/+3
When looking up an executable via the `_which` function, Git GUI imitates the `execlp()` strategy where the environment variable `PATH` is interpreted as a list of paths in which to search. For historical reasons, stemming from the olden times when it was uncommon to download a lot of files from the internet into the current directory, empty elements in this list are treated as if the current directory had been specified. Nowadays, of course, this treatment is highly dangerous as the current directory often contains files that have just been downloaded and not yet been inspected by the user. Unix/Linux users are essentially expected to be very, very careful to simply not add empty `PATH` elements, i.e. not to make use of that feature. On Windows, however, it is quite common for `PATH` to contain empty elements by mistake, e.g. as an unintended left-over entry when an application was installed from the Windows Store and then uninstalled manually. While it would probably make most sense to safe-guard not only Windows users, it seems to be common practice to ignore these empty `PATH` elements _only_ on Windows, but not on other platforms. Sadly, this practice is followed inconsistently between different software projects, where projects with few, if any, Windows-based contributors tend to be less consistent or even "blissful" about it. Here is a non-exhaustive list: Cygwin: It specifically "eats" empty paths when converting path lists to POSIX: https://github.com/cygwin/cygwin/commit/753702223c7d I.e. it follows the common practice. PowerShell: It specifically ignores empty paths when searching the `PATH`. The reason for this is apparently so self-evident that it is not even mentioned here: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables#path-information I.e. it follows the common practice. CMD: Oh my, CMD. Let's just forget about it, nobody in their right (security) mind takes CMD as inspiration. It is so unsafe by default that we even planned on dropping `Git CMD` from Git for Windows altogether, and only walked back on that plan when we found a super ugly hack, just to keep Git's users secure by default: https://github.com/git-for-windows/MINGW-packages/commit/82172388bb51 So CMD chooses to hide behind the battle cry "Works as Designed!" that all too often leaves users vulnerable. CMD is probably the most prominent project whose lead you want to avoid following in matters of security. Win32 API (`CreateProcess()`) Just like CMD, `CreateProcess()` adheres to the original design of the path lookup in the name of backward compatibility (see https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw for details): If the file name does not contain a directory path, the system searches for the executable file in the following sequence: 1. The directory from which the application loaded. 2. The current directory for the parent process. [...] I.e. the Win32 API itself chooses backwards compatibility over users' safety. Git LFS: There have been not one, not two, but three security advisories about Git LFS executing executables from the current directory by mistake. As part of one of them, a change was introduced to stop treating empty `PATH` elements as equivalent to `.`: https://github.com/git-lfs/git-lfs/commit/7cd7bb0a1f0d I.e. it follows the common practice. Go: Go does not follow the common practice, and you can think about that what you want: https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go#L114-L135 https://github.com/golang/go/blob/go1.19.3/src/path/filepath/path_windows.go#L108-L137 Git Credential Manager: It tries to imitate Git LFS, but unfortunately misses the empty `PATH` element handling. As of time of writing, this is in the process of being fixed: https://github.com/GitCredentialManager/git-credential-manager/pull/968 So now that we have established that it is a common practice to ignore empty `PATH` elements on Windows, let's assess this commit's change using Schneier's Five-Step Process (https://www.schneier.com/crypto-gram/archives/2002/0415.html#1): Step 1: What problem does it solve? It prevents an entire class of Remote Code Execution exploits via Git GUI's `Clone` functionality. Step 2: How well does it solve that problem? Very well. It prevents the attack vector of luring an unsuspecting victim into cloning an executable into the worktree root directory that Git GUI immediately executes. Step 3: What other security problems does it cause? Maybe non-security problems: If a project (ab-)uses the unsafe `PATH` lookup. That would not only be unsafe, though, but fragile in the first place because it would break when running in a subdirectory. Therefore I would consider this a scenario not worth keeping working. Step 4: What are the costs of this measure? Almost nil, except for the time writing up this commit message ;-) Step 5: Given the answers to steps two through four, is the security measure worth the costs? Yes. Keeping Git's users Secure By Default is worth it. It's a tiny price to pay compared to the damages even a single successful exploit can cost. So let's follow that common practice in Git GUI, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2023-01-23The tenth batchJunio C Hamano1-0/+24
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-23Merge branch 'zh/scalar-progress'Junio C Hamano2-3/+33
"scalar" learned to give progress bar. * zh/scalar-progress: scalar: show progress if stderr refers to a terminal
2023-01-23Merge branch 'ds/omit-trailing-hash-in-index'Junio C Hamano1-0/+1
Quickfix for a topic already in 'master'. * ds/omit-trailing-hash-in-index: t1600: fix racy index.skipHash test
2023-01-23Merge branch 'jc/format-patch-v-unleak'Junio C Hamano1-3/+3
Plug a small leak. * jc/format-patch-v-unleak: format-patch: unleak "-v <num>"
2023-01-23Merge branch 'sk/win32-close-handle-upon-pthread-join'Junio C Hamano1-11/+14
Pthread emulation on Win32 leaked thread handle when a thread is joined. * sk/win32-close-handle-upon-pthread-join: win32: close handles of threads that have been joined win32: prepare pthread.c for change by formatting
2023-01-23Merge branch 'rs/use-enhanced-bre-on-macos'Junio C Hamano4-0/+24
Newer regex library macOS stopped enabling GNU-like enhanced BRE, where '\(A\|B\)' works as alternation, unless explicitly asked with the REG_ENHANCED flag. "git grep" now can be compiled to do so, to retain the old behaviour. * rs/use-enhanced-bre-on-macos: use enhanced basic regular expressions on macOS
2023-01-23Merge branch 'ab/test-env-helper'Junio C Hamano9-50/+50
Remove "git env--helper" and demote it to a test-tool subcommand. * ab/test-env-helper: env-helper: move this built-in to "test-tool env-helper"
2023-01-23Merge branch 'kn/attr-from-tree'Junio C Hamano12-55/+156
"git check-attr" learned to take an optional tree-ish to read the .gitattributes file from. * kn/attr-from-tree: attr: add flag `--source` to work with tree-ish t0003: move setup for `--all` into new block
2023-01-23Merge branch 'rs/ls-tree-path-expansion-fix'Junio C Hamano2-7/+10
"git ls-tree --format='%(path) %(path)' $tree $path" showed the path three times, which has been corrected. * rs/ls-tree-path-expansion-fix: ls-tree: remove dead store and strbuf for quote_c_style() ls-tree: fix expansion of repeated %(path)
2023-01-23Merge branch 'en/t6426-todo-cleanup'Junio C Hamano1-32/+20
Test clean-up. * en/t6426-todo-cleanup: t6426: fix TODO about making test more comprehensive
2023-01-23Merge branch 'ab/cache-api-cleanup'Junio C Hamano13-41/+64
Code clean-up to tighten the use of in-core index in the API. * ab/cache-api-cleanup: cache API: add a "INDEX_STATE_INIT" macro/function, add release_index() read-cache.c: refactor set_new_index_sparsity() for subsequent commit sparse-index API: BUG() out on NULL ensure_full_index() sparse-index.c: expand_to_path() can assume non-NULL "istate" builtin/difftool.c: { 0 }-initialize rather than using memset()
2023-01-23Documentation: render dash correctlyAndrei Rybak6-6/+6
Three hyphens are rendered verbatim in documentation, so "--" has to be used to produce a dash. Fix asciidoc output for dashes. This is similar to previous commits f0b922473e (Documentation: render special characters correctly, 2021-07-29) and de82095a95 (doc hash-function-transition: fix asciidoc output, 2021-02-05). Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-22bisect: fix "reset" when branch is checked out elsewhereRubén Justo2-1/+25
Since 1d0fa89 (checkout: add --ignore-other-wortrees, 2015-01-03) we have a safety valve in checkout/switch to prevent the same branch from being checked out simultaneously in multiple worktrees. If a branch is bisected in a worktree while also being checked out in another worktree; when the bisection is finished, checking out the branch back in the current worktree may fail. Let's teach bisect to use the "--ignore-other-worktrees" flag. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-22t0003: call dd with portable blocksizeTorsten Bögershausen1-2/+2
The command `dd bs=101M count=1` is not portable, e.g. dd shipped with MacOs does not understand the 'M'. Use `dd bs=1048576 count=101`, which achives the same, instead. Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-21The ninth batchJunio C Hamano1-0/+12
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-21Merge branch 'yc/doc-fetch-fix'Junio C Hamano1-4/+4
Doc fix. * yc/doc-fetch-fix: doc: fix non-existent config name
2023-01-21Merge branch 'ab/bisect-cleanup'Junio C Hamano3-36/+40
Code clean-up. * ab/bisect-cleanup: bisect: no longer try to clean up left-over `.git/head-name` files bisect: remove Cogito-related code bisect run: fix the error message bisect: verify that a bogus option won't try to start a bisection bisect--helper: make the order consistently `argc, argv` bisect--helper: simplify exit code computation
2023-01-21Merge branch 'tl/ls-tree-code-clean-up'Junio C Hamano2-108/+150
Code clean-up. * tl/ls-tree-code-clean-up: t3104: remove shift code in 'test_ls_tree_format' ls-tree: cleanup the redundant SPACE ls-tree: make "line_termination" less generic ls-tree: fold "show_tree_data" into "cb" struct ls-tree: use a "struct options" ls-tree: don't use "show_tree_data" for "fast" callbacks