On 02/08/2018 10:06 PM, Ævar Arnfjörð Bjarmason wrote:>> Hmm, OK, so I guess I'll try to update the patch when I get some time to >> delve into git's internals, as my use case (forbidding some fetches) >> couldn't afaik be covered by a wrapper hook. > > Per my reading of > https://public-inbox.org/git/20111224234212.GA21533@xxxxxxxxxxxxxxx/ > what Joey implemented is not what you described in your initial mail. > > His is a *post*-fetch hook, we've already done the fetch and are just > telling you as a courtesy what refs changed. You could also implement > this as some cronjob that polls git for-each-ref but it's easier as a > hook, fine. I was thinking along the lines of https://marc.info/?l=git&m=132486687023893&w=2 with high-level description at https://marc.info/?l=git&m=132480559712592&w=2 With the high-level description given here, I'm pretty sure I can hack a hook together to make things work as I want them to. > What you're describing is something like a pre-fetch hook analogous to > the pre-receive hooks, where you're fed refs updated on the remote on > stdin, and can say you don't want some of those to be updated. > > This may just be a lack of imagination on my part, but I don't see how > that's sensible at all. > > The refs we fetch are our *copy* of the remote refs, why would you not > want to track the upstream remote. You're going to refuse some branches > and what? Be further behind until some point in the future where the tip > is GPG-signed and you accept it, at which poich you'll need to do more > work than if you were up-to-date with the almost-GPG-signed version? That's about it. I want all fetching to be blocked in case of the tip not being signed. As there is a pre-push hook ensuring committers don't forget to sign before pushing, the only case the tip could not be signed is in case of an attack, which means it's better to just force-push master because any git repo that fetched it is doomed anyway. Definitely would not want to allow an untrusted revision get into anything that could even remotely be taken as “endorsed” by the user. (BTW, in order to avoid the case of someone forgetting to sign the commit and not having installed the pre-push hook, there can be holes in the commit-signing chain, the drawback being that the committer pushing a signed commit takes responsibility for all unsigned commits directly preceding his -- allowing them to recover in case of a mistaken push) > I think you're confusing two things here. One is the reasonable concern > of wanting to not have your local copy of remote refs have undesirable > content, but a pre-fetch hook is not the way to accomplish that. Well, a pre-fetch hook is a possible way of accomplishing that, and I don't know of any better one? > The other is e.g. to ensure that you never locally check out some "bad" > ref, we don't have hook support for that, but could add it, > e.g. git-checkout and git reset --hard could be taught about some > pre-checkout hook. Issue is, once we have to fix checkout and reset, all other commands that potentially touch the worktree also have to be fixed (eg. I don't know whether worktree add triggers pre-checkout?) Also, this requires the hook to store a database of all the paths that have been checked, because there is no logic in how one may choose to checkout the repo. While having a tweak-fetch hook would make the implementation straightforward, because at the time of invoking the hook the “refname at remote” commit is already trusted, and the “object name” is the commit whose validity we want to check, so we just have to check the path between those two. (I don't know if you checked my current scripts, but basically as the set of allowed PGP keys can change at any commit, it's only possible to check a commit path, not a single commit out-of-nowhere) The only issue that could arise with a tweak-fetch hook is in case of a force-fetch (and even maybe it's not even an actual issue, I haven't given it real thought yet), but this can reasonably be banned, as once a commit is signed it enters the “real” master branch, that should never be moved backward, as it can't be the sign of an attack. > You could also have some intermediate step between these two, where > e.g. your refspec for "origin" is > "+refs/heads/*:refs/remotes/origin-untrusted/*" instead of the default > "+refs/heads/*:refs/remotes/origin/*", you fetch all refs to that > location, then you move them (with some alias/hook) to > "refs/remotes/origin/*" once they're seen to be "OK". That is indeed another possibility, but then the idea is to make things as transparent as possible for the end-user, not to completely change their git workflow. As such, considering only signed commits to be part of the upstream seems to make sense to me? Cheers, Leo
