hook: reject unknown hook names in git-hook(1)

Teach "git hook run" and "git hook list" to reject hook event names
that are not recognized by Git. This helps catch typos such as
"prereceive" when "pre-receive" was intended, since in 99% of the
cases users want known (already-existing) hook names.

The list of known hooks is derived from the generated hook-list.h
(built from Documentation/githooks.adoc). This is why the Makefile
is updated, so builtin/hook.c depends on hook-list.h. In meson the
header is already a dependency for all builtins, no change required.

The "--allow-unknown-hook-name" flag can be used to bypass this check.

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>

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index e7d399ae57af54a21fbec2f0a8b3ba14578a0ec6..318c637bd8eba5a57b4019e02025ddb50d17ae10 100644 (file)
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -8,8 +8,8 @@ git-hook - Run git hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
-'git hook' list [-z] [--show-scope] <hook-name>
+'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
+'git hook' list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
 -----------
@@ -121,6 +121,13 @@ list [-z] [--show-scope]::
 OPTIONS
 -------
 
+--allow-unknown-hook-name::
+       By default `git hook run` and `git hook list` will bail out when
+       `<hook-name>` is not a hook event known to Git (see linkgit:githooks[5]
+       for the list of known hooks). This is meant to help catch typos
+       such as `prereceive` when `pre-receive` was intended. Pass this
+       flag to allow unknown hook names.
+
 --to-stdin::
        For "run"; specify a file which will be streamed into the
        hook's stdin. The hook will receive the entire file from
@@ -159,7 +166,7 @@ Then, in your 'mywrapper' tool, you can invoke any users' configured hooks by
 running:
 
 ----
-git hook run mywrapper-start-tests \
+git hook run --allow-unknown-hook-name mywrapper-start-tests \
   # providing something to stdin
   --stdin some-tempfile-123 \
   # execute hooks in serial

diff --git a/Makefile b/Makefile
index f3264d0a37cc5067f99f0bf1db8f2f716560dae3..c5a1b549a894a33ff2c81d3333b6ad6b62920112 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -2663,6 +2663,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
+builtin/hook.sp builtin/hook.s builtin/hook.o: hook-list.h
 
 builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \

diff --git a/builtin/hook.c b/builtin/hook.c
index f671e7f91a033b87f9484a76745190c2410086a9..c0585587e5e4fafeb377d8815dedf6bd2067b55f 100644 (file)
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -4,12 +4,22 @@
 #include "environment.h"
 #include "gettext.h"
 #include "hook.h"
+#include "hook-list.h"
 #include "parse-options.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
