mirror of
https://gerrit.googlesource.com/git-repo
synced 2024-12-21 07:16:21 +00:00
RepoHook: allow users to approve hooks via manifests
The constant prompting when registered hooks change can be tedious and has a large multiplication factor when the project is large (e.g. the AOSP). It gets worse as people want to write more checks, hooks, docs, and tests (or fix bugs), but every CL that goes in will trigger a new prompt to approve. Let's tweak our trust model when it comes to hooks. Since people start off by calling `repo init` with a URL to a manifest, and that manifest defines all the hooks, anchor trust in that. This requires that we get the manifest over a trusted link (e.g. https or ssh) so that it can't be MITM-ed. If the user chooses to use an untrusted link (e.g. git or http), then we'll fallback to the existing hash based approval. Bug: Issue 226 Change-Id: I77be9e4397383f264fcdaefb582e345ea4069a13
This commit is contained in:
parent
69297c1b77
commit
40252c20f7
103
project.py
103
project.py
@ -40,7 +40,13 @@ from trace import IsTrace, Trace
|
|||||||
from git_refs import GitRefs, HEAD, R_HEADS, R_TAGS, R_PUB, R_M
|
from git_refs import GitRefs, HEAD, R_HEADS, R_TAGS, R_PUB, R_M
|
||||||
|
|
||||||
from pyversion import is_python3
|
from pyversion import is_python3
|
||||||
if not is_python3():
|
if is_python3():
|
||||||
|
import urllib.parse
|
||||||
|
else:
|
||||||
|
import imp
|
||||||
|
import urlparse
|
||||||
|
urllib = imp.new_module('urllib')
|
||||||
|
urllib.parse = urlparse
|
||||||
# pylint:disable=W0622
|
# pylint:disable=W0622
|
||||||
input = raw_input
|
input = raw_input
|
||||||
# pylint:enable=W0622
|
# pylint:enable=W0622
|
||||||
@ -343,6 +349,7 @@ class RepoHook(object):
|
|||||||
hook_type,
|
hook_type,
|
||||||
hooks_project,
|
hooks_project,
|
||||||
topdir,
|
topdir,
|
||||||
|
manifest_url,
|
||||||
abort_if_user_denies=False):
|
abort_if_user_denies=False):
|
||||||
"""RepoHook constructor.
|
"""RepoHook constructor.
|
||||||
|
|
||||||
@ -356,11 +363,13 @@ class RepoHook(object):
|
|||||||
topdir: Repo's top directory (the one containing the .repo directory).
|
topdir: Repo's top directory (the one containing the .repo directory).
|
||||||
Scripts will run with CWD as this directory. If you have a manifest,
|
Scripts will run with CWD as this directory. If you have a manifest,
|
||||||
this is manifest.topdir
|
this is manifest.topdir
|
||||||
|
manifest_url: The URL to the manifest git repo.
|
||||||
abort_if_user_denies: If True, we'll throw a HookError() if the user
|
abort_if_user_denies: If True, we'll throw a HookError() if the user
|
||||||
doesn't allow us to run the hook.
|
doesn't allow us to run the hook.
|
||||||
"""
|
"""
|
||||||
self._hook_type = hook_type
|
self._hook_type = hook_type
|
||||||
self._hooks_project = hooks_project
|
self._hooks_project = hooks_project
|
||||||
|
self._manifest_url = manifest_url
|
||||||
self._topdir = topdir
|
self._topdir = topdir
|
||||||
self._abort_if_user_denies = abort_if_user_denies
|
self._abort_if_user_denies = abort_if_user_denies
|
||||||
|
|
||||||
@ -409,15 +418,38 @@ class RepoHook(object):
|
|||||||
def _CheckForHookApproval(self):
|
def _CheckForHookApproval(self):
|
||||||
"""Check to see whether this hook has been approved.
|
"""Check to see whether this hook has been approved.
|
||||||
|
|
||||||
We'll look at the hash of all of the hooks. If this matches the hash that
|
We'll accept approval of manifest URLs if they're using secure transports.
|
||||||
the user last approved, we're done. If it doesn't, we'll ask the user
|
This way the user can say they trust the manifest hoster. For insecure
|
||||||
about approval.
|
hosts, we fall back to checking the hash of the hooks repo.
|
||||||
|
|
||||||
Note that we ask permission for each individual hook even though we use
|
Note that we ask permission for each individual hook even though we use
|
||||||
the hash of all hooks when detecting changes. We'd like the user to be
|
the hash of all hooks when detecting changes. We'd like the user to be
|
||||||
able to approve / deny each hook individually. We only use the hash of all
|
able to approve / deny each hook individually. We only use the hash of all
|
||||||
hooks because there is no other easy way to detect changes to local imports.
|
hooks because there is no other easy way to detect changes to local imports.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True if this hook is approved to run; False otherwise.
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
HookError: Raised if the user doesn't approve and abort_if_user_denies
|
||||||
|
was passed to the consturctor.
|
||||||
|
"""
|
||||||
|
if self._ManifestUrlHasSecureScheme():
|
||||||
|
return self._CheckForHookApprovalManifest()
|
||||||
|
else:
|
||||||
|
return self._CheckForHookApprovalHash()
|
||||||
|
|
||||||
|
def _CheckForHookApprovalHelper(self, subkey, new_val, main_prompt,
|
||||||
|
changed_prompt):
|
||||||
|
"""Check for approval for a particular attribute and hook.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
subkey: The git config key under [repo.hooks.<hook_type>] to store the
|
||||||
|
last approved string.
|
||||||
|
new_val: The new value to compare against the last approved one.
|
||||||
|
main_prompt: Message to display to the user to ask for approval.
|
||||||
|
changed_prompt: Message explaining why we're re-asking for approval.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
True if this hook is approved to run; False otherwise.
|
True if this hook is approved to run; False otherwise.
|
||||||
|
|
||||||
@ -426,43 +458,34 @@ class RepoHook(object):
|
|||||||
was passed to the consturctor.
|
was passed to the consturctor.
|
||||||
"""
|
"""
|
||||||
hooks_config = self._hooks_project.config
|
hooks_config = self._hooks_project.config
|
||||||
git_approval_key = 'repo.hooks.%s.approvedhash' % self._hook_type
|
git_approval_key = 'repo.hooks.%s.%s' % (self._hook_type, subkey)
|
||||||
|
|
||||||
# Get the last hash that the user approved for this hook; may be None.
|
# Get the last value that the user approved for this hook; may be None.
|
||||||
old_hash = hooks_config.GetString(git_approval_key)
|
old_val = hooks_config.GetString(git_approval_key)
|
||||||
|
|
||||||
# Get the current hash so we can tell if scripts changed since approval.
|
if old_val is not None:
|
||||||
new_hash = self._GetHash()
|
|
||||||
|
|
||||||
if old_hash is not None:
|
|
||||||
# User previously approved hook and asked not to be prompted again.
|
# User previously approved hook and asked not to be prompted again.
|
||||||
if new_hash == old_hash:
|
if new_val == old_val:
|
||||||
# Approval matched. We're done.
|
# Approval matched. We're done.
|
||||||
return True
|
return True
|
||||||
else:
|
else:
|
||||||
# Give the user a reason why we're prompting, since they last told
|
# Give the user a reason why we're prompting, since they last told
|
||||||
# us to "never ask again".
|
# us to "never ask again".
|
||||||
prompt = 'WARNING: Scripts have changed since %s was allowed.\n\n' % (
|
prompt = 'WARNING: %s\n\n' % (changed_prompt,)
|
||||||
self._hook_type)
|
|
||||||
else:
|
else:
|
||||||
prompt = ''
|
prompt = ''
|
||||||
|
|
||||||
# Prompt the user if we're not on a tty; on a tty we'll assume "no".
|
# Prompt the user if we're not on a tty; on a tty we'll assume "no".
|
||||||
if sys.stdout.isatty():
|
if sys.stdout.isatty():
|
||||||
prompt += ('Repo %s run the script:\n'
|
prompt += main_prompt + ' (yes/always/NO)? '
|
||||||
' %s\n'
|
|
||||||
'\n'
|
|
||||||
'Do you want to allow this script to run '
|
|
||||||
'(yes/yes-never-ask-again/NO)? ') % (self._GetMustVerb(),
|
|
||||||
self._script_fullpath)
|
|
||||||
response = input(prompt).lower()
|
response = input(prompt).lower()
|
||||||
print()
|
print()
|
||||||
|
|
||||||
# User is doing a one-time approval.
|
# User is doing a one-time approval.
|
||||||
if response in ('y', 'yes'):
|
if response in ('y', 'yes'):
|
||||||
return True
|
return True
|
||||||
elif response == 'yes-never-ask-again':
|
elif response == 'always':
|
||||||
hooks_config.SetString(git_approval_key, new_hash)
|
hooks_config.SetString(git_approval_key, new_val)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
# For anything else, we'll assume no approval.
|
# For anything else, we'll assume no approval.
|
||||||
@ -472,6 +495,42 @@ class RepoHook(object):
|
|||||||
|
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
def _ManifestUrlHasSecureScheme(self):
|
||||||
|
"""Check if the URI for the manifest is a secure transport."""
|
||||||
|
secure_schemes = ('file', 'https', 'ssh', 'persistent-https', 'sso', 'rpc')
|
||||||
|
parse_results = urllib.parse.urlparse(self._manifest_url)
|
||||||
|
return parse_results.scheme in secure_schemes
|
||||||
|
|
||||||
|
def _CheckForHookApprovalManifest(self):
|
||||||
|
"""Check whether the user has approved this manifest host.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True if this hook is approved to run; False otherwise.
|
||||||
|
"""
|
||||||
|
return self._CheckForHookApprovalHelper(
|
||||||
|
'approvedmanifest',
|
||||||
|
self._manifest_url,
|
||||||
|
'Run hook scripts from %s' % (self._manifest_url,),
|
||||||
|
'Manifest URL has changed since %s was allowed.' % (self._hook_type,))
|
||||||
|
|
||||||
|
def _CheckForHookApprovalHash(self):
|
||||||
|
"""Check whether the user has approved the hooks repo.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True if this hook is approved to run; False otherwise.
|
||||||
|
"""
|
||||||
|
prompt = ('Repo %s run the script:\n'
|
||||||
|
' %s\n'
|
||||||
|
'\n'
|
||||||
|
'Do you want to allow this script to run '
|
||||||
|
'(yes/yes-never-ask-again/NO)? ') % (self._GetMustVerb(),
|
||||||
|
self._script_fullpath)
|
||||||
|
return self._CheckForHookApprovalHelper(
|
||||||
|
'approvedhash',
|
||||||
|
self._GetHash(),
|
||||||
|
prompt,
|
||||||
|
'Scripts have changed since %s was allowed.' % (self._hook_type,))
|
||||||
|
|
||||||
def _ExecuteHook(self, **kwargs):
|
def _ExecuteHook(self, **kwargs):
|
||||||
"""Actually execute the given hook.
|
"""Actually execute the given hook.
|
||||||
|
|
||||||
|
@ -456,7 +456,9 @@ Gerrit Code Review: http://code.google.com/p/gerrit/
|
|||||||
|
|
||||||
if pending and (not opt.bypass_hooks):
|
if pending and (not opt.bypass_hooks):
|
||||||
hook = RepoHook('pre-upload', self.manifest.repo_hooks_project,
|
hook = RepoHook('pre-upload', self.manifest.repo_hooks_project,
|
||||||
self.manifest.topdir, abort_if_user_denies=True)
|
self.manifest.topdir,
|
||||||
|
self.manifest.manifestProject.GetRemote('origin').url,
|
||||||
|
abort_if_user_denies=True)
|
||||||
pending_proj_names = [project.name for (project, avail) in pending]
|
pending_proj_names = [project.name for (project, avail) in pending]
|
||||||
pending_worktrees = [project.worktree for (project, avail) in pending]
|
pending_worktrees = [project.worktree for (project, avail) in pending]
|
||||||
try:
|
try:
|
||||||
|
Loading…
Reference in New Issue
Block a user