Vendor a copy of Gerrit's commit-msg Git hook

Gerrit wants each commit message to include a unique identifier
string in a special footer line, so provides a commit-msg hook to
randomly generate and insert one. Traditionally, this file is served
directly from each Gerrit server and users retrieve it via SCP or
HTTPS to install a local copy in their clone of every repository.

Retrieving this file over the network has historically presented a
number of challenges: modern OpenSSH has deprecated the SCP protocol
while the mina-sshd library Gerrit uses hasn't implemented
compatible SFTP support, authentication failures can shadow some
clearer error handling later in git-review's workflow leading to
confusing error messages, and then there are the security concerns
with needing to trust the Gerrit server to supply a script which
will end up running locally on the developer's machine.

In order to address these problems, making git-review more robust
and secure, we embed a copy of the Gerrit upstream project's
commit-msg hook in the client itself and write that to disk by
default rather than pulling a remote copy. This approach does mean
that the user will end up with a frozen version of the script
contemporary with the git-review release they've installed (but its
function is simple and the implementation has changed very
infrequently). It may also break workflows for sites which rely on
users retrieving a customized commit-msg hook. For those reasons, a
command-line option is provided to restore the prior behavior.

Change-Id: Ia26abc781a281817115cb1cafcd5e7b78b383e39
This commit is contained in:
Jeremy Stanley 2024-02-21 20:23:00 +00:00
parent 4fddad6558
commit d633541ecc
5 changed files with 183 additions and 3 deletions

View File

