Commit Graph

145696 Commits

Author SHA1 Message Date
Johannes Schindelin
88e383a82d azure-pipeline: use partial clone/parallel checkout to initialize minimal-sdk
The Azure Pipeline `git-sdk-64-minimal` was retired...

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
1a36960c65 azure-pipeline: drop the code to write to/read from a file share
We haven't used this feature in ages, we don't actually need to.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
46fca9814b azure-pipeline: stop hard-coding apt-get calls
We have `ci/install-dependencies.sh` for that. Incidentally, this avoids
the following error in the linux-* jobs:

    The following packages have unmet dependencies:
    git-email : Depends: git (< 1:2.25.1-.) but 1:2.35.1-0ppa1~ubuntu20.04.1 is to be installed
	  Recommends: libemail-valid-perl but it is not going to be installed

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
5e8a990c89 azure-pipeline: drop the GETTEXT_POISON job
This is a follow-up to 6c280b4142 (ci: remove GETTEXT_POISON jobs,
2021-01-20) after reinstating the Azure Pipeline.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
03be20c731 ci: reinstate Azure Pipelines support
... so that we can test a MinGit backport in a private repository (with
GitHub Actions, minutes and parallel jobs are limited way more than with
Azure Pipelines in private repositories).

In this commit, we reinstate the exact version of `azure-pipelines.yml`
as 6081d3898f (ci: retire the Azure Pipelines definition, 2020-04-11)
deleted.

Naturally, many adjustments are required to make it work again. Some of
the changes are actually outside of that file (such as the
`runs_on_pool` changes that are needed in the Azure Pipelines part of
`ci/lib.sh`) and they were made in the commits leading up to this here
commit.

However, other adjustments are required in the `azure-pipelines.yml`
file itself, and for ease of review (read: to build confidence in those
changes) they will be made in subsequent, individual commits that
explain the intent, context, implementation and justification like every
good commit message should do.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
9944a9df8e ci: stop linking the prove cache
It is not useful because we do not have any persisted directory anymore,
not since dropping our Travis CI support.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
ca4260698c ci: adjust Azure Pipeline for runs_on_pool
These refactorings are really gifts that keep on giving.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
4d6d3a7e6a vcxproj: avoid escaping double quotes in the defines
Visual Studio 2022 does not like that at all.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
afbc0f7053 vcxproj: handle libreftable_test, too
Since ef8a6c6268 (reftable: utility functions, 2021-10-07) we not only
have a libreftable, but also a libreftable_test.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
166470aae0 vcxproj: include reftable when committing .vcxproj files
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
77caf99a3e vcxproj: ignore the -pedantic option
This is now passed by default, ever since 6a8cbc41ba (developer: enable
pedantic by default, 2021-09-03).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
f70de51f2b vcxproj: require C11
This fixes the build after 7bc341e21b (git-compat-util: add a test
balloon for C99 support, 2021-12-01).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
cab6aeef03 vcxproj: allow building with NO_PERL again
This is another fall-out of the recent refactoring flurry.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:24 -07:00
Johannes Schindelin
744e2c28b8 git maintenance: avoid console window in scheduled tasks on Windows
We just introduced a helper to avoid showing a console window when the
scheduled task runs `git.exe`. Let's actually use it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
2023-05-19 11:52:20 -07:00
Johannes Schindelin
fd69bfd057 win32: add a helper to run git.exe without a foreground window
On Windows, there are two kinds of executables, console ones and
non-console ones. Git's executables are all console ones.

When launching the former e.g. in a scheduled task, a CMD window pops
up. This is not what we want for the tasks installed via the `git
maintenance` command.

To work around this, let's introduce `headless-git.exe`, which is a
non-console program that does _not_ pop up any window. All it does is to
re-launch `git.exe`, suppressing that console window, passing through
all command-line arguments as-are.

Helped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Helped-by: Yuyi Wang <Strawberry_Str@hotmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
2023-05-19 11:52:20 -07:00
Johannes Schindelin
893d80ce56 vcxproj: handle GUI programs, too
So far, we only built Console programs, but we are about to introduce a
program that targets the Windows subsystem (i.e. it is a so-called "GUI"
program).

Let's handle this preemptively in the script that generates the Visual
Studio files.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:20 -07:00
Johannes Schindelin
1879d42add vcxproj: ignore -fno-stack-protector and -fno-common
An upcoming commit will introduce those compile options; MSVC does not
understand them, so let's suppress them when generating the Visual
Studio project files.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:20 -07:00
Johannes Schindelin
1ce0f27c15 vcxproj: handle resource files, too
On Windows, we also compile a "resource" file, which is similar to
source code, but contains metadata (such as the program version).

