Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1231)

Side by Side Diff: PRESUBMIT.py

Issue 2070483002: Add PRESUBMIT rule to make sure IPC changes are security reviewed. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Update OWNERS and add Blink-specific patterns Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | android_webview/common/OWNERS » ('j') | chrome/browser/importer/OWNERS » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Top-level presubmit script for Chromium. 5 """Top-level presubmit script for Chromium.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into depot_tools. 8 for more details about the presubmit API built into depot_tools.
9 """ 9 """
10 10
(...skipping 410 matching lines...) Expand 10 before | Expand all | Expand 10 after
421 def _CheckDCHECK_IS_ONHasBraces(input_api, output_api): 421 def _CheckDCHECK_IS_ONHasBraces(input_api, output_api):
422 """Checks to make sure DCHECK_IS_ON() does not skip the braces.""" 422 """Checks to make sure DCHECK_IS_ON() does not skip the braces."""
423 errors = [] 423 errors = []
424 pattern = input_api.re.compile(r'DCHECK_IS_ON(?!\(\))', 424 pattern = input_api.re.compile(r'DCHECK_IS_ON(?!\(\))',
425 input_api.re.MULTILINE) 425 input_api.re.MULTILINE)
426 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile): 426 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
427 if (not f.LocalPath().endswith(('.cc', '.mm', '.h'))): 427 if (not f.LocalPath().endswith(('.cc', '.mm', '.h'))):
428 continue 428 continue
429 for lnum, line in f.ChangedContents(): 429 for lnum, line in f.ChangedContents():
430 if input_api.re.search(pattern, line): 430 if input_api.re.search(pattern, line):
431 errors.append(output_api.PresubmitError( 431 errors.append(output_api.PresubmitError(
432 ('%s:%d: Use of DCHECK_IS_ON() must be written as "#if ' + 432 ('%s:%d: Use of DCHECK_IS_ON() must be written as "#if ' +
433 'DCHECK_IS_ON()", not forgetting the braces.') 433 'DCHECK_IS_ON()", not forgetting the braces.')
434 % (f.LocalPath(), lnum))) 434 % (f.LocalPath(), lnum)))
435 return errors 435 return errors
436 436
437 437
438 def _FindHistogramNameInLine(histogram_name, line): 438 def _FindHistogramNameInLine(histogram_name, line):
439 """Tries to find a histogram name or prefix in a line.""" 439 """Tries to find a histogram name or prefix in a line."""
440 if not "affected-histogram" in line: 440 if not "affected-histogram" in line:
441 return histogram_name in line 441 return histogram_name in line
442 # A histogram_suffixes tag type has an affected-histogram name as a prefix of 442 # A histogram_suffixes tag type has an affected-histogram name as a prefix of
443 # the histogram_name. 443 # the histogram_name.
444 if not '"' in line: 444 if not '"' in line:
(...skipping 138 matching lines...) Expand 10 before | Expand all | Expand 10 after
583 return True 583 return True
584 return False 584 return False
585 585
586 def CheckForMatch(affected_file, line_num, line, func_name, message, error): 586 def CheckForMatch(affected_file, line_num, line, func_name, message, error):
587 matched = False 587 matched = False
588 if func_name[0:1] == '/': 588 if func_name[0:1] == '/':
589 regex = func_name[1:] 589 regex = func_name[1:]
590 if input_api.re.search(regex, line): 590 if input_api.re.search(regex, line):
591 matched = True 591 matched = True
592 elif func_name in line: 592 elif func_name in line:
593 matched = True 593 matched = True
594 if matched: 594 if matched:
595 problems = warnings; 595 problems = warnings
596 if error: 596 if error:
597 problems = errors; 597 problems = errors
598 problems.append(' %s:%d:' % (affected_file.LocalPath(), line_num)) 598 problems.append(' %s:%d:' % (affected_file.LocalPath(), line_num))
599 for message_line in message: 599 for message_line in message:
600 problems.append(' %s' % message_line) 600 problems.append(' %s' % message_line)
601 601
602 file_filter = lambda f: f.LocalPath().endswith(('.mm', '.m', '.h')) 602 file_filter = lambda f: f.LocalPath().endswith(('.mm', '.m', '.h'))
603 for f in input_api.AffectedFiles(file_filter=file_filter): 603 for f in input_api.AffectedFiles(file_filter=file_filter):
604 for line_num, line in f.ChangedContents(): 604 for line_num, line in f.ChangedContents():
605 for func_name, message, error in _BANNED_OBJC_FUNCTIONS: 605 for func_name, message, error in _BANNED_OBJC_FUNCTIONS:
606 CheckForMatch(f, line_num, line, func_name, message, error) 606 CheckForMatch(f, line_num, line, func_name, message, error)
607 607
(...skipping 784 matching lines...) Expand 10 before | Expand all | Expand 10 after
1392 import checkstyle 1392 import checkstyle
1393 finally: 1393 finally:
1394 # Restore sys.path to what it was before. 1394 # Restore sys.path to what it was before.
1395 sys.path = original_sys_path 1395 sys.path = original_sys_path
1396 1396
1397 return checkstyle.RunCheckstyle( 1397 return checkstyle.RunCheckstyle(
1398 input_api, output_api, 'tools/android/checkstyle/chromium-style-5.0.xml', 1398 input_api, output_api, 'tools/android/checkstyle/chromium-style-5.0.xml',
1399 black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST) 1399 black_list=_EXCLUDED_PATHS + input_api.DEFAULT_BLACK_LIST)
1400 1400
1401 1401
1402 def _CheckIpcOwners(input_api, output_api):
1403 """Checks that affected files involving IPC have an IPC OWNERS rule.
1404
1405 Whether or not a file affects IPC is determined by a simple whitelist of
1406 filename patterns."""
1407 file_patterns = [
1408 '*_messages.cc',
1409 '*_messages*.h',
1410 '*_param_traits*.*',
1411 '*.mojom',
1412 '*_struct_traits*.*',
1413 '*_type_converter*.*',
1414 # Blink uses a different file naming convention
1415 '*StructTraits*.*',
1416 '*TypeConverter*.*',
1417 ]
1418
1419 # Dictionary mapping an OWNERS file path to Patterns.
1420 # Patterns is a dictionary mapping glob patterns (suitable for use in per-file
1421 # rules ) to a PatternEntry.
1422 # PatternEntry is a dictionary with two keys:
1423 # - 'files': the files that are matched by this pattern
1424 # - 'rules': the per-file rules needed for this pattern
1425 # For example, if we expect OWNERS file to contain rules for *.mojom and
1426 # *_struct_traits*.*, Patterns might look like this:
1427 # {
1428 # '*.mojom': {
1429 # 'files': ...,
1430 # 'rules': [
1431 # 'per-file *.mojom=set noparent',
1432 # 'per-file *.mojom=file://ipc/SECURITY_OWNERS',
1433 # ],
1434 # },
1435 # '*_struct_traits*.*': {
1436 # 'files': ...,
1437 # 'rules': [
1438 # 'per-file *_struct_traits*.*=set noparent',
1439 # 'per-file *_struct_traits*.*=file://ipc/SECURITY_OWNERS',
1440 # ],
1441 # },
1442 # }
1443 to_check = {}
1444
1445 # Iterate through the affected files to see what we actually need to check
1446 # for. We should only nag patch authors about per-file rules if a file in that
1447 # directory would match that pattern. If a directory only contains *.mojom
1448 # files and no *_messages*.h files, we should only nag about rules for
1449 # *.mojom files.
1450 for f in input_api.change.AffectedFiles():
1451 for pattern in file_patterns:
1452 if input_api.fnmatch.fnmatch(
1453 input_api.os_path.basename(f.LocalPath()), pattern):
1454 owners_file = input_api.os_path.join(
1455 input_api.os_path.dirname(f.LocalPath()), 'OWNERS')
1456 if owners_file not in to_check:
1457 to_check[owners_file] = {}
1458 if pattern not in to_check[owners_file]:
1459 to_check[owners_file][pattern] = {
1460 'files': [],
1461 'rules': [
1462 'per-file %s=set noparent' % pattern,
1463 'per-file %s=file://ipc/SECURITY_OWNERS' % pattern,
1464 ]
1465 }
1466 to_check[owners_file][pattern]['files'].append(f)
1467 break
1468
1469 # Now go through the OWNERS files we collected, filtering out rules that are
1470 # already present in that OWNERS file.
1471 for owners_file, patterns in to_check.iteritems():
1472 try:
1473 with file(owners_file) as f:
1474 lines = set(f.read().splitlines())
1475 for entry in patterns.itervalues():
1476 entry['rules'] = [rule for rule in entry['rules'] if rule not in lines
1477 ]
1478 except IOError:
1479 # No OWNERS file, so all the rules are definitely missing.
1480 continue
1481
1482 # All the remaining lines weren't found in OWNERS files, so emit an error.
1483 errors = []
1484 for owners_file, patterns in to_check.iteritems():
1485 missing_lines = []
1486 files = []
1487 for pattern, entry in patterns.iteritems():
1488 missing_lines.extend(entry['rules'])
1489 files.extend([' %s' % f.LocalPath() for f in entry['files']])
1490 if missing_lines:
1491 errors.append(
1492 '%s is missing the following lines:\n\n%s\n\nfor changed files:\n%s' %
1493 (owners_file, '\n'.join(missing_lines), '\n'.join(files)))
1494
1495 results = []
1496 if errors:
1497 results.append(output_api.PresubmitError(
1498 'Found changes to IPC files without a security OWNER!',
1499 long_text='\n\n'.join(errors)))
1500
1501 return results
1502
1503
1402 def _CheckAndroidToastUsage(input_api, output_api): 1504 def _CheckAndroidToastUsage(input_api, output_api):
1403 """Checks that code uses org.chromium.ui.widget.Toast instead of 1505 """Checks that code uses org.chromium.ui.widget.Toast instead of
1404 android.widget.Toast (Chromium Toast doesn't force hardware 1506 android.widget.Toast (Chromium Toast doesn't force hardware
1405 acceleration on low-end devices, saving memory). 1507 acceleration on low-end devices, saving memory).
1406 """ 1508 """
1407 toast_import_pattern = input_api.re.compile( 1509 toast_import_pattern = input_api.re.compile(
1408 r'^import android\.widget\.Toast;$') 1510 r'^import android\.widget\.Toast;$')
1409 1511
1410 errors = [] 1512 errors = []
1411 1513
(...skipping 443 matching lines...) Expand 10 before | Expand all | Expand 10 after
1855 results.extend(_CheckNoDeprecatedCSS(input_api, output_api)) 1957 results.extend(_CheckNoDeprecatedCSS(input_api, output_api))
1856 results.extend(_CheckNoDeprecatedJS(input_api, output_api)) 1958 results.extend(_CheckNoDeprecatedJS(input_api, output_api))
1857 results.extend(_CheckParseErrors(input_api, output_api)) 1959 results.extend(_CheckParseErrors(input_api, output_api))
1858 results.extend(_CheckForIPCRules(input_api, output_api)) 1960 results.extend(_CheckForIPCRules(input_api, output_api))
1859 results.extend(_CheckForCopyrightedCode(input_api, output_api)) 1961 results.extend(_CheckForCopyrightedCode(input_api, output_api))
1860 results.extend(_CheckForWindowsLineEndings(input_api, output_api)) 1962 results.extend(_CheckForWindowsLineEndings(input_api, output_api))
1861 results.extend(_CheckSingletonInHeaders(input_api, output_api)) 1963 results.extend(_CheckSingletonInHeaders(input_api, output_api))
1862 results.extend(_CheckNoDeprecatedCompiledResourcesGYP(input_api, output_api)) 1964 results.extend(_CheckNoDeprecatedCompiledResourcesGYP(input_api, output_api))
1863 results.extend(_CheckPydepsNeedsUpdating(input_api, output_api)) 1965 results.extend(_CheckPydepsNeedsUpdating(input_api, output_api))
1864 results.extend(_CheckJavaStyle(input_api, output_api)) 1966 results.extend(_CheckJavaStyle(input_api, output_api))
1967 results.extend(_CheckIpcOwners(input_api, output_api))
1865 1968
1866 if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()): 1969 if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
1867 results.extend(input_api.canned_checks.RunUnitTestsInDirectory( 1970 results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
1868 input_api, output_api, 1971 input_api, output_api,
1869 input_api.PresubmitLocalPath(), 1972 input_api.PresubmitLocalPath(),
1870 whitelist=[r'^PRESUBMIT_test\.py$'])) 1973 whitelist=[r'^PRESUBMIT_test\.py$']))
1871 return results 1974 return results
1872 1975
1873 1976
1874 def _CheckAuthorizedAuthor(input_api, output_api): 1977 def _CheckAuthorizedAuthor(input_api, output_api):
1875 """For non-googler/chromites committers, verify the author's email address is 1978 """For non-googler/chromites committers, verify the author's email address is
1876 in AUTHORS. 1979 in AUTHORS.
1877 """ 1980 """
1878 # TODO(maruel): Add it to input_api?
1879 import fnmatch
1880
1881 author = input_api.change.author_email 1981 author = input_api.change.author_email
1882 if not author: 1982 if not author:
1883 input_api.logging.info('No author, skipping AUTHOR check') 1983 input_api.logging.info('No author, skipping AUTHOR check')
1884 return [] 1984 return []
1885 authors_path = input_api.os_path.join( 1985 authors_path = input_api.os_path.join(
1886 input_api.PresubmitLocalPath(), 'AUTHORS') 1986 input_api.PresubmitLocalPath(), 'AUTHORS')
1887 valid_authors = ( 1987 valid_authors = (
1888 input_api.re.match(r'[^#]+\s+\<(.+?)\>\s*$', line) 1988 input_api.re.match(r'[^#]+\s+\<(.+?)\>\s*$', line)
1889 for line in open(authors_path)) 1989 for line in open(authors_path))
1890 valid_authors = [item.group(1).lower() for item in valid_authors if item] 1990 valid_authors = [item.group(1).lower() for item in valid_authors if item]
1891 if not any(fnmatch.fnmatch(author.lower(), valid) for valid in valid_authors): 1991 if not any(input_api.fnmatch.fnmatch(author.lower(), valid)
1992 for valid in valid_authors):
1892 input_api.logging.info('Valid authors are %s', ', '.join(valid_authors)) 1993 input_api.logging.info('Valid authors are %s', ', '.join(valid_authors))
1893 return [output_api.PresubmitPromptWarning( 1994 return [output_api.PresubmitPromptWarning(
1894 ('%s is not in AUTHORS file. If you are a new contributor, please visit' 1995 ('%s is not in AUTHORS file. If you are a new contributor, please visit'
1895 '\n' 1996 '\n'
1896 'http://www.chromium.org/developers/contributing-code and read the ' 1997 'http://www.chromium.org/developers/contributing-code and read the '
1897 '"Legal" section\n' 1998 '"Legal" section\n'
1898 'If you are a chromite, verify the contributor signed the CLA.') % 1999 'If you are a chromite, verify the contributor signed the CLA.') %
1899 author)] 2000 author)]
1900 return [] 2001 return []
1901 2002
(...skipping 220 matching lines...) Expand 10 before | Expand all | Expand 10 after
2122 results.extend(input_api.canned_checks.CheckTreeIsOpen( 2223 results.extend(input_api.canned_checks.CheckTreeIsOpen(
2123 input_api, 2224 input_api,
2124 output_api, 2225 output_api,
2125 json_url='http://chromium-status.appspot.com/current?format=json')) 2226 json_url='http://chromium-status.appspot.com/current?format=json'))
2126 2227
2127 results.extend(input_api.canned_checks.CheckChangeHasBugField( 2228 results.extend(input_api.canned_checks.CheckChangeHasBugField(
2128 input_api, output_api)) 2229 input_api, output_api))
2129 results.extend(input_api.canned_checks.CheckChangeHasDescription( 2230 results.extend(input_api.canned_checks.CheckChangeHasDescription(
2130 input_api, output_api)) 2231 input_api, output_api))
2131 return results 2232 return results
OLDNEW
« no previous file with comments | « no previous file | android_webview/common/OWNERS » ('j') | chrome/browser/importer/OWNERS » ('J')

Powered by Google App Engine
This is Rietveld 408576698