Re: [PATCH v2 5/8] hook(clone protections): add escape hatch

From: Jeff King <peff@peff.net>
Date: May 18, 2024, 18:14 UTC
Message-ID: <20240518181432.GA1570600@coredump.intra.peff.net>
In-Reply-To: <b841db8392ebd924d1893829a7e5e22240f1e9cf.1716028366.git.gitgitgadget@gmail.com>

On Sat, May 18, 2024 at 10:32:43AM +0000, Johannes Schindelin via GitGitGadget wrote:

Show 7 quoted lines

> To help Git LFS, and other tools behaving similarly (if there are any),
> let's add a new, multi-valued `safe.hook.sha256` config setting. Like
> the already-existing `safe.*` settings, it is ignored in
> repository-local configs, and it is interpreted as a list of SHA-256
> checksums of hooks' contents that are safe to execute during a clone
> operation. Future Git LFS versions will need to write those entries at
> the same time they install the `smudge`/`clean` filters.

This scheme seems more complicated for the user than the sometimes discussed ability to specify hook paths via config (not core.hooksPath, which covers _all_ hooks, but one which allows a per-hook path).

In either case, we're considering config to be a trusted source of truth, so I think the security properties are the same. But for the system here, a user updating a hook needs to do multiple steps:

  - compute the sha256 of the hook (for which we provide no tooling
    support, though hopefully it is obvious how to use other tools)

  - add the config for the sha256

  - install the new hook into $GIT_DIR/hooks

Whereas if the config can just point at the hook, then there is only one step: add the config for the hook (presumably pointing to a system version that would have been copied into $GIT_DIR/hooks previously).

Likewise for updates of the hooks, where the sha256 scheme requires computing and adding a new hash. But when the config just points to the path, there is no additional step for updating.

In either scheme, programs like git-lfs would have to adjust to the new world view. The main advantage of the sha256 scheme, it seems to me, is that the baked-in sha256 values let existing versions of git-lfs work. But we could also support that internally, without exposing safe.hook.sha256 to the world (and thus creating an ecosystem where we have to support it forever).

Implied here is that I also think config-based hooks have a lot of _other_ advantages, and so would be worth pursuing anyway, and this extra safety would come along for free. I won't enumerate those advantages here, but we that can be a separate discussion if need be.

And of course that feature doesn't yet exist, and is a much larger one. But besides un-breaking current LFS, I'm not sure that we need to rush out a more generic version of the feature.

-Peff