So far, we did not compile it in `MSVC` mode, only when compiling Git
for Windows with the GNU C Compiler.

In preparation for including it also when compiling with MS Visual C,
let's teach our `vcxproj` generator to handle those sort of files, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:20 -07:00
Johannes Schindelin
731f0eeaa1 buildsystems: remove duplicate clause
This seems to have been there since 259d87c354 (Add scripts to
generate projects for other buildsystems (MSVC vcproj, QMake),
2009-09-16), i.e. since the beginning of that file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:20 -07:00
Jeff Hostetler
dec0be4d72 clink.pl: move default linker options for MSVC=1 builds
Move the default `-ENTRY` and `-SUBSYSTEM` arguments for
MSVC=1 builds from `config.mak.uname` into `clink.pl`.
These args are constant for console-mode executables.

Add support to `clink.pl` for generating a Win32 GUI application
using the `-mwindows` argument (to match how GCC does it).  This
changes the `-ENTRY` and `-SUBSYSTEM` arguments accordingly.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:20 -07:00
Jeff Hostetler
0c1018855a clink.pl: ignore no-stack-protector arg on MSVC=1 builds
Ignore the `-fno-stack-protector` compiler argument when building
with MSVC.  This will be used in a later commit that needs to build
a Win32 GUI app.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:20 -07:00
Jeff Hostetler
17839eeeb0 config.mak.uname: add git.rc to MSVC builds
Teach MSVC=1 builds to depend on the `git.rc` file so that
the resulting executables have Windows-style resources and
version number information within them.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:20 -07:00
Jeff Hostetler
92a7786dd0 vcbuild: add support for compiling Windows resource files
Create a wrapper for the Windows Resource Compiler (RC.EXE)
for use by the MSVC=1 builds. This is similar to the CL.EXE
and LIB.EXE wrappers used for the MSVC=1 builds.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:19 -07:00
Jeff Hostetler
4dd2270bc0 Makefile: clean up .ilk files when MSVC=1
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:19 -07:00
Jeff Hostetler
279774ee6b clink.pl: fix libexpatd.lib link error when using MSVC
When building with `make MSVC=1 DEBUG=1`, link to `libexpatd.lib`
rather than `libexpat.lib`.

It appears that the `vcpkg` package for "libexpat" has changed and now
creates `libexpatd.lib` for debug mode builds.  Previously, both debug
and release builds created a ".lib" with the same basename.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
2023-05-19 11:52:19 -07:00
Johannes Schindelin
79b9c71291 Merge branch 'dscho-avoid-d-f-conflict-in-vs-master'
Merge this early to resolve merge conflicts early.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:19 -07:00
Johannes Schindelin
0de8a64712 mingw: ignore HOMEDRIVE/HOMEPATH if it points to Windows' system directory
Internally, Git expects the environment variable `HOME` to be set, and
to point to the current user's home directory.

This environment variable is not set by default on Windows, and
therefore Git tries its best to construct one if it finds `HOME` unset.

There are actually two different approaches Git tries: first, it looks
at `HOMEDRIVE`/`HOMEPATH` because this is widely used in corporate
environments with roaming profiles, and a user generally wants their
global Git settings to be in a roaming profile.

Only when `HOMEDRIVE`/`HOMEPATH` is either unset or does not point to a
valid location, Git will fall back to using `USERPROFILE` instead.

However, starting with Windows Vista, for secondary logons and services,
the environment variables `HOMEDRIVE`/`HOMEPATH` point to Windows'
system directory (usually `C:\Windows\system32`).

That is undesirable, and that location is usually write-protected anyway.

So let's verify that the `HOMEDRIVE`/`HOMEPATH` combo does not point to
Windows' system directory before using it, falling back to `USERPROFILE`
if it does.

This fixes git-for-windows#2709

