tequilaOS/platform_bionic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Check the committer rather than the Gerrit owner.

Browse source 
Guarding based on the Gerrit owner can be circumvented by an arbitrary
user uploading a different patch with a Change-Id that is non-unique,
with the other copy being owned by a Googler.

Change-Id: I5414b679e361d4c38d70bf9c4516c122f668fc49
This commit is contained in:
Dan Albert 2015-01-12 16:23:53 -08:00
parent 8d50e16aa9
commit b4060330aa
3 changed files with 67 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
tools/bionicbb/gerrit.py
View file

@ -24,6 +24,11 @@ class GerritError(RuntimeError):
super(GerritError, self).__init__('Error {}: {}'.format(code, url))
def get_commit(change_id, revision):
return json.loads(
call('/changes/{}/revisions/{}/commit'.format(change_id, revision)))
def call(endpoint, method='GET'):
if method != 'GET':
raise NotImplementedError('Currently only HTTP GET is supported.')

13
tools/bionicbb/gmail_listener.py
View file

@ -51,11 +51,14 @@ def get_headers(msg):
return headers
def should_skip_message(gerrit_info):
match = re.search(r'<(\S+)>$', gerrit_info['Owner'])
if match:
return not match.group(1).endswith('@google.com')
raise RuntimeError('Gerrit info missing Gerrit-Owner info.')
def should_skip_message(info):
if info['MessageType'] in ('newchange', 'newpatchset', 'comment'):
commit = gerrit.get_commit(info['Change-Id'], info['PatchSet'])
committer = commit['committer']['email']
return not committer.endswith('@google.com')
else:
raise ValueError('should_skip_message() is only valid for new '
'changes, patch sets, and commits.')
def build_service():

65
tools/bionicbb/test_gmail_listener.py
View file

@ -1,20 +1,63 @@
import gmail_listener
import mock
import unittest
class TestGerritParsers(unittest.TestCase):
def test_should_skip_message(self):
info = gmail_listener.get_gerrit_info(
'Gerrit-Owner: Some Googler <somegoogler@google.com>\n')
self.assertFalse(gmail_listener.should_skip_message(info))
class TestShouldSkipMessage(unittest.TestCase):
def test_accepts_googlers(self):
for message_type in ('newchange', 'newpatchset', 'comment'):
with mock.patch('gerrit.get_commit') as mock_commit:
mock_commit.return_value = {
'committer': {'email': 'googler@google.com'}
}
info = gmail_listener.get_gerrit_info(
'Gerrit-Owner: Fake Googler <fakegoogler@google.com.foo.com>\n')
self.assertTrue(gmail_listener.should_skip_message(info))
self.assertFalse(gmail_listener.should_skip_message({
'MessageType': message_type,
'Change-Id': '',
'PatchSet': '',
}))
info = gmail_listener.get_gerrit_info(
'Gerrit-Owner: John Doe <johndoe@example.com>\n')
self.assertTrue(gmail_listener.should_skip_message(info))
def test_rejects_non_googlers(self):
for message_type in ('newchange', 'newpatchset', 'comment'):
with mock.patch('gerrit.get_commit') as mock_commit:
mock_commit.return_value = {
'committer': {'email': 'fakegoogler@google.com.fake.com'}
}
self.assertTrue(gmail_listener.should_skip_message({
'MessageType': message_type,
'Change-Id': '',
'PatchSet': '',
}))
with mock.patch('gerrit.get_commit') as mock_commit:
mock_commit.return_value = {
'committer': {'email': 'johndoe@example.com'}
}
self.assertTrue(gmail_listener.should_skip_message({
'MessageType': message_type,
'Change-Id': '',
'PatchSet': '',
}))
def test_calls_gerrit_get_commit(self): # pylint: disable=no-self-use
for message_type in ('newchange', 'newpatchset', 'comment'):
with mock.patch('gerrit.get_commit') as mock_commit:
gmail_listener.should_skip_message({
'MessageType': message_type,
'Change-Id': 'foo',
'PatchSet': 'bar',
})
mock_commit.assert_called_once_with('foo', 'bar')
with mock.patch('gerrit.get_commit') as mock_commit:
gmail_listener.should_skip_message({
'MessageType': message_type,
'Change-Id': 'baz',
'PatchSet': 'qux',
})
mock_commit.assert_called_once_with('baz', 'qux')
if __name__ == '__main__':