From 4250face7bcd74e27b2ba04ea6e84b75d6b0310d Mon Sep 17 00:00:00 2001 From: Junji Watanabe Date: Fri, 6 Sep 2024 08:09:07 +0000 Subject: [PATCH] ninjalog_uploader: Collect all GN args except sensitive information Bug: 364971744 Fixed: 364971744 Change-Id: Ia09ba43f3cd12638923b3add458f2f32f9a03860 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5836322 Commit-Queue: Junji Watanabe Auto-Submit: Junji Watanabe Reviewed-by: Fumitoshi Ukai Reviewed-by: Takuto Ikuta Commit-Queue: Fumitoshi Ukai --- ninjalog_uploader.py | 47 ++++++++++++++++----------------- tests/ninjalog_uploader_test.py | 31 +++++++++++++++++++++- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/ninjalog_uploader.py b/ninjalog_uploader.py index e9aaaa0c9..5cd741339 100755 --- a/ninjalog_uploader.py +++ b/ninjalog_uploader.py @@ -17,6 +17,7 @@ See also the privacy review. http://eldar/assessments/656778450 """ import argparse +import getpass import gzip import http import io @@ -33,41 +34,39 @@ import urllib.request import build_telemetry -# These build configs affect build performance. -ALLOWLISTED_CONFIGS = ( - "android_static_analysis", - "blink_symbol_level", - "disable_android_lint", - "enable_nacl", - "host_cpu", - "host_os", - "incremental_install", - "is_component_build", - "is_debug", - "is_java_debug", - "symbol_level", - "target_cpu", - "target_os", - "treat_warnings_as_errors", - "use_errorprone_java_compiler", - "use_remoteexec", - "use_siso", +# Configs that should not be uploaded as is. +SENSITIVE_CONFIGS = ( + "google_api_key", + "google_default_client_id", + "google_default_client_secret", + "ios_credential_provider_extension_api_key", + "ios_credential_provider_extension_client_id", + "ios_encryption_export_compliance_code", + "ios_google_test_oauth_client_id", + "ios_google_test_oauth_client_secret", ) - def ParseGNArgs(gn_args): """Parse gn_args as json and return config dictionary.""" configs = json.loads(gn_args) build_configs = {} + user = getpass.getuser() for config in configs: key = config["name"] - if key not in ALLOWLISTED_CONFIGS: - continue if "current" in config: - build_configs[key] = config["current"]["value"] + value = config["current"]["value"] else: - build_configs[key] = config["default"]["value"] + value = config["default"]["value"] + value = value.strip('"') + if key in SENSITIVE_CONFIGS and value: + value = '' + # Do not upload username. + if os.path.isabs(value): + value = os.path.join(*[ + p if p != user else "$USER" for p in pathlib.Path(value).parts + ]) + build_configs[key] = value return build_configs diff --git a/tests/ninjalog_uploader_test.py b/tests/ninjalog_uploader_test.py index 787326f57..13b408c7a 100755 --- a/tests/ninjalog_uploader_test.py +++ b/tests/ninjalog_uploader_test.py @@ -41,7 +41,7 @@ class NinjalogUploaderTest(unittest.TestCase): }, ])), { 'is_component_build': 'true', - 'host_cpu': '"x64"', + 'host_cpu': 'x64', }) self.assertEqual( @@ -70,6 +70,35 @@ class NinjalogUploaderTest(unittest.TestCase): 'use_remoteexec': 'false' }) + # Do not include sensitive information. + with unittest.mock.patch('getpass.getuser', return_value='bob'): + args = ninjalog_uploader.ParseGNArgs( + json.dumps([ + { + 'current': { + 'value': 'xyz' + }, + 'default': { + 'value': '' + }, + 'name': 'google_api_key' + }, + { + 'current': { + 'value': '/home/bob/bobo' + }, + 'default': { + 'value': '' + }, + 'name': 'path_with_homedir' + }, + ])) + self.assertEqual( + args, { + 'google_api_key': '', + 'path_with_homedir': '/home/$USER/bobo', + }) + def test_get_ninjalog(self): # No args => default to cwd. self.assertEqual(ninjalog_uploader.GetNinjalog(['ninja']),