@ -147,6 +147,9 @@ Don\(aqt actually perform any commands that have direct effects. Print them
instead. instead.
.It Fl r Ar remote , Fl \-remote= Ns Ar remote .It Fl r Ar remote , Fl \-remote= Ns Ar remote
Git remote to use for Gerrit. Git remote to use for Gerrit.
.It Fl \-remote\-hook
When installing the commit-msg hook, fetch it from the remote rather than using
the built-in version.
.It Fl s , Fl \-setup .It Fl s , Fl \-setup
Just run the repo setup commands but don\(aqt submit anything. Just run the repo setup commands but don\(aqt submit anything.
.It Fl t Ar topic , Fl \-topic= Ns Ar topic .It Fl t Ar topic , Fl \-topic= Ns Ar topic

View File

@ -16,6 +16,8 @@
from __future__ import print_function from __future__ import print_function
from . import hooks
import argparse import argparse
import configparser import configparser
import datetime import datetime
@ -385,7 +387,10 @@ def set_hooks_commit_msg(remote, target_file):
if not os.path.exists(target_file) or UPDATE: if not os.path.exists(target_file) or UPDATE:
remote_url = get_remote_url(remote) remote_url = get_remote_url(remote)
if (remote_url.startswith('http://') or if not remote_url:
with open(target_file, 'w') as f:
f.write(hooks.COMMIT_MSG)
elif (remote_url.startswith('http://') or
remote_url.startswith('https://')): remote_url.startswith('https://')):
hook_url = urljoin(remote_url, '/tools/hooks/commit-msg') hook_url = urljoin(remote_url, '/tools/hooks/commit-msg')
if VERBOSE: if VERBOSE:
@ -575,6 +580,11 @@ def get_remote_url(remote):
URL rather than the fetch URL. URL rather than the fetch URL.
""" """
# Short-circuit to a false value if the remote passed in is falsey, used
# for determining commit-msg hook retrieval behavior
if not remote:
return remote
push_url = git_config_get_value('remote.%s' % remote, 'pushurl') push_url = git_config_get_value('remote.%s' % remote, 'pushurl')
if push_url is not None: if push_url is not None:
# Git rewrites pushurl using insteadOf but not pushInsteadOf. # Git rewrites pushurl using insteadOf but not pushInsteadOf.
@ -1564,6 +1574,9 @@ additional information:
help="Regenerate Change-id before submitting") help="Regenerate Change-id before submitting")
parser.add_argument("-r", "--remote", dest="remote", parser.add_argument("-r", "--remote", dest="remote",
help="git remote to use for gerrit") help="git remote to use for gerrit")
parser.add_argument("--remote-hook", dest="remote_hook",
action="store_true",
help="Fetch the remote version of the commit-msg hook")
parser.add_argument("--use-pushurl", dest="usepushurl", parser.add_argument("--use-pushurl", dest="usepushurl",
action="store_true", action="store_true",
help="Use remote push-url logic instead of separate" help="Use remote push-url logic instead of separate"
@ -1798,7 +1811,10 @@ additional information:
have_hook = os.path.exists(hook_file) and os.access(hook_file, os.X_OK) have_hook = os.path.exists(hook_file) and os.access(hook_file, os.X_OK)
if not have_hook: if not have_hook:
set_hooks_commit_msg(remote, hook_file) if options.remote_hook:
set_hooks_commit_msg(remote, hook_file)
else:
set_hooks_commit_msg(None, hook_file)
if options.setup: if options.setup:
if options.finish and not options.dry: if options.finish and not options.dry:

133
git_review/hooks.py Normal file
View File

@ -0,0 +1,133 @@
# Copyright (C) 2009 The Android Open Source Project
# Copyright OpenDev Contributors
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
# Because wheels are unable to embed and install data files, we store
# our vendored copy of the Gerrit commit-msg hook script in a string and
# write it into place at runtime.
# Taken from https://gerrit.googlesource.com/gerrit/+/refs/heads/master
# /resources/com/google/gerrit/server/tools/root/hooks/commit-msg as of
# commit 05d6470 on 2024-02-21.
COMMIT_MSG = r"""#!/bin/sh
#
# Part of Gerrit Code Review (https://www.gerritcodereview.com/)
#
# Copyright (C) 2009 The Android Open Source Project
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
set -u
# avoid [[ which is not POSIX sh.
if test "$#" != 1 ; then
echo "$0 requires an argument."
exit 1
fi
if test ! -f "$1" ; then
echo "file does not exist: $1"
exit 1
fi
# Do not create a change id if requested
case "$(git config --get gerrit.createChangeId)" in
false)
exit 0
;;
always)
;;
*)
# Do not create a change id for squash/fixup commits.
if head -n1 "$1" | LC_ALL=C grep -q '^[a-z][a-z]*! '; then
exit 0
fi
;;
esac
if git rev-parse --verify HEAD >/dev/null 2>&1; then
refhash="$(git rev-parse HEAD)"
else
refhash="$(git hash-object -t tree /dev/null)"
fi
random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } | git hash-object --stdin)
dest="$1.tmp.${random}"
trap 'rm -f "$dest" "$dest-2"' EXIT
if ! cat "$1" | sed -e '/>8/q' | git stripspace --strip-comments > "${dest}" ; then
echo "cannot strip comments from $1"
exit 1
fi
if test ! -s "${dest}" ; then
echo "file is empty: $1"
exit 1
fi
reviewurl="$(git config --get gerrit.reviewUrl)"
if test -n "${reviewurl}" ; then
token="Link"
value="${reviewurl%/}/id/I$random"
pattern=".*/id/I[0-9a-f]\{40\}"
else
token="Change-Id"
value="I$random"
pattern=".*"
fi
if git interpret-trailers --parse < "$1" | grep -q "^$token: $pattern$" ; then
exit 0
fi
# There must be a Signed-off-by trailer for the code below to work. Insert a
# sentinel at the end to make sure there is one.
# Avoid the --in-place option which only appeared in Git 2.8
if ! git interpret-trailers \
--trailer "Signed-off-by: SENTINEL" < "$1" > "$dest-2" ; then
echo "cannot insert Signed-off-by sentinel line in $1"
exit 1
fi
# Make sure the trailer appears before any Signed-off-by trailers by inserting
# it as if it was a Signed-off-by trailer and then use sed to remove the
# Signed-off-by prefix and the Signed-off-by sentinel line.
# Avoid the --in-place option which only appeared in Git 2.8
# Avoid the --where option which only appeared in Git 2.15
if ! git -c trailer.where=before interpret-trailers \
--trailer "Signed-off-by: $token: $value" < "$dest-2" |
sed -e "s/^Signed-off-by: \($token: \)/\1/" \
-e "/^Signed-off-by: SENTINEL/d" > "$dest" ; then
echo "cannot insert $token line in $1"
exit 1
fi
if ! mv "${dest}" "$1" ; then
echo "cannot mv ${dest} to $1"
exit 1
fi
""" # noqa

View File

@ -105,6 +105,24 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
self._run_git_review( self._run_git_review(
'-s', chdir=os.path.join(self.test_dir, 'subdirectory')) '-s', chdir=os.path.join(self.test_dir, 'subdirectory'))
def test_install_embedded_hook(self):
"""Test whether git-review -s correctly creates the commit-msg hook
from the embedded copy with appropriate permissions and content.
"""
hooks_subdir = ".git/hooks"
hook_file = os.path.join(self.test_dir, hooks_subdir, 'commit-msg')
self.reset_remote()
self._run_git_review('-s')
self.assertTrue(os.path.exists(hook_file))
self.assertTrue(os.access(hook_file, os.W_OK))
self.assertTrue(os.access(hook_file, os.X_OK))
with open(hook_file) as f:
content = f.read()
self.assertTrue(content.startswith('#!/'))
# This line should not appear in the embedded version
self.assertNotIn('# From Gerrit Code Review', content)
def test_install_remote_hook(self): def test_install_remote_hook(self):
"""Test whether git-review -s correctly creates the commit-msg hook """Test whether git-review -s correctly creates the commit-msg hook
from the Gerrit server with appropriate permissions and content. from the Gerrit server with appropriate permissions and content.
@ -113,7 +131,7 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
hook_file = os.path.join(self.test_dir, hooks_subdir, 'commit-msg') hook_file = os.path.join(self.test_dir, hooks_subdir, 'commit-msg')
self.reset_remote() self.reset_remote()
self._run_git_review('-s') self._run_git_review('-s', '--remote-hook')
self.assertTrue(os.path.exists(hook_file)) self.assertTrue(os.path.exists(hook_file))
self.assertTrue(os.access(hook_file, os.W_OK)) self.assertTrue(os.access(hook_file, os.W_OK))
self.assertTrue(os.access(hook_file, os.X_OK)) self.assertTrue(os.access(hook_file, os.X_OK))

View File

@ -0,0 +1,10 @@
---
features:
- |
A copy of the Gerrit upstream project's commit-msg hook is now included
directly and written to disk by default rather than pulling a remote copy.
This approach is safer and more robust, but may break workflows for sites
which rely on users retrieving a customized commit-msg hook. For this
reason, a command-line override is provided so that users can, for example,
execute `git review -s --remote-hook` to get the old behavior when setting
up a new clone.