Initial-Path-by: Ivan Pozdeev <vano@mail.mipt.ru>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
3548998449 t5505/t5516: fix white-space around redirectors
The convention in Git project's shell scripts is to have white-space
_before_, but not _after_ the `>` (or `<`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
7d6943e6a6 mingw: allow git.exe to be used instead of the "Git wrapper"
Git for Windows wants to add `git.exe` to the users' `PATH`, without
cluttering the latter with unnecessary executables such as `wish.exe`.
To that end, it invented the concept of its "Git wrapper", i.e. a tiny
executable located in `C:\Program Files\Git\cmd\git.exe` (originally a
CMD script) whose sole purpose is to set up a couple of environment
variables and then spawn the _actual_ `git.exe` (which nowadays lives in
`C:\Program Files\Git\mingw64\bin\git.exe` for 64-bit, and the obvious
equivalent for 32-bit installations).

Currently, the following environment variables are set unless already
initialized:

- `MSYSTEM`, to make sure that the MSYS2 Bash and the MSYS2 Perl
  interpreter behave as expected, and

- `PLINK_PROTOCOL`, to force PuTTY's `plink.exe` to use the SSH
  protocol instead of Telnet,

- `PATH`, to make sure that the `bin` folder in the user's home
  directory, as well as the `/mingw64/bin` and the `/usr/bin`
  directories are included. The trick here is that the `/mingw64/bin/`
  and `/usr/bin/` directories are relative to the top-level installation
  directory of Git for Windows (which the included Bash interprets as
  `/`, i.e. as the MSYS pseudo root directory).

Using the absence of `MSYSTEM` as a tell-tale, we can detect in
`git.exe` whether these environment variables have been initialized
properly. Therefore we can call `C:\Program Files\Git\mingw64\bin\git`
in-place after this change, without having to call Git through the Git
wrapper.

Obviously, above-mentioned directories must be _prepended_ to the `PATH`
variable, otherwise we risk picking up executables from unrelated Git
installations. We do that by constructing the new `PATH` value from
scratch, appending `$HOME/bin` (if `HOME` is set), then the MSYS2 system
directories, and then appending the original `PATH`.

Side note: this modification of the `PATH` variable is independent of
the modification necessary to reach the executables and scripts in
`/mingw64/libexec/git-core/`, i.e. the `GIT_EXEC_PATH`. That
modification is still performed by Git, elsewhere, long after making the
changes described above.

While we _still_ cannot simply hard-link `mingw64\bin\git.exe` to `cmd`
(because the former depends on a couple of `.dll` files that are only in
`mingw64\bin`, i.e. calling `...\cmd\git.exe` would fail to load due to
missing dependencies), at least we can now avoid that extra process of
running the Git wrapper (which then has to wait for the spawned
`git.exe` to finish) by calling `...\mingw64\bin\git.exe` directly, via
its absolute path.

Testing this is in Git's test suite tricky: we set up a "new" MSYS
pseudo-root and copy the `git.exe` file into the appropriate location,
then verify that `MSYSTEM` is set properly, and also that the `PATH` is
modified so that scripts can be found in `$HOME/bin`, `/mingw64/bin/`
and `/usr/bin/`.

This addresses https://github.com/git-for-windows/git/issues/2283

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
384691edf4 t5505/t5516: allow running without .git/branches/ in the templates
When we commit the template directory as part of `make vcxproj`, the
`branches/` directory is not actually commited, as it is empty.

Two tests were not prepared for that situation.

This developer tried to get rid of the support for `.git/branches/` a
long time ago, but that effort did not bear fruit, so the best we can do
is work around in these here tests.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
e94869ffe5 mingw: ensure valid CTYPE
A change between versions 2.4.1 and 2.6.0 of the MSYS2 runtime modified
how Cygwin's runtime (and hence Git for Windows' MSYS2 runtime
derivative) handles locales: d16a56306d (Consolidate wctomb/mbtowc calls
for POSIX-1.2008, 2016-07-20).

An unintended side-effect is that "cold-calling" into the POSIX
emulation will start with a locale based on the current code page,
something that Git for Windows is very ill-prepared for, as it expects
to be able to pass a command-line containing non-ASCII characters to the
shell without having those characters munged.

One symptom of this behavior: when `git clone` or `git fetch` shell out
to call `git-upload-pack` with a path that contains non-ASCII
characters, the shell tried to interpret the entire command-line
(including command-line parameters) as executable path, which obviously
must fail.

This fixes https://github.com/git-for-windows/git/issues/1036

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
a1eedfa0d0 vcxproj: unclash project directories with build outputs
It already caused problems with the test suite that the directory
containing `git.vcxproj` is called the same as the Git executable
without its file extension: `./git` is ambiguous, it could refer both to
the directory `git/` as well as to `git.exe`.

Now there is one more problem: when our GitHub workflow runs on the
`vs/master` branch, it fails in all but the Windows builds, as they want
to write the file `git` but there is already a directory in the way.

