Mark Levedahl [Wed, 21 May 2025 20:31:14 +0000 (16:31 -0400)]
git-gui: always use themed widgets from ttk
git-gui optionally uses themed ui elements from ttk, but the full set of
ttk ui elements is always available with Tk 8.6. Keeping code making
ttk use optional increases maintenance burden for no benefit. Let's use
ttk always, allowing removal of alternate code paths in subsequent
patches.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Mark Levedahl [Sun, 18 Feb 2024 18:06:05 +0000 (13:06 -0500)]
git-gui: remove redundant check for Tk >= 8.5
Since commit c80d7be5e1e0d, git-gui checks for the availability of ttk
before enabling its use, but this check is redundant as Tk >= 8.6 is
required. Remove the redundant check.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Mark Levedahl [Fri, 16 Feb 2024 23:24:06 +0000 (18:24 -0500)]
git-gui: remove unreachable Tk 8.4 code
git-gui has remnant code to allow some drawing with Tk 8.4 predating the
addition of themed widgets. As git-gui requires Tk >= 8.6, this code can
never trigger. Remove it.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Mark Levedahl [Sat, 17 May 2025 02:25:17 +0000 (22:25 -0400)]
git-gui: Make TclTk 8.6 the minimum, allow 8.7
git-gui requires that Tcl and Tk are 8.5, though the check using
'package require' allows 8.6. As git-gui runs under wish, both Tcl and
Tk are always available and of the same version, so only one need be
checked.
The 8.5 requirement is very outdated as the earliest Tcl currently
shipping on any supported OS is 8.6. 8.7 is in alpha test and is
generally compatible with 8.6, so should also be allowed. Tcl 9.0 has
planned compatibility breaking changes so cannot be allowed.
Let's update the requirements to be 8.6 or 8.7, and check only on Tcl as
Tk will be the same version.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Johannes Sixt [Tue, 8 Jul 2025 19:22:00 +0000 (21:22 +0200)]
Merge branch 'js/fix-open-exec-git'
This addresses CVE-2025-46835, Git GUI can create and overwrite a
user's files:
When a user clones an untrusted repository and is tricked into editing
a file located in a maliciously named directory in the repository, then
Git GUI can create and overwrite files for which the user has write
permission.
* js/fix-open-exec-git:
git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls
git-gui: do not mistake command arguments as redirection operators
git-gui: introduce function git_redir for git calls with redirections
git-gui: pass redirections as separate argument to git_read
git-gui: pass redirections as separate argument to _open_stdout_stderr
git-gui: convert git_read*, git_write to be non-variadic
git-gui: use git_read in githook_read
git-gui: break out a separate function git_read_nice
git-gui: remove option --stderr from git_read
git-gui: sanitize 'exec' arguments: background
git-gui: sanitize 'exec' arguments: simple cases
git-gui: treat file names beginning with "|" as relative paths
git-gui: remove git config --list handling for git < 1.5.3
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
Johannes Sixt [Tue, 8 Jul 2025 19:19:28 +0000 (21:19 +0200)]
Merge branch 'ml/replace-auto-execok'
This addresses CVE-2025-46334, Git GUI malicious command injection on
Windows.
A malicious repository can ship versions of sh.exe or typical textconv
filter programs such as astextplain. Due to the unfortunate design of
Tcl on Windows, the search path when looking for an executable always
includes the current directory. The mentioned programs are invoked when
the user selects "Git Bash" or "Browse Files" from the menu.
* ml/replace-auto-execok:
git-gui: override exec and open only on Windows
git-gui: sanitize $PATH on all platforms
git-gui: assure PATH has only absolute elements.
git-gui: cleanup git-bash menu item
git-gui: avoid auto_execok in do_windows_shortcut
git-gui: avoid auto_execok for git-bash menu item
git-gui: remove unused proc is_shellscript
git-gui: remove special treatment of Windows from open_cmd_pipe
git-gui: use only the configured shell
git-gui: make _shellpath usable on startup
git-gui: use [is_Windows], not bad _shellpath
git-gui: _which, only add .exe suffix if not present
Johannes Sixt [Fri, 6 Jun 2025 05:41:42 +0000 (07:41 +0200)]
git-gui: don't delete source files when auto_mkindex fails
Commit 2cc5b0facfa4 (git-gui: extract script to generate "tclIndex",
2025-03-11) converted commands in a Makefile rule to a shell script.
In this process, the Makefile variable $@ had to be replaced by the
file name that it represents, 'lib/tclIndex'. However, the occurrence
in `rm -f $@` was missed. In a shell script, $@ expands to all
command line arguments, which happen to be the source files lib/*.tcl
in this case. Needless to say that we do not want to remove source
files during a build. Replace $@ by the intended 'lib/tclIndex'.
Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Johannes Sixt [Thu, 29 May 2025 08:01:14 +0000 (10:01 +0200)]
Merge branch 'pks-meson-support' of github.com:pks-t/git-gui
* 'pks-meson-support' of github.com:pks-t/git-gui:
git-gui: wire up support for the Meson build system
git-gui: stop including GIT-VERSION-FILE file
git-gui: extract script to generate macOS app
git-gui: extract script to generate macOS wrapper
git-gui: extract script to generate "tclIndex"
git-gui: extract script to generate "git-gui"
git-gui: drop no-op GITGUI_SCRIPT replacement
git-gui: make output of GIT-VERSION-GEN source'able
git-gui: prepare GIT-VERSION-GEN for out-of-tree builds
git-gui: replace GIT-GUI-VARS with GIT-GUI-BUILD-OPTIONS
Johannes Sixt [Sun, 4 May 2025 19:59:19 +0000 (21:59 +0200)]
git-gui: do not mistake command arguments as redirection operators
Tcl 'open' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are many calls of the
'open' variant that runs a process which construct arguments that are
taken from the Git repository or are user input. However, when file
names or ref names are taken from the repository, it is possible to
find names that have these special forms. They must not be interpreted
by 'open' lest it redirects input or output, or attempts to build a
pipeline using a command name controlled by the repository.
Use the helper function make_arglist_safe, which identifies such
arguments and prepends "./" to force such a name to be regarded as a
relative file name.
After this change the following 'open' calls that start a process do not
apply the argument processing:
In all cases, the command arguments are constant strings (or begin with
a constant string) that are of a form that would not be affected by the
processing anyway.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 18:26:11 +0000 (20:26 +0200)]
git-gui: introduce function git_redir for git calls with redirections
Proc git invokes git and collects all output, which is it returns.
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. A few invocations also pass redirection operators as
command arguments deliberately. Rewrite these cases to use a new
function git_redir that takes two lists, one for the regular command
arguments and one for the redirection operations.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 13:39:03 +0000 (15:39 +0200)]
git-gui: pass redirections as separate argument to git_read
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. Rewrite direct call sites of git_read
to pass intentional redirections as a second (optional) argument.
git_read defers to safe_open_command, but we cannot make it safe, yet,
because one of the callers of git_read is proc git, which does not yet
know which of its arguments are redirections. This is the topic of the
next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 4 May 2025 13:06:11 +0000 (15:06 +0200)]
git-gui: pass redirections as separate argument to _open_stdout_stderr
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. Rewrite direct callers of
_open_stdout_stderr to pass intentional redirections as a second
(optional) argument.
Passing arbitrary arguments is not safe right now, but we rename it
to safe_open_command anyway to avoid having to touch the call sites
again later when we make it actually safe.
We cannot make the function safe right away because one caller is
git_read, which does not yet know which of its arguments are
redirections. This is the topic of the next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:24:48 +0000 (13:24 +0200)]
git-gui: convert git_read*, git_write to be non-variadic
We are going to treat command arguments and redirections differently to
avoid passing arguments that look like redirections to the command
accidentally. To do so, it will be necessary to know which arguments
are intentional redirections. As a preparation, convert git_read,
git_read_nice, and git_write to take just a single argument that is
the command in a list. Adjust all call sites accordingly.
In the future, this argument will be the regular command arguments and
a second argument will be the redirection operations.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:58:20 +0000 (10:58 -0400)]
git-gui: override exec and open only on Windows
Since aae9560a355d (Work around Tcl's default `PATH` lookup,
2022-11-23), git-gui overrides exec and open on all platforms. But,
this was done in response to Tcl adding elements to $PATH on Windows,
while exec, open, and auto_execok honor $PATH as given on all other
platforms.
Let's do the override only on Windows, restoring others to using their
native exec and open. These honor the sanitized $PATH as that is written
out to env(PATH) in a previous commit. auto_execok is also safe on these
platforms, so can be used for _which.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 17:21:53 +0000 (19:21 +0200)]
git-gui: use git_read in githook_read
0730a5a3a5e6 ("git-gui - use git-hook, honor core.hooksPath", 2023-09-17)
rewrote githook_read to use `git hook` to run a hook script. The code
that was replaced discovered the hook script file manually and invoked
it using function _open_stdout_stderr. After the rewrite, this function
is still invoked, but it calls into `git` instead of the hook scripts.
Notice though, that we have function git_read that invokes git and
prepares a pipe for the caller to read from. Replace the implementation
of githook_read to be just a wrapper around git_read. This unifies the
way in which the git executable is invoked. git_read ultimately also
calls into _open_stdout_stderr, but it modifies the path to the git
executable before doing so.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:47:04 +0000 (10:47 -0400)]
git-gui: sanitize $PATH on all platforms
Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui removes empty elements from $PATH, and a prior commit made this
remove all non-absolute elements from $PATH. But, this happens only on
Windows. Unsafe $PATH elements in $PATH are possible on all platforms.
Let's sanitize $PATH on all platforms to have consistent behavior. If a
user really wants the current repository on $PATH, they can add its
absolute name to $PATH.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:11:21 +0000 (13:11 +0200)]
git-gui: break out a separate function git_read_nice
There are two callers of git_read that request special treatment using
option --nice. Rewrite them to call a new function git_read_nice that
does the special treatment. Now we can remove all option treatment from
git_read.
git_write has the same capability, but there are no callers that
request --nice. Remove the feature without substitution.
This is a preparation for a later change where we want to make git_read
and friends non-variadic. Then it cannot have optional arguments.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 11 Apr 2025 14:08:52 +0000 (10:08 -0400)]
git-gui: assure PATH has only absolute elements.
Since 8f23432b38d9 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui excises all empty paths from $PATH, but still allows '.' or
other relative paths, which can also allow executing code from the
repository. Let's remove anything except absolute elements. While here,
let's remove duplicated elements, which are very common on Windows:
only the first such item can do anything except waste time repeating a
search.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 09:52:35 +0000 (11:52 +0200)]
git-gui: remove option --stderr from git_read
Some callers of git_read want to redirect stderr of the invoked command
to stdout. The function offers option --stderr for this purpose.
However, the option only appends 2>@1 to the commands. The callers can
do that themselves. In lib/console.tcl we even have a caller that
already knew implictly what --stderr does behind the scenes.
This is a preparation for a later change where we want to make git_read
non-variadic. Then it cannot have optional leading arguments.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Mon, 7 Apr 2025 21:12:56 +0000 (17:12 -0400)]
git-gui: cleanup git-bash menu item
git-gui on Git for Windows creates a menu item to start a git-bash
session for the current repository. This menu-item works as desired when
git-gui is installed in the Git for Windows (g4w) distribution, but
not when run from a different location such as normally done in
development. The reason is that git-bash's location is known to be
'/git-bash' in the Unix pathname space known to MSYS, but this is not
known in the Windows pathname space. Instead, git-gui derives a pathname
for git-bash assuming it is at a known relative location.
If git-gui is run from a different directory than assumed in g4w, the
relative location changes, and git-gui resorts to running a generic bash
login session in a Windows console.
But, the MSYS system underlying Git for Windows includes the 'cygpath'
utility to convert between Unix and Windows pathnames. Let's use this so
git-bash's Windows pathname is determined directly from /git-bash.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 26 Apr 2025 16:46:06 +0000 (18:46 +0200)]
git-gui: sanitize 'exec' arguments: background
As in the previous commits, introduce a function that sanitizes
arguments intended for the process, but runs the process in the
background. Convert 'exec' calls to use this new function.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Thu, 3 Apr 2025 04:37:08 +0000 (00:37 -0400)]
git-gui: avoid auto_execok in do_windows_shortcut
git-gui on Windows uses auto_execok to locate git-gui.exe,
which performs the same flawed search as does the builtin exec.
Use _which instead, performing a safe PATH lookup.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 21 Apr 2025 16:14:54 +0000 (18:14 +0200)]
git-gui: sanitize 'exec' arguments: simple cases
Tcl 'exec' assigns special meaning to its argument when they begin with
redirection, pipe or background operator. There are a number of
invocations of 'exec' which construct arguments that are taken from the
Git repository or a user input. However, when file names or ref names
are taken from the repository, it is possible to find names that have
these special forms. They must not be interpreted by 'exec' lest it
redirects input or output, or attempts to build a pipeline using a
command name controlled by the repository.
Introduce a helper function that identifies such arguments and prepends
"./" to force such a name to be regarded as a relative file name.
Convert those 'exec' calls where the arguments can simply be packed
into a list.
Note that most commands containing the word 'exec' route through
console::exec or console::chain, which we will treat in another commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 2 Apr 2025 21:37:27 +0000 (17:37 -0400)]
git-gui: avoid auto_execok for git-bash menu item
On Windows, git-gui offers to open a git-bash session for the current
repository from the menu, but uses [auto_execok start] to get the
command to actually run that shell.
The code for auto_execok, in /usr/share/tcl8.6/tcl.init, has 'start' in
the 'shellBuiltins' list for cmd.exe on Windows: as a result,
auto_execok does not actually search for start, meaning this usage is
technically ok with auto_execok now. However, leaving this use of
auto_execok in place will just induce confusion about why a known unsafe
function is being used on Windows. Instead, let's switch to using our
known safe _which function that looks only in $PATH, excluding the
current working directory.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Mon, 21 Apr 2025 15:07:10 +0000 (17:07 +0200)]
git-gui: treat file names beginning with "|" as relative paths
The Tcl 'open' function has a very wide interface. It can open files as
well as pipes to external processes. The difference is made only by the
first character of the file name: if it is "|", a process is spawned.
We have a number of calls of Tcl 'open' that take a file name from the
environment in which Git GUI is running. Be prepared that insane values
are injected. In particular, when we intend to open a file, do not take
a file name that happens to begin with "|" as a request to run a process.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 4 Apr 2025 20:55:59 +0000 (16:55 -0400)]
git-gui: remove unused proc is_shellscript
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added is_shellscript to test if a file
is executable by the shell, used only when searching for textconv
filters. The previous commit rearranged the tests for finding such
filters, and removed the only user of is_shellscript. Remove this
function.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sat, 3 May 2025 11:37:35 +0000 (13:37 +0200)]
git-gui: remove git config --list handling for git < 1.5.3
git-gui uses `git config --null --list` to parse configuration. Git
versions prior to 1.5.3 do not have --null and need different treatment.
Nobody should be using such an old version anymore. (Moreover, since 0730a5a3a, git-gui requires git v2.36 or later). Keep only the code for
modern Git.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Sun, 18 May 2025 14:08:06 +0000 (16:08 +0200)]
git-gui: remove special treatment of Windows from open_cmd_pipe
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added open_cmd_pipe to run text
conversion in support of blame, with special handling for shell
scripts on Windows. To determine whether the command is a shell
script, 'lindex' is used to pick off the first token from the command.
However, cmd is actually a command string taken from .gitconfig
literally and is not necessarily a syntactically correct Tcl list.
Hence, it cannot be processed by 'lindex' and 'lrange' reliably.
Pass the command string to the shell just like on non-Windows
platforms to avoid the potentially incorrect treatment.
A use of 'auto_execok' is removed by this change. This function is
dangerous on Windows, because it searches programs in the current
directory. Delegating the path lookup to the shell is safe, because
/bin/sh and /bin/bash follow POSIX on all platforms, including the
Git for Windows port.
A possible regression is that the old code, given filter command of
'foo', could find 'foo.bat' as a script, and not just bare 'foo', or
'foo.exe'. This rewrite requires explicitly giving the suffix if it is
not .exe.
This part of Git GUI can be exercised using
git gui blame -- some.file
while some.file has a textconv filter configured and has unstaged
modifications.
Helped-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Fri, 2 May 2025 15:39:55 +0000 (11:39 -0400)]
git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui provides an implementation to detach HEAD on Git versions prior
to 1.5.3. Nobody should be using such an old version anymore.
(Moreover, since 0730a5a3a, git-gui requires git v2.36 or later).
Keep only the code for modern Git.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
[j6t: message tweaked] Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Sun, 6 Apr 2025 22:20:14 +0000 (18:20 -0400)]
git-gui: use only the configured shell
git-gui has a few places where a bare "sh" is passed to exec, meaning
that the first instance of "sh" on $PATH will be used rather than the
shell configured. This violates expectations that the configured shell
is being used. Let's use [shellpath] everywhere.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 20 Sep 2023 21:56:14 +0000 (17:56 -0400)]
git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
Since b792230 ("git-gui: Show a progress meter for checking out files",
2007-07-08), git-gui includes a workaround for Tcl that does not support
using 2>@1 to redirect stderr to stdout. Tcl added such support in
8.4.7, released in 2004, and this is fully supported in all 8.5
releases.
As git-gui has a hard-coded requirement for Tcl >= 8.5, the workaround
is no longer needed. Delete it.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Tue, 1 Apr 2025 15:45:06 +0000 (11:45 -0400)]
git-gui: make _shellpath usable on startup
Since commit d5257fb3c1de (git-gui: handle textconv filter on
Windows and in development, 2010-08-07), git-gui will search for a
usable shell if _shellpath is not configured, and on Windows may
resort to using auto_execok to find 'sh'. While this was intended for
development use, checks are insufficient to assure a proper
configuration when deployed where _shellpath is always set, but might
not give a usable shell.
Let's make this more robust by only searching if _shellpath was not
defined, and then using only our restricted search functions.
Furthermore, we should convert to a Windows path on Windows. Always
check for a valid shell on startup, meaning an absolute path to an
executable, aborting if these conditions are not met.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Wed, 2 Apr 2025 15:23:03 +0000 (11:23 -0400)]
git-gui: use [is_Windows], not bad _shellpath
Commit 7d076d56757c (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added open_cmd_pipe, with special
handling for Windows detected by seeing that _shellpath does not
point to an executable shell. That is bad practice, and is broken by
the next commit that assures _shellpath is valid on all platforms.
Fix this by using [is_Windows] as done for all Windows specific code.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Mark Levedahl [Thu, 3 Apr 2025 14:26:21 +0000 (10:26 -0400)]
git-gui: _which, only add .exe suffix if not present
The _which function finds executables on $PATH, and adds .exe on Windows
unless -script was given. However, win32.tcl executes "wscript.exe"
and "cscript.exe", both of which fail as _which adds .exe to both. This
is already fixed in git-gui released by Git for Windows. Do so here.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Johannes Sixt [Wed, 14 May 2025 20:41:30 +0000 (22:41 +0200)]
git-gui: do not end the commit message with an empty line
The commit message is processed to remove unnecessary empty lines.
In particular, it is ensured that the text ends with at most one LF
character. This one is always present, because the Tk text widget
ensures that is present.
However, did not consider that the processed text is written to the
commit message file using `puts`, which also appends a LF character,
so that the final commit message ends with two LF. Trim all trailing
LF characters, and while we are here, use `string trim`, which lets
us remove the leading LF in the same command.
Reported-by: Gareth Fenn <garethfenn@gmail.com> Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
git-gui: wire up support for the Meson build system
The Git project has started to wire up Meson as a build system in Git
v2.48.0. Wire up support for Meson in "git-gui" so that we can trivially
include it as a subproject in Git.
The "GITGUI_VERSION" variable is made available by generating and
including the "GIT-VERSION-FILE" file. Its value has been used in
various build steps, but in the preceding commits we have refactored
those to instead source the "GIT-VERSION-FILE" directly. As a result,
the variable is now only used in a single recipe, and this use can be
trivially replaced with sed(1).
Refactor the recipe to do so and stop including "GIT-VERSION-FILE" to
simplify the build process.
Extract script to generate the macOS app. This change allows us to reuse
the build logic with the Meson build system.
Note that as part of this change we also modify the TKEXECUTABLE
variable to track its full path. Like this we don't have to propagate
both the TKEXECUTABLE and TKFRAMEWORK variables into the script, and the
basename can be trivially computed from TKEXECUTABLE anyway.
The value of the GITGUI_SCRIPT variable is only used in a single place
as part of an sed(1) script that massages the "git-gui.sh" script.
Interestingly, this specific replacement does seem to be a no-op: we
replace "^ argv0=$$0" with " argv=$(GITGUI_SCRIPT)", which has a value
of "$$0". The result would thus be completely unchanged.
git-gui: make output of GIT-VERSION-GEN source'able
The output of GIT-VERSION-GEN can be sourced by our Makefile to make the
version available there. The output has a couple of spaces around the
equals sign, which is perfectly valid for parsing it in our Makefile.
But in subsequent steps we'll also want to source the file in a couple
of newly-introduced shell scripts, but having spaces around variable
assignments is invalid there.
Prepare for this step by dropping the spaces surrounding the equals
sign. Like this, we can easily use the same file both in our Makefile
and in shell scripts.
git-gui: prepare GIT-VERSION-GEN for out-of-tree builds
The GIT-VERSION-GEN unconditionally writes version information into the
source directory in the form of the "GIT-VERSION-FILE". We are about to
introduce the Meson build system though, which enforces out-of-tree
builds by default, and in that context we cannot continue to write
version information into the source tree.
Prepare the script for out-of-tree builds by treating the source
directory different from the output file.
git-gui: replace GIT-GUI-VARS with GIT-GUI-BUILD-OPTIONS
The GIT-GUI-VARS file is used to track whether any of our build options
has changed. Unfortunately, the format of that file does not allow us to
propagate those build options to other scripts. But as we are about to
introduce support for the Meson build system, we will extract a couple
of scripts to deduplicate core build logic across Makefiles and Meson.
With this refactoring, it will become necessary to make build options
more widely accessible.
Replace GIT-GUI-VARS with a new GIT-GUI-BUILD-OPTIONS file that is being
populated from a template. This file can easily be sourced from build
scripts in subsequent steps.
Johannes Sixt [Tue, 24 Dec 2024 13:47:08 +0000 (14:47 +0100)]
git-gui: treat the message template file as a built file
Follow the lead of 5377abc0c9d5 ("po/git.pot: don't check in result
of "make pot"", 2022-05-26) in the Git repository and do not track
git-gui.pot anymore.
Instead, translators are expected to integrate an up-to-date version
from the master branch into their translation file using
make ALL_POFILES=po/xx.po update-po
Update README to describe the new process. It is now understood that
different translations need not be based on the same message template
file, but rather individual translators should base their translation
on the most up-to-date code. Remove the section that addresses the
i18n coordinator as it does not apply when no common base is required
among translators.
This amends 1ae85ff6d (git-gui: strip comments and consecutive empty
lines from commit messages, 2024-08-13) to deal with custom comment
characters/strings.
The magic commentString value "auto" is not handled, because the option
makes no sense to me - it does not support comments in templates and
hook output, and it seems far-fetched that someone would introduce
comments during editing the message.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
git gui: add directly calling merge tool from configuration
git gui can open a merge tool when conflicts are detected (Right click
in the diff of the file with conflicts).
The merge tools that are allowed to use are hard coded into git gui.
If one wants to add a new merge tool it has to be added to git gui
through a source code change.
This is not convenient in comparison to how it works in git (without gui).
git itself has configuration options for a merge tools path and command
in the git configuration.
New merge tools can be set up there without a source code change.
Those options are used only by pure git in contrast to git gui. git calls
the configured merge tools directly from the configuration while git Gui
doesn't.
With this change git gui can call merge tools configured in the
configuration directly without a change in git gui source code.
It needs a configured "merge.tool" and a configured
"mergetool.<mergetool name>.cmd" configuration entry as shown in the
git-config manual page.
Without the "mergetool.<mergetool name>.cmd" entry and an unsupported
"merge.tool" entry, git gui behaves mainly as before this change and
informs the user about an unsupported merge tool. In addtition, it also
shows a hint to add a configuration entry to use the tool as an
unsupported tool with degraded support.
If a wrong "mergetool.<mergetool name>.cmd" is configured by accident,
it gets handled by git gui already. In this case git gui informs the
user that the merge tool couldn't be opened. This behavior is preserved
by this change and should not change.
"Beyond Compare 3" and "Visual Studio Code" were tested as manually
configured merge tools.
Signed-off-by: Tobias Boesch <tobias.boesch@miele.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
git-gui: strip comments and consecutive empty lines from commit messages
This is also known as "washing". This is consistent with the behavior of
interactive git commit, which we should emulate as closely as possible
to avoid usability problems. This way commit message templates and
prepare hooks can be used properly, and comments from conflicted rebases
and merges are cleaned up without having to introduce special handling
for them.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Johannes Sixt [Sat, 3 Aug 2024 15:22:51 +0000 (17:22 +0200)]
git-gui: Remove forced rescan of stat-dirty files.
It is possible that stat information of tracked files is modified without
actually modifying the content. Plumbing commands would detect such files
as modified, so that Git GUI runs `git update-info --refresh` in order to
synchronize the cached stat info with the reality. However, this can be
an expensive operation in large repositories. As remediation, e534f3a88676 (git-gui: Allow the user to disable update-index --refresh
during rescan, 2006-11-07) introduced an option to skip the expensive
part.
The option was named "trust file modification timestamp". But the catch
is that sometimes file timestamps can't be trusted. In this case, a file
would remain listed in Unstaged Changes although there are no changes.
So 16403d0b1f9d (git-gui: Refresh a file if it has an empty diff,
2006-11-11) introduced a popup message informing the user about the
situation and then removed the file from the Unstaged Changes list.
Now users had to click away the message box for every file that was
stat-dirty. Under the assumption that a file in such a state is not
the only one, 124355d32c06 (git-gui: Always start a rescan on an empty
diff, 2007-01-22) introduced a forced (potentially expensive) refresh
that would de-list all stat-dirty files after the first notification was
dismissed.
Along came 6c510bee2013 (Lazy man's auto-CRLF, 2007-02-13) in Git. It
introduced a new case where a file in the worktree can have no essential
differences to the staged version, but still be detected as modified by
plumbing commands. This time, however, the index cannot be synchronized
fully by `git update-index --refresh`, so that the file remains listed
in Unstaged Changes until it is staged manually.
Needless to say that the message box now becomes an annoyance, because
it must be dismissed every time an affected file is selected, and the
file remains listed nevertheless.
Remove the message box. Write the notice that no differences were found
in the diff panel instead. Also include a link that, when clicked,
initiates the rescan. With this scheme, the rescan does not happen
automatically anymore, but requires an additional click. (This is now
two clicks in total for users who encounter stat-dirty files after
enabling the "trust file modification timestamps" option.) However,
users whom the rescan does not help (autocrlf-related dirty files) save
half the clicks because there is no message box to dismiss.
Johannes Sixt [Sun, 7 Jul 2024 12:00:23 +0000 (14:00 +0200)]
Merge branch 'os/catch-rename'
The problem can be reproduced on Linux with this sequence:
1. Run git gui from a terminal.
2. Edit the commit message and wait for at least 2 seconds.
3. Terminate the instance from the terminal, for example with Ctrl-C,
to simulate crash. This leaves the file .git/GITGUI_BCK behind.
4. Start two instances of git gui &.
At this point the first instance can be closed (it renames
.git/GITGUI_BCK to .git/GITGUI_MSG), but the seconds brings an error
message about the absent file and cannot be closed thereafter and must
be killed from the command line.
The renaming that happens by the first instance is the correct action
and need not be repeated by the second instance. It is the correct
action to ignore the failed renaming.
On the other hand, the second instance could just edit the commit
message again, wait 2 seconds to write GITGUI_BCK, and then can be
closed without failing. At this point, since the user has edited the
message, it is again correct to preserve the edited version in
GITGUI_MSG.
* os/catch-rename:
git-gui: fix inability to quit after closing another instance
Orgad Shaneh [Tue, 7 Feb 2023 07:43:17 +0000 (09:43 +0200)]
git-gui: fix inability to quit after closing another instance
If you open 2 git gui instances in the same directory, then close one
of them and try to close the other, an error message pops up, saying:
'error renaming ".git/GITGUI_BCK": no such file or directory', and it
is no longer possible to close the window ever.
Fix by catching this error, and proceeding even if the file no longer
exists.
Junio C Hamano [Mon, 8 Apr 2024 23:36:05 +0000 (16:36 -0700)]
Makefile(s): do not enforce "all indents must be done with tab"
Our top-level Makefile follows our generic whitespace rule
established by the top-level .gitattributes file that does not
enforce indent-with-non-tab rule by default, but git-gui is set up
to enforce indent-with-non-tab by default. With the upcoming change
to GNU make, we no longer can reject (and worse, "fix") a patch that
adds whitespace indented lines to the Makefile, so loosen the rule
there for git-gui/Makefile, too.
Taylor Blau [Mon, 8 Apr 2024 15:51:44 +0000 (11:51 -0400)]
Makefile(s): avoid recipe prefix in conditional statements
In GNU Make commit 07fcee35 ([SV 64815] Recipe lines cannot contain
conditional statements, 2023-05-22) and following, conditional
statements may no longer be preceded by a tab character (which Make
refers to as the recipe prefix).
There are a handful of spots in our various Makefile(s) which will break
in a future release of Make containing 07fcee35. For instance, trying to
compile the pre-image of this patch with the tip of make.git results in
the following:
$ make -v | head -1 && make
GNU Make 4.4.90
config.mak.uname:842: *** missing 'endif'. Stop.
The kernel addressed this issue in 82175d1f9430 (kbuild: Replace tabs
with spaces when followed by conditionals, 2024-01-28). Address the
issues in Git's tree by applying the same strategy.
When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
more tab characters, replace each tab character with 8 space characters
with the following:
Reported-by: Dario Gjorgjevski <dario.gjorgjevski@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The French word "aperçu", meaning "view" or "preview", contains only a
single letter "p". Remove the extra letter, which is an obvious typo.
Reported-by: Léonard Michelet <leonard@lebasic.com> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Mark Levedahl [Sun, 17 Sep 2023 19:24:31 +0000 (15:24 -0400)]
git-gui - use git-hook, honor core.hooksPath
git-gui currently runs some hooks directly using its own code written
before 2010, long predating git v2.9 that added the core.hooksPath
configuration to override the assumed location at $GIT_DIR/hooks. Thus,
git-gui looks for and runs hooks including prepare-commit-msg,
commit-msg, pre-commit, post-commit, and post-checkout from
$GIT_DIR/hooks, regardless of configuration. Commands (e.g., git-merge)
that git-gui invokes directly do honor core.hooksPath, meaning the
overall behaviour is inconsistent.
Furthermore, since v2.36 git exposes its hook execution machinery via
`git-hook run`, eliminating the need for others to maintain code
duplicating that functionality. Using git-hook will both fix git-gui's
current issues on hook configuration and (presumably) reduce the
maintenance burden going forward. So, teach git-gui to use git-hook.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mark Levedahl [Sat, 16 Sep 2023 21:01:31 +0000 (17:01 -0400)]
git-gui - re-enable use of hook scripts
Earlier, commit aae9560a introduced search in $PATH to find executables
before running them, avoiding an issue where on Windows a same named
file in the current directory can be executed in preference to anything
in a directory in $PATH. This search is intended to find an absolute
path for a bare executable ( e.g, a function "foo") by finding the first
instance of "foo" in a directory given in $PATH, and this search works
correctly. The search is explicitly avoided for an executable named
with an absolute path (e.g., /bin/sh), and that works as well.
Unfortunately, the search is also applied to commands named with a
relative path. A hook script (or executable) $HOOK is usually located
relative to the project directory as .git/hooks/$HOOK. The search for
this will generally fail as that relative path will (probably) not exist
on any directory in $PATH. This means that git hooks in general now fail
to run. Considerable mayhem could occur should a directory on $PATH be
git controlled. If such a directory includes .git/hooks/$HOOK, that
repository's $HOOK will be substituted for the one in the current
project, with unknown consequences.
This lookup failure also occurs in worktrees linked to a remote .git
directory using git-new-workdir. However, a worktree using a .git file
pointing to a separate git directory apparently avoids this: in that
case the hook command is resolved to an absolute path before being
passed down to the code introduced in aae9560a.
Fix this by replacing the test for an "absolute" pathname to a check for
a command name having more than one pathname component. This limits the
search and absolute pathname resolution to bare commands. The new test
uses tcl's "file split" command. Experiments on Linux and Windows, using
tclsh, show that command names with relative and absolute paths always
give at least two components, while a bare command gives only one.
Pratyush Yadav [Thu, 24 Aug 2023 14:46:29 +0000 (16:46 +0200)]
Merge branch 'ml/cygwin-fixes'
Remove some code supporting ancient Cygwin Tcl/Tk versions. Also fix
exploring working directory and making desktop shortcuts on Cygwin.
* ml/cygwin-fixes:
git-gui - use mkshortcut on Cygwin
git-gui - use cygstart to browse on Cygwin
git-gui - remove obsolete Cygwin specific code
git gui Makefile - remove Cygwin modifications
Mark Levedahl [Mon, 26 Jun 2023 16:53:05 +0000 (12:53 -0400)]
git-gui - use mkshortcut on Cygwin
git-gui enables the "Repository->Create Desktop Icon" item on Cygwin,
offering to create a shortcut that starts git-gui on the current
repository. The code in do_cygwin_shortcut invokes function
win32_create_lnk to create the shortcut. This latter function is shared
between Cygwin and Git For Windows and expects Windows rather than unix
pathnames, though do_cygwin_shortcut provides unix pathnames. Also, this
function tries to invoke the Windows Script Host to run a javascript
snippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just
does not support Cygwin.
However, Cygwin's default installation provides /bin/mkshortcut for
creating desktop shortcuts. This is compatible with exec under Cygwin's
Tcl, understands Cygwin's unix pathnames, and avoids the need for shell
escapes to encode troublesome paths. So, teach git-gui to use mkshortcut
on Cygwin, leaving win32_create_lnk unchanged and for exclusive use by
Git For Windows.
Notes: "CHERE_INVOKING=1" is recognized by Cygwin's /etc/profile and
prevents a "chdir $HOME", leaving the shell in the working directory
specified by the shortcut. That directory is written directly by
mkshortcut eliminating any problems with shell escapes and quoting.
The code being replaced includes the full pathname of the git-gui
creating the shortcut, but that git-gui might not be compatible with the
git found after /etc/profile sets the path, and might have a pathname
that defies encoding using shell escapes that can survive the multiple
incompatible interpreters involved in the chain of creating and using
this shortcut. The new code uses bare "git gui" as the command to
execute, thus using the system git to launch the system git-gui, and
avoiding both issues.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
Mark Levedahl [Mon, 26 Jun 2023 16:53:04 +0000 (12:53 -0400)]
git-gui - use cygstart to browse on Cygwin
git-gui enables the "Repository->Explore Working Copy" menu on Cygwin,
offering to open a Windows graphical file browser at the root of the
working directory. This code, shared with Git For Windows support,
depends upon use of Windows pathnames. However, git gui on Cygwin uses
unix pathnames, so this shared code will not work on Cygwin.
A base install of Cygwin provides the /bin/cygstart utility that runs
a registered Windows application based upon the file type, after
translating unix pathnames to Windows. Adding the --explore option
guarantees that the Windows file explorer is opened, regardless of the
supplied pathname's file type and avoiding possibility of some other
action being taken.
So, teach git-gui to use cygstart --explore on Cygwin, restoring the
pre-2012 behavior of opening a Windows file explorer for browsing. This
separates the Git For Windows and Cygwin code paths. Note that
is_Windows is never true on Cygwin, and is_Cygwin is never true on Git
for Windows, though this is not obvious by examining the code for those
independent functions.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
Mark Levedahl [Mon, 26 Jun 2023 16:53:03 +0000 (12:53 -0400)]
git-gui - remove obsolete Cygwin specific code
In the current git release, git-gui runs on Cygwin without enabling any
of git-gui's Cygwin specific code. This happens as the Cygwin specific
code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's
then supplied Tcl/Tk which was an incompletely ported variant of the
8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was
replaced with a full port based upon the upstream unix/X11 code,
since maintained up to date. The two Tcl/Tk packages are completely
incompatible, and have different signatures.
When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
detected Cygwin, so did not enable Cygwin specific code, and the POSIX
environment provided by Cygwin since 2012 supported git-gui as a generic
unix. Thus, no-one apparently noticed the existence of incompatible
Cygwin specific code.
However, since commit c5766eae6f in the git-gui source tree
(https://github.com/prati0100/git-gui, master at a5005ded), and not yet
pulled into the git repository, the is_Cygwin function does detect
Cygwin using the unix/X11 Tcl/Tk. The Cygwin specific code is enabled,
causing use of Windows rather than unix pathnames, and enabling
incorrect warnings about environment variables that were relevant only
to the old Tcl/Tk. The end result is that (upstream) git-gui is now
incompatible with Cygwin.
So, delete Cygwin specific code (code protected by "if is_Cygwin") that
is not needed in any form to work with the unix/X11 Tcl/Tk.
Cygwin specific code required to enable file browsing and shortcut
creation is not addressed in this patch, does not currently work, and
invocation of those items may leave git-gui in a confused state.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
Mark Levedahl [Mon, 26 Jun 2023 16:53:02 +0000 (12:53 -0400)]
git gui Makefile - remove Cygwin modifications
git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries
into git-gui, destroying the ability to package git-gui on one machine and
distribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is
installed, but the test for this is wrong with the unix/X11 Tcl/Tk shipped
since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.
The Cygwin git maintainer disables this code, so this code is definitely
not in use in the Cygwin distribution.
The simplest fix is to just delete the Cygwin specific code,
allowing the Makefile to work out of the box on Cygwin. Do so.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4
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>
Pratyush Yadav [Tue, 24 Jan 2023 13:13:46 +0000 (14:13 +0100)]
Merge branch 'js/windows-rce'
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
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>
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>
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>
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:
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>
This commit causes breakage on macOS (10.13). It causes errors on
startup and completely breaks the commit functionality. There are two
main problems. First, it uses `string cat` which is not supported on
older Tcl versions. Second, it does a half close of the bidirectional
pipe to git-stripspace which is also not supported on older Tcl
versions.
Reported-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
Pratyush Yadav [Tue, 2 Feb 2021 19:58:12 +0000 (01:28 +0530)]
git-gui: remove lines starting with the comment character
The comment character is specified by the config variable
'core.commentchar'. Any lines starting with this character is considered
a comment and should not be included in the final commit message.
Teach git-gui to filter out lines in the commit message that start with
the comment character using git-stripspace. If the config is not set,
'#' is taken as the default. Also add a message educating users about
the comment character.
projects / thirdparty / git.git / log
