Allow -1 as a value for hook.jobs, hook.<event>.jobs, and the -j
CLI flag to mean "use as many jobs as there are CPU cores", matching
the convention used by fetch.parallel and other Git subsystems.
The value is resolved to online_cpus() at parse time so the rest
of the code always works with a positive resolved count.
Other non-positive values (0, -2, etc) are rejected with a warning
(config) or die (CLI).
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a hook.<event>.enabled config key that disables all hooks for
a given event, when set to false, acting as a high-level switch
above the existing per-hook hook.<friendly-name>.enabled.
Event-disabled hooks are shown in "git hook list" with an
"event-disabled" tab-separated prefix before the name:
$ git hook list test-hook
event-disabled hook-1
event-disabled hook-2
With --show-scope:
$ git hook list --show-scope test-hook
local event-disabled hook-1
When a hook is both per-hook disabled and event-disabled, only
"event-disabled" is shown: the event-level switch is the more
relevant piece of information, and the per-hook "disabled" status
will surface once the event is re-enabled.
Using an event name as a friendly-name (e.g. hook.<event>.enabled)
can cause ambiguity, so a fatal error is issued when using a known
event name and a warning is issued for unknown event name, since
a collision cannot be detected with certainty for unknown events.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move is_known_hook() from builtin/hook.c (static) into hook.c and
export it via hook.h so it can be reused.
Make it return bool and the iterator `h` for clarity (iterate hooks).
Both meson.build and the Makefile are updated to reflect that the
header is now used by libgit, not the builtin sources.
The next commit will use this to reject hook friendly-names that
collide with known event names.
Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Issue a warning when the user confuses the hook process and event
namespaces by setting hook.<friendly-name>.jobs.
Detect this by checking whether the name carrying .jobs also has
.command, .event, or .parallel configured. Extract is_friendly_name()
as a helper for this check, to be reused by future per-event config
handling.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a hook.<event>.jobs count config that allows users to override the
global hook.jobs setting for specific hook events.
This allows finer-grained control over parallelism on a per-event basis.
For example, to run `post-receive` hooks with up to 4 parallel jobs
while keeping other events at their global default:
[hook]
post-receive.jobs = 4
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Expose the parallel job count as a command-line flag so callers can
request parallelism without relying only on the hook.jobs config.
Add tests covering serial/parallel execution and TTY behaviour under
-j1 vs -jN.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pre-push is the only hook that keeps stdout and stderr separate (for
backwards compatibility with git-lfs and potentially other users). This
prevents parallelizing it because run-command needs stdout_to_stderr=1
to buffer and de-interleave parallel outputs.
Since we now default to jobs=1, backwards compatibility is maintained
without needing any extension or extra config: when no parallelism is
requested, pre-push behaves exactly as before.
When the user explicitly opts into parallelism via hook.jobs > 1,
hook.<event>.jobs > 1, or -jN, they accept the changed output behavior.
Document this and let get_hook_jobs() set stdout_to_stderr=1 automatically
when jobs > 1, removing the need for any extension infrastructure.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hooks always run in sequential order due to the hardcoded jobs == 1
passed to run_process_parallel(). Remove that hardcoding to allow
users to run hooks in parallel (opt-in).
Users need to decide which hooks to run in parallel, by specifying
"parallel = true" in the config, because Git cannot know if their
specific hooks are safe to run or not in parallel (for e.g. two hooks
might write to the same file or call the same program).
Some hooks are unsafe to run in parallel by design: these will marked
in the next commit using RUN_HOOKS_OPT_INIT_FORCE_SERIAL.
The hook.jobs config specifies the default number of jobs applied to all
hooks which have parallelism enabled.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The hook.jobs config is a global way to set hook parallelization for
all hooks, in the sense that it is not per-event nor per-hook.
Finer-grained configs will be added in later commits which can override
it, for e.g. via a per-event type job options. Next commits will also
add to this item's documentation.
Parse hook.jobs config key in hook_config_lookup_all() and store its
value in hook_all_config_cb.jobs, then transfer it into r->jobs after
the config pass completes.
This is mostly plumbing and the cached value is not yet used.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Disabled hooks were filtered out of the cache entirely, making them
invisible to "git hook list". Keep them in the cache with a new
"disabled" flag which is propagated to the respective struct hook.
"git hook list" now shows disabled hooks as tab-separated columns,
with the status as a prefix before the name (like scope with
--show-scope). With --show-scope it looks like:
$ git hook list --show-scope pre-commit
global linter
local disabled no-leaks
hook from hookdir
A disabled hook without a command issues a warning instead of the
fatal "hook.X.command must be configured" error. We could also throw
an error, however it seemd a bit excessive to me in this case.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Users running "git hook list" can see which hooks are configured but
have no way to tell at which config scope (local, global, system...)
each hook was defined.
Store the scope from ctx->kvi->scope in the single-pass config callback,
then carry it through the cache to the hook structs, so we can expose it
to users via the "git hook list --show-scope" flag, which mirrors the
existing git config --show-scope convention.
Without the flag the output is unchanged.
The scope is printed as a tab-separated prefix (like "git config --show-scope"),
making it unambiguously machine-parseable even when the friendly name
contains spaces.
Example usage:
$ git hook list --show-scope pre-commit
global linter
local no-leaks
hook from hookdir
Traditional hooks from the hookdir are unaffected by --show-scope since
the config scope concept does not apply to them.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace the bare `char *command` util pointer stored in each string_list
item with a heap-allocated `struct hook_config_cache_entry` that carries
that command string.
This is just a refactoring with no behavior changes, to give the cache
entry room to grow, so it can carry the additional hook metadata we'll
be adding in the following commits.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Both `name` and `friendly-name` is being used. Standardize on
`friendly-name` for consistency since name is rather generic,
even when used in the hooks namespace.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace the custom function with string_list_clear_func() which
is a more common pattern for clearing a string_list.
To be able to do this, rework hook_clear() into hook_free(), so
it can be passed to string_list_clear_func().
A slight complication is the need to keep a copy of the internal
cb data free() pointer, however I think it's worth it since the
API becomes cleaner, e.g. no more calls with NULL function args
like hook_list_clear(hooks, NULL).
In other words, the callers don't need to keep track of hook
internal state to determine when cleanup is necessary or not
(pass NULL) because each `struct hook` now owns its data_free
callback.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Trigger a bug when an unknown hook type is encountered while
setting up hook execution.
Also issue a bug if a configured hook is enabled without a cmd.
Mostly useful for defensive coding.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rename the hook callback function types to use the hook prefix.
This is a style fix with no logic changes.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix some minor style nits pointed out by Patrick, Junio and Eric:
* Use CALLOC_ARRAY instead of xcalloc.
* Init struct members during declaration.
* Simplify if condition boolean logic.
* Missing curly braces in if/else stmts.
* Unnecessary header includes.
* Capitalization and full-stop in error/warn messages.
* Curly brace on separate line when defining struct.
* Comment spelling: free'd -> freed.
* Sort the included headers.
* Blank line fixes to improve readability.
These contain no logic changes, the code behaves the same as before.
Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the convenience wrapper from hook to string-list since
it's a more suitable place. Add a doc comment to the header.
Also add a free_util arg to make the function more generic
and make the API similar to other functions in string-list.h.
Update the existing call-sites.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since hooks can now be supplied via the config, and a config can be
present without a gitdir via the global and system configs, we can start
to allow 'git hook run' to occur without a gitdir. This enables us to do
things like run sendemail-validate hooks when running 'git send-email'
from a nongit directory.
It still doesn't make sense to look for hooks in the hookdir in nongit
repos, though, as there is no hookdir.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the ability for empty events to clear previously set multivalue
variables, so the newly added "hook.*.event" behave like the other
multivalued keys.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hooks specified via configs are always enabled, however users
might want to disable them without removing from the config,
like locally disabling a global hook.
Add a hook.<name>.enabled config which defaults to true and
can be optionally set for each configured hook.
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the hook.[hc] library to parse configs to populate the list of
hooks to run for a given event.
Multiple commands can be specified for a given hook by providing
"hook.<friendly-name>.command = <path-to-hook>" and
"hook.<friendly-name>.event = <hook-event>" lines.
Hooks will be started in config order of the "hook.<name>.event"
lines and will be run sequentially (.jobs == 1) like before.
Running the hooks in parallel will be enabled in a future patch.
The "traditional" hook from the hookdir is run last, if present.
A strmap cache is added to struct repository to avoid re-reading
the configs on each rook run. This is useful for hooks like the
ref-transaction which gets executed multiple times per process.
Examples:
$ git config --get-regexp "^hook\."
hook.bar.command=~/bar.sh
hook.bar.event=pre-commit
# Will run ~/bar.sh, then .git/hooks/pre-commit
$ git hook run pre-commit
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit introduced an ability to run multiple commands for
hook events and next commit will introduce the ability to define hooks
from configs, in addition to the "traditional" hooks from the hookdir.
Introduce a new command "git hook list" to make inspecting hooks easier
both for users and for the tests we will add.
Further commits will expand on this, e.g. by adding a -z output mode.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hooks are limited to run one command (the default from the hookdir) for
each event. This limitation makes it impossible to run multiple commands
via config files, which the next commits will add.
Implement the ability to run a list of hooks in hook.[ch]. For now, the
list contains only one entry representing the "default" hook from the
hookdir, so there is no user-visible change in this commit.
All hook commands still run sequentially like before. A separate patch
series will enable running them in parallel.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some hooks use opaque structs to keep internal state between callbacks.
Because hooks ran sequentially (jobs == 1) with one command per hook,
these internal states could be allocated on the stack for each hook run.
Next commits add the ability to run multiple commands for each hook, so
the states cannot be shared or stored on the stack anymore, especially
since down the line we will also enable parallel execution (jobs > 1).
Add alloc/free helpers for each hook, doing a "deep" alloc/init & free
of their internal opaque struct.
The alloc callback takes a context pointer, to initialize the struct at
at the time of resource acquisition.
These callbacks must always be provided together: no alloc without free
and no free without alloc, otherwise a BUG() is triggered.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow the API callers to specify the number of jobs across which
hook execution can be parallelized. It defaults to 1 and no hook
currently changes it, so all hooks run sequentially as before.
This allows us to both pave the way for parallel hook execution
(that will be a follow-up patch series building upon this) and to
finish the API conversion of builtin/receive-pack.c, keeping the
output async sideband thread ("muxer") design as Peff suggested.
When .jobs==1 nothing changes, the "copy_to_sideband" async thread
still outputs directly via sideband channel 2, keeping the current
(mostly) real-time output characteristics, avoids unnecessary poll
delays or deadlock risks.
When .jobs > 1, a more complex muxer is needed to buffer the hook
output and avoid interleaving. After working on this mux I quickly
realized I was re-implementing run-command with ungroup=0 so that
idea was dropped in favor of run-command which outputs to stderr.
In other words, run-command itself already can buffer/deinterleave
pp child outputs (ungroup=0), so we can just connect its stderr to
the sideband async task when jobs > 1.
Maybe it helps to illustrate how it works with ascii graphics:
[ Sequential (jobs = 1) ] [ Parallel (jobs > 1) ]
+--------------+ +--------+ +--------+
| Hook Process | | Hook 1 | | Hook 2 |
+--------------+ +--------+ +--------+
| | |
| stderr (inherited) | stderr pipe |
| | (captured) |
v v v
+-------------------------------------------------------------+
| Parent Process |
| |
| (direct write) [run-command (buffered)] |
| | | |
| | | writes |
| v v |
| +-------------------------------------------+ |
| | stderr (FD 2) | |
| +-------------------------------------------+ |
| | |
| | (dup2'd to pipe) |
| v |
| +-----------------------+ |
| | sideband async thread | |
| +-----------------------+ |
+-------------------------------------------------------------+
When use_sideband == 0, the sideband async thread is missing, so
this same architecture just outputs via the parent stderr stream.
See the following commits for the hook API conversions doing this,
using pre-existing sideband thread logic from `copy_to_sideband`.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The hook API assumes that all hooks merge stdout to stderr.
This assumption is proven wrong by pre-push: some of its users
actually expect separate stdout and stderr streams and merging
them will cause a regression.
Therefore this adds a mechanism to allow pre-push to separate
the streams, which will be used in the next commit.
The mechanism is generic via struct run_hooks_opt just in case
there are any more surprise exceptions like this.
Reported-by: Chris Darroch <chrisd@apache.org>
Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This adds a callback mechanism for feeding stdin to hooks alongside
the existing path_to_stdin (which slurps a file's content to stdin).
The advantage of this new callback is that it can feed stdin without
going through the FS layer. This helps when feeding large amount of
data and uses the run-command parallel stdin callback introduced in
the preceding commit.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit f406b89552,
reversing changes made to 1627809eef.
It seems to have caused a few regressions, two of the three known
ones we have proposed solutions for. Let's give ourselves a bit
more room to maneuver during the pre-release freeze period and
restart once the 2.53 ships.
Some server-side hooks will require capturing output to send over
sideband instead of printing directly to stderr. Expose that capability.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When calling run_process_parallel() in run_hooks_opt(), the
ungroup option is currently hardcoded to .ungroup = 1.
This causes problems when ungrouping should be disabled, for
example when sideband-reading collated output from child hooks,
because sideband-reading and ungrouping are mutually exclusive.
Thus a new hook.h option is added to allow overriding.
The existing ungroup=1 behavior is preserved in the run_hooks()
API and the "hook run" command. We could modify these to take
an option if necessary, so I added two code comments there.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This adds a callback mechanism for feeding stdin to hooks alongside
the existing path_to_stdin (which slurps a file's content to stdin).
The advantage of this new callback is that it can feed stdin without
going through the FS layer. This helps when feeding large amount of
data and uses the run-command parallel stdin callback introduced in
the preceding commit.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As explained in an earlier commit, we're refactoring path-related
functions to provide a consistent interface for computing paths into the
commondir, gitdir and worktree. Refactor the "gitdir" family of
functions accordingly.
Note that the `repo_git_pathv()` function is converted into an internal
implementation detail. It is only used to implement `the_repository`
compatibility shims and will eventually be removed from the public
interface.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The advice messages now tell the newer 'git config set' command to
set the advice.token configuration variable to squelch a message.
* bf/explicit-config-set-in-advice-messages:
advice: suggest using subcommand "git config set"
The advice message currently suggests using "git config advice..." to
disable advice messages, but since
00bbdde141 (builtin/config: introduce "set" subcommand, 2024-05-06)
we have the "set" subcommand for config. Since using the subcommand is
more in-line with the modern interface, any advice should be promoting
its usage. Change the disable advice message to use the subcommand
instead. Change all uses of "git config advice" in the tests to use the
subcommand.
Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We implicitly depend on `the_repository` in our hook subsystem because
we use `strbuf_git_path()` to compute hook paths. Remove this dependency
by accepting a `struct repository` as parameter instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* fixes/2.45.1/2.41:
Revert "fsck: warn about symlink pointing inside a gitdir"
Revert "Add a helper function to compare file contents"
clone: drop the protections where hooks aren't run
tests: verify that `clone -c core.hooksPath=/dev/null` works again
Revert "core.hooksPath: add some protection while cloning"
init: use the correct path of the templates directory again
hook: plug a new memory leak
ci: stop installing "gcc-13" for osx-gcc
ci: avoid bare "gcc" for osx-gcc job
ci: drop mention of BREW_INSTALL_PACKAGES variable
send-email: avoid creating more than one Term::ReadLine object
send-email: drop FakeTerm hack
* fixes/2.45.1/2.40:
Revert "fsck: warn about symlink pointing inside a gitdir"
Revert "Add a helper function to compare file contents"
clone: drop the protections where hooks aren't run
tests: verify that `clone -c core.hooksPath=/dev/null` works again
Revert "core.hooksPath: add some protection while cloning"
init: use the correct path of the templates directory again
hook: plug a new memory leak
ci: stop installing "gcc-13" for osx-gcc
ci: avoid bare "gcc" for osx-gcc job
ci: drop mention of BREW_INSTALL_PACKAGES variable
send-email: avoid creating more than one Term::ReadLine object
send-email: drop FakeTerm hack
Revert overly aggressive "layered defence" that went into 2.45.1
and friends, which broke "git-lfs", "git-annex", and other use
cases, so that we can rebuild necessary counterparts in the open.
* jc/fix-2.45.1-and-friends-for-2.39:
Revert "fsck: warn about symlink pointing inside a gitdir"
Revert "Add a helper function to compare file contents"
clone: drop the protections where hooks aren't run
tests: verify that `clone -c core.hooksPath=/dev/null` works again
Revert "core.hooksPath: add some protection while cloning"
init: use the correct path of the templates directory again
hook: plug a new memory leak
ci: stop installing "gcc-13" for osx-gcc
ci: avoid bare "gcc" for osx-gcc job
ci: drop mention of BREW_INSTALL_PACKAGES variable
send-email: avoid creating more than one Term::ReadLine object
send-email: drop FakeTerm hack
As part of the security bug-fix releases v2.39.4, ..., v2.45.1, I
introduced logic to safeguard `git clone` from running hooks that were
installed _during_ the clone operation.
The rationale was that Git's CVE-2024-32002, CVE-2021-21300,
CVE-2019-1354, CVE-2019-1353, CVE-2019-1352, and CVE-2019-1349 should
have been low-severity vulnerabilities but were elevated to
critical/high severity by the attack vector that allows a weakness where
files inside `.git/` can be inadvertently written during a `git clone`
to escalate to a Remote Code Execution attack by virtue of installing a
malicious `post-checkout` hook that Git will then run at the end of the
operation without giving the user a chance to see what code is executed.
Unfortunately, Git LFS uses a similar strategy to install its own
`post-checkout` hook during a `git clone`; In fact, Git LFS is
installing four separate hooks while running the `smudge` filter.
While this pattern is probably in want of being improved by introducing
better support in Git for Git LFS and other tools wishing to register
hooks to be run at various stages of Git's commands, let's undo the
clone protections to unbreak Git LFS-enabled clones.
This reverts commit 8db1e8743c (clone: prevent hooks from running
during a clone, 2024-03-28).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 8db1e8743c (clone: prevent hooks from running during a clone,
2024-03-28), I introduced an inadvertent memory leak that was
unfortunately not caught before v2.45.1 was released. Here is a fix.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint-2.41: (38 commits)
Git 2.41.1
Git 2.40.2
Git 2.39.4
fsck: warn about symlink pointing inside a gitdir
core.hooksPath: add some protection while cloning
init.templateDir: consider this config setting protected
clone: prevent hooks from running during a clone
Add a helper function to compare file contents
init: refactor the template directory discovery into its own function
find_hook(): refactor the `STRIP_EXTENSION` logic
clone: when symbolic links collide with directories, keep the latter
entry: report more colliding paths
t5510: verify that D/F confusion cannot lead to an RCE
submodule: require the submodule path to contain directories only
clone_submodule: avoid using `access()` on directories
submodules: submodule paths must not contain symlinks
clone: prevent clashing git dirs when cloning submodule in parallel
t7423: add tests for symlinked submodule directories
has_dir_name(): do not get confused by characters < '/'
docs: document security issues around untrusted .git dirs
...
* maint-2.40: (39 commits)
Git 2.40.2
Git 2.39.4
fsck: warn about symlink pointing inside a gitdir
core.hooksPath: add some protection while cloning
init.templateDir: consider this config setting protected
clone: prevent hooks from running during a clone
Add a helper function to compare file contents
init: refactor the template directory discovery into its own function
find_hook(): refactor the `STRIP_EXTENSION` logic
clone: when symbolic links collide with directories, keep the latter
entry: report more colliding paths
t5510: verify that D/F confusion cannot lead to an RCE
submodule: require the submodule path to contain directories only
clone_submodule: avoid using `access()` on directories
submodules: submodule paths must not contain symlinks
clone: prevent clashing git dirs when cloning submodule in parallel
t7423: add tests for symlinked submodule directories
has_dir_name(): do not get confused by characters < '/'
docs: document security issues around untrusted .git dirs
upload-pack: disable lazy-fetching by default
...
* maint-2.39: (38 commits)
Git 2.39.4
fsck: warn about symlink pointing inside a gitdir
core.hooksPath: add some protection while cloning
init.templateDir: consider this config setting protected
clone: prevent hooks from running during a clone
Add a helper function to compare file contents
init: refactor the template directory discovery into its own function
find_hook(): refactor the `STRIP_EXTENSION` logic
clone: when symbolic links collide with directories, keep the latter
entry: report more colliding paths
t5510: verify that D/F confusion cannot lead to an RCE
submodule: require the submodule path to contain directories only
clone_submodule: avoid using `access()` on directories
submodules: submodule paths must not contain symlinks
clone: prevent clashing git dirs when cloning submodule in parallel
t7423: add tests for symlinked submodule directories
has_dir_name(): do not get confused by characters < '/'
docs: document security issues around untrusted .git dirs
upload-pack: disable lazy-fetching by default
fetch/clone: detect dubious ownership of local repositories
...
Critical security issues typically combine relatively common
vulnerabilities such as case confusion in file paths with other
weaknesses in order to raise the severity of the attack.
One such weakness that has haunted the Git project in many a
submodule-related CVE is that any hooks that are found are executed
during a clone operation. Examples are the `post-checkout` and
`fsmonitor` hooks.
However, Git's design calls for hooks to be disabled by default, as only
disabled example hooks are copied over from the templates in
`<prefix>/share/git-core/templates/`.
As a defense-in-depth measure, let's prevent those hooks from running.
Obviously, administrators can choose to drop enabled hooks into the
template directory, though, _and_ it is also possible to override
`core.hooksPath`, in which case the new check needs to be disabled.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When looking for a hook and not finding one, and when `STRIP_EXTENSION`
is available (read: if we're on Windows and `.exe` is the required
extension for executable programs), we want to look also for a hook with
that extension.
Previously, we added that handling into the conditional block that was
meant to handle when no hook was found (possibly providing some advice
for the user's benefit). If the hook with that file extension was found,
we'd return early from that function instead of writing out said advice,
of course.
However, we're about to introduce a safety valve to prevent hooks from
being run during a clone, to reduce the attack surface of bugs that
allow writing files to be written into arbitrary locations.
To prepare for that, refactor the logic to avoid the early return, by
separating the `STRIP_EXTENSION` handling from the conditional block
handling the case when no hook was found.
This commit is best viewed with `--patience`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Move abspath-related functions from strbuf.[ch] to abspath.[ch] so that
strbuf is focused on string manipulation routines with minimal
dependencies.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
hash.h depends upon and includes repository.h, due to the definition and
use of the_hash_algo (defined as the_repository->hash_algo). However,
most headers trying to include hash.h are only interested in the layout
of the structs like object_id. Move the parts of hash.h that do not
depend upon repository.h into a new file hash-ll.h (the "low level"
parts of hash.h), and adjust other files to use this new header where
the convenience inline functions aren't needed.
This allows hash.h and object.h to be fairly small, minimal headers. It
also exposes a lot of hidden dependencies on both path.h (which was
brought in by repository.h) and repository.h (which was previously
implicitly brought in by object.h), so also adjust other files to be
more explicit about what they depend upon.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
More work towards -Wunused.
* jk/unused-post-2.39-part2: (21 commits)
help: mark unused parameter in git_unknown_cmd_config()
run_processes_parallel: mark unused callback parameters
userformat_want_item(): mark unused parameter
for_each_commit_graft(): mark unused callback parameter
rewrite_parents(): mark unused callback parameter
fetch-pack: mark unused parameter in callback function
notes: mark unused callback parameters
prio-queue: mark unused parameters in comparison functions
for_each_object: mark unused callback parameters
list-objects: mark unused callback parameters
mark unused parameters in signal handlers
run-command: mark error routine parameters as unused
mark "pointless" data pointers in callbacks
ref-filter: mark unused callback parameters
http-backend: mark unused parameters in virtual functions
http-backend: mark argc/argv unused
object-name: mark unused parameters in disambiguate callbacks
serve: mark unused parameters in virtual functions
serve: use repository pointer to get config
ls-refs: drop config caching
...
Our parallel process API takes several callbacks via function pointers
in the run_process_paralell_opts struct. Not every callback needs every
parameter; let's mark the unused ones to make -Wunused-parameter happy.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