Let's just go ahead and append `.proj` to all of those directories, e.g.
`git.proj/` instead of `git/`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:18 -07:00
Johannes Schindelin
3a43746ea6 Merge branch 'safe-PATH-lookup-in-gitk-on-Windows'
This topic branch extends the protections introduced for Git GUI's
CVE-2022-41953 to cover `gitk`, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:16 -07:00
Johannes Schindelin
8b55c77e79 Merge branch 'fix-win-rce'
This topic branch fixes a vulnerability in Git GUI's "clone" feature
(tracked as CVE-2022-41953) that was graded with a CVSS Score 8.6/10
(high).

These patches were backported to Git GUI in
https://github.com/prati0100/git-gui/pull/85

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:15 -07:00
Johannes Schindelin
88d856154a gitk(Windows): avoid inadvertently calling executables in the worktree
Just like CVE-2022-41953 for Git GUI, there exists a vulnerability of
`gitk` where it looks for `taskkill.exe` in the current directory before
searching `PATH`.

Note that the many `exec git` calls are unaffected, due to an obscure
quirk in Tcl's `exec` function. Typically, `git.exe` lives next to
`wish.exe` (i.e. the program that is run to execute `gitk` or Git GUI)
in Git for Windows, and that is the saving grace for `git.exe because
`exec` searches the directory where `wish.exe` lives even before the
current directory, according to
https://www.tcl-lang.org/man/tcl/TclCmd/exec.htm#M24:

	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 same is not true, however, for `taskkill.exe`: it lives in the
Windows system directory (never mind the 32-bit, Tcl's documentation is
outdated on that point, it really means `C:\Windows\system32`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2023-05-19 11:52:15 -07:00
Johannes Schindelin
15492bc3be Work around Tcl's default PATH lookup
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>
2023-05-19 11:52:15 -07:00
Johannes Schindelin
f133efb718 Move the _which function (almost) to the top
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>
2023-05-19 11:52:15 -07:00
Johannes Schindelin
d6e50bb3e4 Move is_<platform> functions to the beginning
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>
2023-05-19 11:52:15 -07:00
Matthew John Cheetham
92604ba1d4 Start the merging-rebase to v2.41.0-rc1
This commit starts the rebase of 3fa4728d8e to 7cdbaa5697
2023-05-19 11:52:14 -07:00
Johannes Schindelin
e73c2927c4 is_Cygwin: avoid execing anything
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>
2023-05-19 11:52:14 -07:00
Johannes Schindelin
ef0abed3ee windows: ignore empty PATH elements
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>
2023-05-19 11:52:14 -07:00
Junio C Hamano
4a714b3702 Git 2.41-rc1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-19 09:27:07 -07:00
Junio C Hamano
646ca89558 Merge branch 'jk/http-test-cgipassauth-unavailable-in-older-apache'
We started unconditionally testing with CGIPassAuth directive but
it is unavailable in older Apache that ships with CentOS 7 that has
about a year of shelf-life still left.  The test has conditionally
been disabled when running with an ancient Apache.  This was a fix
for a recent regression caught before the release, so no need to
mention it in the release notes.

* jk/http-test-cgipassauth-unavailable-in-older-apache:
  t/lib-httpd: make CGIPassAuth support conditional
2023-05-19 09:27:07 -07:00
Junio C Hamano
633390bd08 Merge branch 'bc/clone-empty-repo-via-protocol-v0'
The server side of "git clone" now advertises the necessary hints
to clients to help them to clone from an empty repository and learn
object hash algorithm and the (unborn) branch pointed at by HEAD,
even over the older v0/v1 protocol.

* bc/clone-empty-repo-via-protocol-v0:
  upload-pack: advertise capabilities when cloning empty repos
2023-05-19 09:27:06 -07:00
Junio C Hamano
b04671b638 Merge branch 'jc/send-email-pre-process-fix'
When "git send-email" that uses the validate hook is fed a message
without and then with Message-ID, it failed to auto-assign a unique
Message-ID to the former and instead reused the Message-ID from the
latter, which has been corrected.  This was a fix for a recent
regression caught before the release, so no need to mention it in
the release notes.

* jc/send-email-pre-process-fix:
  t9001: mark the script as no longer leak checker clean
  send-email: clear the $message_id after validation
2023-05-19 09:27:06 -07:00
Junio C Hamano
75ab1fa5ab Merge branch 'tb/run-command-needs-alloc-h'
Fix the build problem with NO_PTHREADS defined, a fallout from
recent header file shuffling.

* tb/run-command-needs-alloc-h:
  run-command.c: fix missing include under `NO_PTHREADS`
2023-05-19 09:27:06 -07:00
Jeff King
eb1c42da8e t/lib-httpd: make CGIPassAuth support conditional
Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access,
2023-02-27) added tests that require Apache to support the CGIPassAuth
directive, which was added in Apache 2.4.13. This is fairly old (~8
years), but recent enough that we still encounter it in the wild (e.g.,
RHEL/CentOS 7, which is not EOL until June 2024).

We can live with skipping the new tests on such a platform. But
unfortunately, since the directive is used unconditionally in our
apache.conf, it means the web server fails to start entirely, and we
cannot run other HTTP tests at all (e.g., the basic ones in t5551).

We can fix that by making the config conditional, and only triggering it
for t5563. That solves the problem for t5551 (which then ignores the
directive entirely). For t5563, we'd see apache complain in start_httpd;
with the default setting of GIT_TEST_HTTPD, we'd then skip the whole
script.

But that leaves one small problem: people may set GIT_TEST_HTTPD=1
explicitly, which instructs the tests to fail (rather than skip) when we
can't start the webserver (to avoid accidentally missing some tests).

This could be worked around by having the user manually set
GIT_SKIP_TESTS on a platform with an older Apache. But we can be a bit
friendlier by doing the version check ourselves and setting an
appropriate prereq. We'll use the (lack of) prereq to then skip the rest
of t5563. In theory we could use the prereq to skip individual tests, but
in practice this whole script depends on it.

Reported-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-18 14:29:32 -07:00
Junio C Hamano
20bd08aefb t9001: mark the script as no longer leak checker clean
The test uses "format-patch --thread" which is known to leak the
generated message ID list.

Plugging these leaks involves straightening out the memory ownership
rules around rev_info.message_id and rev_info.ref_message_ids, and
is beyond the scope of send-email fix, so for now mark the test as
leaky to unblock the topic before the release.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-17 16:47:36 -07:00
Junio C Hamano
3ece9bf0f9 send-email: clear the $message_id after validation
Recently git-send-email started parsing the same message twice, once
to validate _all_ the message before sending even the first one, and
then after the validation hook is happy and each message gets sent,
to read the contents to find out where to send to etc.

Unfortunately, the effect of reading the messages for validation
lingered even after the validation is done.  Namely $message_id gets
assigned if exists in the input files but the variable is global,
and it is not cleared before pre_process_file runs.  This causes
reading a message without a message-id followed by reading a message
with a message-id to misbehave---the sub reports as if the message
had the same id as the previously written one.

Clear the variable before starting to read the headers in
pre_process_file.

Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-17 14:11:38 -07:00
brian m. carlson
933e3a4ee2 upload-pack: advertise capabilities when cloning empty repos
When cloning an empty repository, protocol versions 0 and 1 currently
offer nothing but the header and flush packets for the /info/refs
endpoint. This means that no capabilities are provided, so the client
side doesn't know what capabilities are present.

However, this does pose a problem when working with SHA-256
repositories, since we use the capabilities to know the remote side's
object format (hash algorithm).  As of 8b214c2e9d ("clone: propagate
object-format when cloning from void", 2023-04-05), this has been fixed
for protocol v2, since there we always read the hash algorithm from the
remote.

Fortunately, the push version of the protocol already indicates a clue
for how to solve this.  When the /info/refs endpoint is accessed for a
push and the remote is empty, we include a dummy "capabilities^{}" ref
pointing to the all-zeros object ID.  The protocol documentation already
indicates this should _always_ be sent, even for fetches and clones, so
let's just do that, which means we'll properly announce the hash
algorithm as part of the capabilities.  This just works with the
existing code because we share the same ref code for fetches and clones,
and libgit2, JGit, and dulwich do as well.

There is one minor issue to fix, though.  If we called send_ref with
namespaces, we would return NULL with the capabilities entry, which
would cause a crash.  Instead, let's refactor out a function to print
just the ref itself without stripping the namespace and use it for our
special capabilities entry.

Add several sets of tests for HTTP as well as for local clones.  The
behavior can be slightly different for HTTP versus a local or SSH clone
because of the stateless-rpc functionality, so it's worth testing both.

Signed-off-by: brian m. carlson <bk2204@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-17 13:22:46 -07:00