-       N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
+       N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
-       N_("git hook list [-z] [--show-scope] <hook-name>")
+       N_("git hook list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>")
+
+static int is_known_hook(const char *name)
+{
+       const char **p;
+       for (p = hook_name_list; *p; p++)
+               if (!strcmp(*p, name))
+                       return 1;
+       return 0;
+}
 
 static const char * const builtin_hook_usage[] = {
        BUILTIN_HOOK_RUN_USAGE,
@@ -34,6 +44,7 @@ static int list(int argc, const char **argv, const char *prefix,
        const char *hookname = NULL;
        int line_terminator = '\n';
        int show_scope = 0;
+       int allow_unknown = 0;
        int ret = 0;
 
        struct option list_options[] = {
@@ -41,6 +52,8 @@ static int list(int argc, const char **argv, const char *prefix,
                            N_("use NUL as line terminator"), '\0'),
                OPT_BOOL(0, "show-scope", &show_scope,
                         N_("show the config scope that defined each hook")),
+               OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown,
+                        N_("allow running a hook with a non-native hook name")),
                OPT_END(),
        };
 
@@ -57,6 +70,13 @@ static int list(int argc, const char **argv, const char *prefix,
 
        hookname = argv[0];
 
+       if (!allow_unknown && !is_known_hook(hookname)) {
+               error(_("unknown hook event '%s';\n"
+                       "use --allow-unknown-hook-name to allow non-native hook names"),
+                     hookname);
+               return 1;
+       }
+
        head = list_hooks(repo, hookname, NULL);
 
        if (!head->nr) {
@@ -103,8 +123,11 @@ static int run(int argc, const char **argv, const char *prefix,
        int i;
        struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
        int ignore_missing = 0;
+       int allow_unknown = 0;
        const char *hook_name;
        struct option run_options[] = {
+               OPT_BOOL(0, "allow-unknown-hook-name", &allow_unknown,
+                        N_("allow running a hook with a non-native hook name")),
                OPT_BOOL(0, "ignore-missing", &ignore_missing,
                         N_("silently ignore missing requested <hook-name>")),
                OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
@@ -136,6 +159,14 @@ static int run(int argc, const char **argv, const char *prefix,
        repo_config(the_repository, git_default_config, NULL);
 
        hook_name = argv[0];
+
+       if (!allow_unknown && !is_known_hook(hook_name)) {
+               error(_("unknown hook event '%s';\n"
+                       "use --allow-unknown-hook-name to allow non-native hook names"),
+                     hook_name);
+               return 1;
+       }
+
        if (!ignore_missing)
                opt.error_if_missing = 1;
        ret = run_hooks_opt(the_repository, hook_name, &opt);

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 8c5237449dc416e4a9d8f8d7514fd19be67f5a8f..96749fc06d722396d43b0e5d358415ff7844315c 100755 (executable)
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -31,11 +31,41 @@ test_expect_success 'git hook usage' '
        grep "unknown option" err
 '
 
+test_expect_success 'git hook list: unknown hook name is rejected' '
+       test_must_fail git hook list prereceive 2>err &&
+       test_grep "unknown hook event" err
+'
+
+test_expect_success 'git hook run: unknown hook name is rejected' '
+       test_must_fail git hook run prereceive 2>err &&
+       test_grep "unknown hook event" err
+'
+
+test_expect_success 'git hook list: known hook name is accepted' '
+       test_must_fail git hook list pre-receive 2>err &&
+       test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook run: known hook name is accepted' '
+       git hook run --ignore-missing pre-receive 2>err &&
+       test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook run: --allow-unknown-hook-name overrides rejection' '
+       git hook run --allow-unknown-hook-name --ignore-missing custom-hook 2>err &&
+       test_grep ! "unknown hook event" err
+'
+
+test_expect_success 'git hook list: --allow-unknown-hook-name overrides rejection' '
+       test_must_fail git hook list --allow-unknown-hook-name custom-hook 2>err &&
+       test_grep ! "unknown hook event" err
+'
+
 test_expect_success 'git hook list: nonexistent hook' '
        cat >stderr.expect <<-\EOF &&
        warning: no hooks found for event '\''test-hook'\''
        EOF
-       test_expect_code 1 git hook list test-hook 2>stderr.actual &&
+       test_expect_code 1 git hook list --allow-unknown-hook-name test-hook 2>stderr.actual &&
        test_cmp stderr.expect stderr.actual
 '
 
@@ -47,7 +77,7 @@ test_expect_success 'git hook list: traditional hook from hookdir' '
        cat >expect <<-\EOF &&
        hook from hookdir
        EOF
-       git hook list test-hook >actual &&
+       git hook list --allow-unknown-hook-name test-hook >actual &&
        test_cmp expect actual
 '
 
@@ -56,7 +86,7 @@ test_expect_success 'git hook list: configured hook' '
        test_config hook.myhook.event test-hook --add &&
 
        echo "myhook" >expect &&
-       git hook list test-hook >actual &&
+       git hook list --allow-unknown-hook-name test-hook >actual &&
        test_cmp expect actual
 '
 
@@ -68,7 +98,7 @@ test_expect_success 'git hook list: -z shows NUL-terminated output' '
        test_config hook.myhook.event test-hook --add &&
 
        printf "myhookQhook from hookdirQ" >expect &&
-       git hook list -z test-hook >actual.raw &&
+       git hook list --allow-unknown-hook-name -z test-hook >actual.raw &&
        nul_to_q <actual.raw >actual &&
        test_cmp expect actual
 '
@@ -77,12 +107,12 @@ test_expect_success 'git hook run: nonexistent hook' '
        cat >stderr.expect <<-\EOF &&
        error: cannot find a hook named test-hook
        EOF
-       test_expect_code 1 git hook run test-hook 2>stderr.actual &&
+       test_expect_code 1 git hook run --allow-unknown-hook-name test-hook 2>stderr.actual &&
        test_cmp stderr.expect stderr.actual
 '
 
 test_expect_success 'git hook run: nonexistent hook with --ignore-missing' '
-       git hook run --ignore-missing does-not-exist 2>stderr.actual &&
+       git hook run --allow-unknown-hook-name --ignore-missing does-not-exist 2>stderr.actual &&
        test_must_be_empty stderr.actual
 '
 
@@ -94,7 +124,7 @@ test_expect_success 'git hook run: basic' '
        cat >expect <<-\EOF &&
        Test hook
        EOF
-       git hook run test-hook 2>actual &&
+       git hook run --allow-unknown-hook-name test-hook 2>actual &&
        test_cmp expect actual
 '
 
@@ -108,7 +138,7 @@ test_expect_success 'git hook run: stdout and stderr both write to our stderr' '
        Will end up on stderr
        Will end up on stderr
        EOF
-       git hook run test-hook >stdout.actual 2>stderr.actual &&
+       git hook run --allow-unknown-hook-name test-hook >stdout.actual 2>stderr.actual &&
        test_cmp stderr.expect stderr.actual &&
        test_must_be_empty stdout.actual
 '
@@ -120,12 +150,12 @@ do
                exit $code
                EOF
 
-               test_expect_code $code git hook run test-hook
+               test_expect_code $code git hook run --allow-unknown-hook-name test-hook
        '
 done
 
 test_expect_success 'git hook run arg u ments without -- is not allowed' '
-       test_expect_code 129 git hook run test-hook arg u ments
+       test_expect_code 129 git hook run --allow-unknown-hook-name test-hook arg u ments
 '
 
 test_expect_success 'git hook run -- pass arguments' '
@@ -139,7 +169,7 @@ test_expect_success 'git hook run -- pass arguments' '
        u ments
        EOF
 
-       git hook run test-hook -- arg "u ments" 2>actual &&
+       git hook run --allow-unknown-hook-name test-hook -- arg "u ments" 2>actual &&
        test_cmp expect actual
 '
 
@@ -148,12 +178,12 @@ test_expect_success 'git hook run: out-of-repo runs execute global hooks' '
        test_config_global hook.global-hook.command "echo no repo no problems" --add &&
 
        echo "global-hook" >expect &&
-       nongit git hook list test-hook >actual &&
+       nongit git hook list --allow-unknown-hook-name test-hook >actual &&
        test_cmp expect actual &&
 
        echo "no repo no problems" >expect &&
 
-       nongit git hook run test-hook 2>actual &&
+       nongit git hook run --allow-unknown-hook-name test-hook 2>actual &&
        test_cmp expect actual
 '
 
@@ -178,11 +208,11 @@ test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
        # Test various ways of specifying the path. See also
        # t1350-config-hooks-path.sh
        >actual &&
-       git hook run test-hook -- ignored 2>>actual &&
-       git -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&
-       git -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&
-       git -c core.hooksPath="$PWD/my-hooks" hook run test-hook -- three 2>>actual &&
-       git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook -- four 2>>actual &&
+       git hook run --allow-unknown-hook-name test-hook -- ignored 2>>actual &&
+       git -c core.hooksPath=my-hooks hook run --allow-unknown-hook-name test-hook -- one 2>>actual &&
+       git -c core.hooksPath=my-hooks/ hook run --allow-unknown-hook-name test-hook -- two 2>>actual &&
+       git -c core.hooksPath="$PWD/my-hooks" hook run --allow-unknown-hook-name test-hook -- three 2>>actual &&
+       git -c core.hooksPath="$PWD/my-hooks/" hook run --allow-unknown-hook-name test-hook -- four 2>>actual &&
        test_cmp expect actual
 '
 
@@ -262,7 +292,7 @@ test_expect_success 'hook can be configured for multiple events' '
        # 'ghi' should be included in both 'pre-commit' and 'test-hook'
        git hook list pre-commit >actual &&
        grep "ghi" actual &&
-       git hook list test-hook >actual &&
+       git hook list --allow-unknown-hook-name test-hook >actual &&
        grep "ghi" actual
 '
 
@@ -336,15 +366,15 @@ test_expect_success 'stdin to multiple hooks' '
        b3
        EOF
 
-       git hook run --to-stdin=input test-hook 2>actual &&
+       git hook run --allow-unknown-hook-name --to-stdin=input test-hook 2>actual &&
        test_cmp expected actual
 '
 
 test_expect_success 'rejects hooks with no commands configured' '
        test_config hook.broken.event "test-hook" &&
-       test_must_fail git hook list test-hook 2>actual &&
+       test_must_fail git hook list --allow-unknown-hook-name test-hook 2>actual &&
        test_grep "hook.broken.command" actual &&
-       test_must_fail git hook run test-hook 2>actual &&
+       test_must_fail git hook run --allow-unknown-hook-name test-hook 2>actual &&
        test_grep "hook.broken.command" actual
 '
 
@@ -353,7 +383,7 @@ test_expect_success 'disabled hook is not run' '
        test_config hook.skipped.command "echo \"Should not run\"" &&
        test_config hook.skipped.enabled false &&
 
-       git hook run --ignore-missing test-hook 2>actual &&
+       git hook run --allow-unknown-hook-name --ignore-missing test-hook 2>actual &&
        test_must_be_empty actual
 '
 
@@ -403,7 +433,7 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
        test_config hook.global-hook.enabled true &&
 
        echo "global-hook ran" >expected &&
-       git hook run test-hook 2>actual &&
+       git hook run --allow-unknown-hook-name test-hook 2>actual &&
        test_cmp expected actual
 '
 
@@ -463,7 +493,7 @@ test_expect_success 'git hook run a hook with a bad shebang' '
 
        test_expect_code 1 git \
                -c core.hooksPath=bad-hooks \
-               hook run test-hook >out 2>err &&
+               hook run --allow-unknown-hook-name test-hook >out 2>err &&
        test_must_be_empty out &&
 
        # TODO: We should emit the same (or at least a more similar)
@@ -487,7 +517,7 @@ test_expect_success 'stdin to hooks' '
        EOF
 
        echo hello >input &&
-       git hook run --to-stdin=input test-hook 2>actual &&
+       git hook run --allow-unknown-hook-name --to-stdin=input test-hook 2>actual &&
        test_cmp expect actual
 '