diff --git a/README.md b/README.md index 90733b8..a423a29 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,13 @@ mix git_hooks.install To disable the automatic install of the git hooks set the configuration key `auto_install` to `false`. +### External hooks path + +If `core.hooksPath` resolves to a directory outside the repository (for example, a global hooks +directory), `git_hooks` will refuse to install by default to avoid overwriting shared hooks. +To override this behavior, set `allow_external_hooks_path: true` or export +`GIT_HOOKS_ALLOW_EXTERNAL=1`. + ### Hook configuration One or more git hooks can be configured, those hooks will be the ones @@ -130,6 +137,9 @@ Setting a custom _git hooks_ config path is also supported: git config core.hooksPath .myCustomGithooks/ ``` +If the configured hooks path points outside the repository, set +`allow_external_hooks_path: true` (or `GIT_HOOKS_ALLOW_EXTERNAL=1`) to allow installation. + ### Custom project path This library assumes a simple Elixir project architecture. This is, an Elixir diff --git a/lib/config.ex b/lib/config.ex index 5b60b00..62824be 100644 --- a/lib/config.ex +++ b/lib/config.ex @@ -75,6 +75,14 @@ defmodule GitHooks.Config do Application.get_env(:git_hooks, :extra_success_returns, []) end + @doc """ + Returns whether installs are allowed when the resolved hooks path is outside the repo. + """ + @spec allow_external_hooks_path?() :: boolean + def allow_external_hooks_path? do + Application.get_env(:git_hooks, :allow_external_hooks_path, false) + end + defdelegate tasks(git_hook_type), to: TasksConfig defdelegate verbose?, to: VerboseConfig diff --git a/lib/git_hooks.ex b/lib/git_hooks.ex index 451edf1..c338b84 100644 --- a/lib/git_hooks.ex +++ b/lib/git_hooks.ex @@ -58,11 +58,11 @@ defmodule GitHooks do end def new_task({_module, _function, _arity} = mfa, git_hook_type, git_hook_args) do - MFA.new(mfa, git_hook_type, git_hook_args) + MFA.from_config(mfa, git_hook_type, git_hook_args) end def new_task({_module, _function} = mfa, git_hook_type, git_hook_args) do - MFA.new(mfa, git_hook_type, git_hook_args) + MFA.from_config(mfa, git_hook_type, git_hook_args) end def new_task(task_config, git_hook_type, _git_hook_args) do diff --git a/lib/mix/tasks/git_hooks/install.ex b/lib/mix/tasks/git_hooks/install.ex index 4152cfc..343717b 100644 --- a/lib/mix/tasks/git_hooks/install.ex +++ b/lib/mix/tasks/git_hooks/install.ex @@ -46,56 +46,71 @@ defmodule Mix.Tasks.GitHooks.Install do mix_path = Config.mix_path() project_path = Application.get_env(:git_hooks, :project_path, GitPath.resolve_app_path()) - - ensure_hooks_folder_exists() - clean_missing_hooks() - track_configured_hooks() - - git_hooks_configs = Config.git_hooks() - - install_result = - Enum.map(git_hooks_configs, fn git_hook -> - git_hook_atom_as_string = Atom.to_string(git_hook) - git_hook_atom_as_kebab_string = Recase.to_kebab(git_hook_atom_as_string) - - case File.read(template_file) do - {:ok, body} -> - target_file_path = GitPath.git_hooks_path_for(git_hook_atom_as_kebab_string) - - unless opts[:quiet] || !Config.verbose?() do - Printer.info("Installed git hook: #{target_file_path}") - end - - target_file_body = - body - |> String.replace("$git_hook", git_hook_atom_as_string) - |> String.replace("$mix_path", mix_path) - |> String.replace("$project_path", project_path) - - unless opts[:quiet] || !Config.verbose?() do - Printer.info( - "Writing git hook for `#{git_hook_atom_as_string}` to `#{target_file_path}`" - ) - end - - backup_current_hook(git_hook_atom_as_kebab_string, opts) - - if opts[:dry_run] do - {git_hook, target_file_body} - else - File.write(target_file_path, target_file_body) - File.chmod(target_file_path, 0o755) - end - - {:error, reason} -> - reason |> inspect() |> Printer.error() - end - end) - - if opts[:dry_run] do - install_result + project_path_abs = Path.expand(project_path) + hooks_path = GitPath.resolve_git_hooks_path() |> Path.expand() + + if !hooks_path_within_repo?(hooks_path, project_path_abs) && !allow_external_hooks_path?() do + Printer.warn(""" + Refusing to install git hooks outside the repository. + Set a repo-local core.hooksPath or configure allow_external_hooks_path: true (or GIT_HOOKS_ALLOW_EXTERNAL=1) to override. + """) + + if opts[:dry_run] do + [] + else + :ok + end else - :ok + ensure_hooks_folder_exists() + clean_missing_hooks() + track_configured_hooks() + + git_hooks_configs = Config.git_hooks() + + install_result = + Enum.map(git_hooks_configs, fn git_hook -> + git_hook_atom_as_string = Atom.to_string(git_hook) + git_hook_atom_as_kebab_string = Recase.to_kebab(git_hook_atom_as_string) + + case File.read(template_file) do + {:ok, body} -> + target_file_path = GitPath.git_hooks_path_for(git_hook_atom_as_kebab_string) + + unless opts[:quiet] || !Config.verbose?() do + Printer.info("Installed git hook: #{target_file_path}") + end + + target_file_body = + body + |> String.replace("$git_hook", git_hook_atom_as_string) + |> String.replace("$mix_path", mix_path) + |> String.replace("$project_path", project_path) + + unless opts[:quiet] || !Config.verbose?() do + Printer.info( + "Writing git hook for `#{git_hook_atom_as_string}` to `#{target_file_path}`" + ) + end + + backup_current_hook(git_hook_atom_as_kebab_string, opts) + + if opts[:dry_run] do + {git_hook, target_file_body} + else + File.write(target_file_path, target_file_body) + File.chmod(target_file_path, 0o755) + end + + {:error, reason} -> + reason |> inspect() |> Printer.error() + end + end) + + if opts[:dry_run] do + install_result + else + :ok + end end end @@ -185,4 +200,20 @@ defmodule Mix.Tasks.GitHooks.Install do {:error, reason} -> Printer.warn("Cannot restore backup: #{inspect(reason)}") end end + + defp hooks_path_within_repo?(hooks_path, project_path) do + relative = Path.relative_to(hooks_path, project_path) + relative != hooks_path && !String.starts_with?(relative, "..") + end + + defp allow_external_hooks_path? do + Config.allow_external_hooks_path?() || truthy_env?("GIT_HOOKS_ALLOW_EXTERNAL") + end + + defp truthy_env?(key) do + case System.get_env(key) do + nil -> false + value -> String.downcase(value) in ["1", "true", "yes"] + end + end end diff --git a/lib/tasks/mfa.ex b/lib/tasks/mfa.ex index 7410db8..8c03574 100644 --- a/lib/tasks/mfa.ex +++ b/lib/tasks/mfa.ex @@ -34,18 +34,21 @@ defmodule GitHooks.Tasks.MFA do defstruct [:module, :function, args: [], result: nil] @doc """ - Creates a new `mfa` struct. + Creates a new `mfa` struct from config. ### Examples - iex> #{__MODULE__}.new({MyModule, :my_function}, :pre_commit, ["commit message"]) + iex> #{__MODULE__}.from_config({MyModule, :my_function}, :pre_commit, ["commit message"]) %#{__MODULE__}{module: MyModule, function: :my_function, args: ["commit message"]} """ - @spec new(mfa() | {module(), atom()}, GitHooks.git_hook_type(), GitHooks.git_hook_args()) :: + @spec from_config( + mfa() | {module(), atom()}, + GitHooks.git_hook_type(), + GitHooks.git_hook_args() + ) :: __MODULE__.t() - @deprecated "Use mfa without arity, all functions are expected to have arity 1 and receive a list with the git hook args" - def new({module, function, _arity}, _git_hook_type, git_hook_args) do + def from_config({module, function, _arity}, _git_hook_type, git_hook_args) do %__MODULE__{ module: module, function: function, @@ -53,13 +56,20 @@ defmodule GitHooks.Tasks.MFA do } end - def new({module, function}, _git_hook_type, git_hook_args) do + def from_config({module, function}, _git_hook_type, git_hook_args) do %__MODULE__{ module: module, function: function, args: git_hook_args } end + + @deprecated "Use mfa without arity, all functions are expected to have arity 1 and receive a list with the git hook args" + @spec new(mfa() | {module(), atom()}, GitHooks.git_hook_type(), GitHooks.git_hook_args()) :: + __MODULE__.t() + def new(mfa, git_hook_type, git_hook_args) do + from_config(mfa, git_hook_type, git_hook_args) + end end defimpl GitHooks.Task, for: GitHooks.Tasks.MFA do diff --git a/test/mix/tasks/git_hooks/install_test.exs b/test/mix/tasks/git_hooks/install_test.exs index 34cfa2d..39a91b2 100644 --- a/test/mix/tasks/git_hooks/install_test.exs +++ b/test/mix/tasks/git_hooks/install_test.exs @@ -5,6 +5,8 @@ defmodule Mix.Tasks.InstallTest do use GitHooks.TestSupport.ConfigCase use GitHooks.TestSupport.GitProjectCase + import ExUnit.CaptureIO + alias Mix.Tasks.GitHooks.Install @tag capture_log: true @@ -32,7 +34,7 @@ defmodule Mix.Tasks.InstallTest do custom_path = Path.join(project_path, "a_custom_path") File.mkdir_p!(custom_path) - System.cmd("git", ["init"], cd: custom_path) + System.cmd("git", ["-c", "init.defaultBranch=master", "init", "--quiet"], cd: custom_path) Application.put_env(:git_hooks, :project_path, custom_path) hooks_file = Install.run(["--dry-run", "--quiet"]) @@ -86,6 +88,49 @@ defmodule Mix.Tasks.InstallTest do ] end) end + + test "refuses to install when hooks path is outside the repo", %{tmp_dir: project_path} do + put_git_hook_config( + [:pre_commit, :pre_push], + tasks: {:cmd, "check"} + ) + + external_hooks_dir = + Path.join(System.tmp_dir!(), "git_hooks_external_#{System.unique_integer([:positive])}") + + File.mkdir_p!(external_hooks_dir) + System.cmd("git", ["config", "core.hooksPath", external_hooks_dir], cd: project_path) + + output = + capture_io(fn -> + assert Install.run(["--dry-run", "--quiet"]) == [] + end) + + assert output =~ "Refusing to install git hooks outside the repository." + end + + test "allows install when hooks path is outside the repo with explicit opt-in", %{ + tmp_dir: project_path + } do + put_git_hook_config( + [:pre_commit, :pre_push], + tasks: {:cmd, "check"} + ) + + external_hooks_dir = + Path.join(System.tmp_dir!(), "git_hooks_external_#{System.unique_integer([:positive])}") + + File.mkdir_p!(external_hooks_dir) + System.cmd("git", ["config", "core.hooksPath", external_hooks_dir], cd: project_path) + Application.put_env(:git_hooks, :allow_external_hooks_path, true) + + hooks_file = Install.run(["--dry-run", "--quiet"]) + + assert hooks_file == [ + pre_commit: expect_hook_template("pre_commit", project_path), + pre_push: expect_hook_template("pre_push", project_path) + ] + end end # diff --git a/test/support/config_case.ex b/test/support/config_case.ex index 03bb9f0..529838b 100644 --- a/test/support/config_case.ex +++ b/test/support/config_case.ex @@ -22,6 +22,7 @@ defmodule GitHooks.TestSupport.ConfigCase do def cleanup_config do Application.delete_env(:git_hooks, :hooks) Application.delete_env(:git_hooks, :verbose) + Application.delete_env(:git_hooks, :allow_external_hooks_path) end @spec put_git_hook_config(list(atom) | atom, keyword) :: :ok diff --git a/test/support/git_project_case.ex b/test/support/git_project_case.ex index 822b891..824e93e 100644 --- a/test/support/git_project_case.ex +++ b/test/support/git_project_case.ex @@ -12,7 +12,7 @@ defmodule GitHooks.TestSupport.GitProjectCase do tmp_dir = Path.join(System.tmp_dir!(), "git_hooks_test_#{:os.system_time(:millisecond)}") File.mkdir_p!(tmp_dir) - System.cmd("git", ["init"], cd: tmp_dir) + System.cmd("git", ["-c", "init.defaultBranch=master", "init", "--quiet"], cd: tmp_dir) Application.put_env(:git_hooks, :project_path, tmp_dir)