From 46de48ea9194dfabb958e412d13788b5fd4ed9a1 Mon Sep 17 00:00:00 2001 From: crivella Date: Wed, 14 May 2025 18:18:32 +0200 Subject: [PATCH 01/12] Added possibility to exlunde environment variables from a report on-demand --- easybuild/tools/testing.py | 20 ++++++++++++++++++++ test/framework/github.py | 20 +++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 2f0b0c8a6f..7729b00de4 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -58,6 +58,8 @@ _log = fancylogger.getLogger('testing', fname=False) +_exclude_env_from_report = [] + def regtest(easyconfig_paths, modtool, build_specs=None): """ @@ -139,6 +141,19 @@ def session_state(): 'system_info': get_system_info(), } +def exclude_env_from_report_add(key): + """ + Exclude key from test report if an environment variables contains key. + :param key: environment variable to exclude + """ + _exclude_env_from_report.append(key.upper()) + +def exclude_env_from_report_clear(): + """ + Clear list of environment variables to exclude from test report. + """ + _exclude_env_from_report.clear() + def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_log=False, easyblock_pr_nrs=None, ec_parse_error=None): @@ -268,6 +283,11 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ else: environment += ["%s = %s" % (key, environ_dump[key])] + environment = list(filter( + lambda x: not any(y in x.upper() for y in _exclude_env_from_report), + environment + )) + test_report.extend(["#### Environment", "```"] + environment + ["```"]) return {'full': '\n'.join(test_report), 'overview': '\n'.join(build_overview)} diff --git a/test/framework/github.py b/test/framework/github.py index ea19d4313d..a239c3fea8 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -55,6 +55,7 @@ from easybuild.tools.github import det_pr_title, fetch_easyconfigs_from_commit, fetch_files_from_commit from easybuild.tools.github import is_patch_for, pick_default_branch from easybuild.tools.testing import create_test_report, post_pr_test_report, session_state +from easybuild.tools.testing import exclude_env_from_report_add, exclude_env_from_report_clear import easybuild.tools.github as gh try: @@ -1317,13 +1318,22 @@ def test_github_create_test_report(self): 'log_file': logfile, }), ] + test_remove_name = 'REMOVEME' + test_remove_val = 'test-abc' init_session_state = { 'easybuild_configuration': ['EASYBUILD_DEBUG=1'], - 'environment': {'USER': 'test'}, + 'environment': { + 'USER': 'test', + test_remove_name: test_remove_val, + }, 'module_list': [{'mod_name': 'test'}], 'system_info': {'name': 'test'}, 'time': gmtime(0), } + + # Test exclude_env_from_report_add/clear + exclude_env_from_report_add(test_remove_name.lower()) # Also check that the name is uppercased in the check + res = create_test_report("just a test", ecs_with_res, init_session_state) patterns = [ "**SUCCESS** _test.eb_", @@ -1331,6 +1341,14 @@ def test_github_create_test_report(self): "01 Jan 1970 00:00:00", "EASYBUILD_DEBUG=1", ] + for pattern in patterns: + self.assertIn(pattern, res['full']) + self.assertNotIn(test_remove_name, res['full']) + + exclude_env_from_report_clear() + + res = create_test_report("just a test", ecs_with_res, init_session_state) + pattern.append(f"{test_remove_name} = {test_remove_val}") for pattern in patterns: self.assertIn(pattern, res['full']) From fbf6a3ac40112e2a6ea3ce78b973b26040500868 Mon Sep 17 00:00:00 2001 From: crivella Date: Wed, 14 May 2025 18:25:04 +0200 Subject: [PATCH 02/12] lint --- easybuild/tools/testing.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 7729b00de4..26f70c135d 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -141,6 +141,7 @@ def session_state(): 'system_info': get_system_info(), } + def exclude_env_from_report_add(key): """ Exclude key from test report if an environment variables contains key. @@ -148,6 +149,7 @@ def exclude_env_from_report_add(key): """ _exclude_env_from_report.append(key.upper()) + def exclude_env_from_report_clear(): """ Clear list of environment variables to exclude from test report. From 28c9b7b727f17de8bbf9100f7b1efcb91ba2c81e Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 15 May 2025 11:22:31 +0200 Subject: [PATCH 03/12] Fix typo --- test/framework/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/github.py b/test/framework/github.py index a239c3fea8..d33160b1b7 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -1348,7 +1348,7 @@ def test_github_create_test_report(self): exclude_env_from_report_clear() res = create_test_report("just a test", ecs_with_res, init_session_state) - pattern.append(f"{test_remove_name} = {test_remove_val}") + patterns.append(f"{test_remove_name} = {test_remove_val}") for pattern in patterns: self.assertIn(pattern, res['full']) From 2f8a1aa62a8070a8851e2b520a72010183e12f51 Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 15 May 2025 14:27:33 +0200 Subject: [PATCH 04/12] Improved test and added default exclude regexes from known web-tokens --- easybuild/tools/testing.py | 35 ++++++++++++++++++++++-- test/framework/github.py | 55 ++++++++++++++++++++++++++++++-------- 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 26f70c135d..7731e16c80 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -37,6 +37,7 @@ """ import copy import os +import re import sys from datetime import datetime from time import gmtime, strftime @@ -59,6 +60,34 @@ _log = fancylogger.getLogger('testing', fname=False) _exclude_env_from_report = [] +DEFAULT_EXCLUDE_FROM_REPORT_RGX = [ + # From PR comments https://github.com/easybuilders/easybuild-framework/pull/4877 + r'AKIA[0-9A-Z]{16}', # AWS access key + r'[A-Za-z0-9/+=]{40}', # AWS secret key + r'eyJ[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+', # JWT token + r'gh[pousr]_[A-Za-z0-9_]{36,}', # GitHub token + r'xox[baprs]-[A-Za-z0-9-]+', # Slack token + + # https://github.com/odomojuli/regextokens + r'^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)?$', # Base64 + r'[1-9][0-9]+-[0-9a-zA-Z]{40}', # Twitter token + r'EAACEdEose0cBA[0-9A-Za-z]+', # Facebook token + r'[0-9a-fA-F]{7}.[0-9a-fA-F]{32}', # Instagram token + r'AIza[0-9A-Za-z-_]{35}', # Google API key + r'4/[0-9A-Za-z-_]+', # Google OAuth 2.0 Auth code + r'ya29.[0-9A-Za-z-_]+', # Google OAuth 2.0 access token + r'[rs]k_live_[0-9a-z]{32}', # Picatic/Stripe API key + r'sqOatp-[0-9A-Za-z-_]{22}', # Square Access token + r'access_token,production$[0-9a-z]{161[0-9a,]{32}', # PayPal token + r'55[0-9a-fA-F]{32}', # Twilio token + r'key-[0-9a-zA-Z]{32}', # Mailgun API key + r'[0-9a-f]{32}-us[0-9]{1,2}', # Mailchimp API key + r'[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}', # Google Cloud Oauth 2.0 token + r'[A-Za-z0-9_]{21}--[A-Za-z0-9_]{8}', # Google Cloud API key + r'[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}', # Heroku token + r'sk-(.*-)?[A-Za-z0-9]{20}T3BlbkFJ[A-Za-z0-9]{20}', # OpenAI API key + r'waka_[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}', # WakaTime API key +] def regtest(easyconfig_paths, modtool, build_specs=None): @@ -282,8 +311,10 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ for key in sorted(environ_dump.keys()): if env_filter is not None and env_filter.search(key): continue - else: - environment += ["%s = %s" % (key, environ_dump[key])] + value = environ_dump[key] + if any(re.match(rgx, value) for rgx in DEFAULT_EXCLUDE_FROM_REPORT_RGX): + continue + environment += ["%s = %s" % (key, value)] environment = list(filter( lambda x: not any(y in x.upper() for y in _exclude_env_from_report), diff --git a/test/framework/github.py b/test/framework/github.py index d33160b1b7..9717c08f58 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -1318,21 +1318,33 @@ def test_github_create_test_report(self): 'log_file': logfile, }), ] - test_remove_name = 'REMOVEME' - test_remove_val = 'test-abc' + environ = { + 'USER': 'test', + 'DONT_REMOVE_ME': 'test-123', + } + JWT_HDR = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9' + JWT_PLD = 'eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNzA4MzQ1MTIzLCJleHAiOjE3MDgzNTUxMjN9' + JWT_SIG = 'SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c' + secret_environ = { + 'REMOVEME': 'test-abc', + 'ReMoVeMe2': 'TeSt-xyz', + + 'AWS_ACCESS_KEY': 'AKIAIOSFODNN7EXAMPLE', + 'AWS_SECRET_KEY': 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', + 'JWT': '.'.join([JWT_HDR, JWT_PLD, JWT_SIG]), + 'GH_TOKEN': 'ghp_123456789_ABCDEFGHIJKlmnopqrstuvwxyz', + 'SLACK_TOKEN': 'xoxb-1234567890-1234567890123-ABCDEFabcdef', + } init_session_state = { 'easybuild_configuration': ['EASYBUILD_DEBUG=1'], - 'environment': { - 'USER': 'test', - test_remove_name: test_remove_val, - }, + 'environment': {**environ, **secret_environ}, 'module_list': [{'mod_name': 'test'}], 'system_info': {'name': 'test'}, 'time': gmtime(0), } # Test exclude_env_from_report_add/clear - exclude_env_from_report_add(test_remove_name.lower()) # Also check that the name is uppercased in the check + exclude_env_from_report_add('REMOVEME'.lower()) # Also check that the name is uppercased in the check res = create_test_report("just a test", ecs_with_res, init_session_state) patterns = [ @@ -1340,20 +1352,41 @@ def test_github_create_test_report(self): "**FAIL (build issue)** _fail.eb_", "01 Jan 1970 00:00:00", "EASYBUILD_DEBUG=1", + "DONT_REMOVE_ME = test-123", ] for pattern in patterns: self.assertIn(pattern, res['full']) - self.assertNotIn(test_remove_name, res['full']) + + # Test that excluded patterns works by matching also partial strings + exclude_patterns1 = [ + 'REMOVEME', + 'ReMoVeMe2', + ] + # Test that known token regexes for ENV vars are excluded by default + exclude_patterns2 = [ + 'AWS_ACCESS_KEY', + 'AWS_SECRET_KEY', + 'JWT', + 'GH_TOKEN', + 'SLACK_TOKEN', + ] + for pattern in exclude_patterns1 + exclude_patterns2: + # .lower() test that variable name is not case sensitive for excluding + self.assertNotIn(pattern.lower(), res['full']) exclude_env_from_report_clear() + patterns += exclude_patterns1 res = create_test_report("just a test", ecs_with_res, init_session_state) - patterns.append(f"{test_remove_name} = {test_remove_val}") for pattern in patterns: self.assertIn(pattern, res['full']) for pattern in patterns[:2]: - self.assertIn(pattern, res['full']) + self.assertIn(pattern, res['overview']) + + for pattern in exclude_patterns2: + # .lower() test that variable name is not case sensitive for excluding + self.assertNotIn(pattern.lower(), res['full']) # mock create_gist function, we don't want to actually create a gist every time we run this test... def fake_create_gist(*args, **kwargs): @@ -1371,7 +1404,7 @@ def fake_create_gist(*args, **kwargs): self.assertIn(pattern, res['full']) for pattern in patterns[:3]: - self.assertIn(pattern, res['full']) + self.assertIn(pattern, res['overview']) self.assertIn("**SUCCESS** _test.eb_", res['overview']) From 4e85b8dc609d7d8df861c83b140681f81a004685 Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 15 May 2025 14:52:36 +0200 Subject: [PATCH 05/12] Added default pattern for excluding environment variables based on name --- easybuild/tools/testing.py | 14 +++++++++++++- test/framework/github.py | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 7731e16c80..c1f4b5bbaf 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -60,6 +60,18 @@ _log = fancylogger.getLogger('testing', fname=False) _exclude_env_from_report = [] +DEFAULT_EXCLUDE_FROM_REPORT= [ + 'KEY', + 'SECRET', + 'TOKEN', + 'PASSWORD', + 'API', + 'AUTH', + 'CREDENTIALS', + 'PRIVATE', + 'LICENSE', + 'LICENCE', +] DEFAULT_EXCLUDE_FROM_REPORT_RGX = [ # From PR comments https://github.com/easybuilders/easybuild-framework/pull/4877 r'AKIA[0-9A-Z]{16}', # AWS access key @@ -317,7 +329,7 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ environment += ["%s = %s" % (key, value)] environment = list(filter( - lambda x: not any(y in x.upper() for y in _exclude_env_from_report), + lambda x: not any(y in x.upper() for y in DEFAULT_EXCLUDE_FROM_REPORT + _exclude_env_from_report), environment )) diff --git a/test/framework/github.py b/test/framework/github.py index 9717c08f58..8d522dd241 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -1334,6 +1334,16 @@ def test_github_create_test_report(self): 'JWT': '.'.join([JWT_HDR, JWT_PLD, JWT_SIG]), 'GH_TOKEN': 'ghp_123456789_ABCDEFGHIJKlmnopqrstuvwxyz', 'SLACK_TOKEN': 'xoxb-1234567890-1234567890123-ABCDEFabcdef', + + 'API_SOMETHING': '1234567890', + 'MY_PASSWORD': '1234567890', + 'ABC_TOKEN': '1234567890', + 'AUTH_XXX': '1234567890', + 'LICENSE': '1234567890', + 'WORLD_KEY': '1234567890', + 'PRIVATE_INFO': '1234567890', + 'SECRET_SECRET': '1234567890', + 'INFO_CREDENTIALS': '1234567890', } init_session_state = { 'easybuild_configuration': ['EASYBUILD_DEBUG=1'], @@ -1369,6 +1379,16 @@ def test_github_create_test_report(self): 'JWT', 'GH_TOKEN', 'SLACK_TOKEN', + + 'API_SOMETHING', + 'MY_PASSWORD', + 'ABC_TOKEN', + 'AUTH_XXX', + 'LICENSE', + 'WORLD_KEY', + 'PRIVATE_INFO', + 'SECRET_SECRET', + 'INFO_CREDENTIALS', ] for pattern in exclude_patterns1 + exclude_patterns2: # .lower() test that variable name is not case sensitive for excluding From f34baa550c81aa2fe1f7e68c22c68380a5fc008b Mon Sep 17 00:00:00 2001 From: crivella Date: Thu, 15 May 2025 14:53:56 +0200 Subject: [PATCH 06/12] lint --- easybuild/tools/testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index c1f4b5bbaf..40edb8abfb 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -60,7 +60,7 @@ _log = fancylogger.getLogger('testing', fname=False) _exclude_env_from_report = [] -DEFAULT_EXCLUDE_FROM_REPORT= [ +DEFAULT_EXCLUDE_FROM_REPORT = [ 'KEY', 'SECRET', 'TOKEN', From ada91d998c7839dc29dbbbf1213add3a6fffd6ed Mon Sep 17 00:00:00 2001 From: Davide Grassano <34096612+Crivella@users.noreply.github.com> Date: Thu, 15 May 2025 16:36:54 +0200 Subject: [PATCH 07/12] Update easybuild/tools/testing.py Co-authored-by: ocaisa --- easybuild/tools/testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 40edb8abfb..73167eda76 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -67,7 +67,7 @@ 'PASSWORD', 'API', 'AUTH', - 'CREDENTIALS', + 'CREDENTIAL', 'PRIVATE', 'LICENSE', 'LICENCE', From ec5dc94f124ca3c467eb6bd5a11f6fae267250b1 Mon Sep 17 00:00:00 2001 From: crivella Date: Fri, 16 May 2025 10:13:16 +0200 Subject: [PATCH 08/12] Improved test to check separately different default behaviors --- test/framework/github.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/framework/github.py b/test/framework/github.py index 8d522dd241..e8e01923e3 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -1329,12 +1329,14 @@ def test_github_create_test_report(self): 'REMOVEME': 'test-abc', 'ReMoVeMe2': 'TeSt-xyz', - 'AWS_ACCESS_KEY': 'AKIAIOSFODNN7EXAMPLE', - 'AWS_SECRET_KEY': 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', - 'JWT': '.'.join([JWT_HDR, JWT_PLD, JWT_SIG]), - 'GH_TOKEN': 'ghp_123456789_ABCDEFGHIJKlmnopqrstuvwxyz', - 'SLACK_TOKEN': 'xoxb-1234567890-1234567890123-ABCDEFabcdef', - + # Test default removal based on variable value + 'TOTALLYPUBLICVAR1': 'AKIAIOSFODNN7EXAMPLE', # AWS_ACCESS_KEY + 'TOTALLYPUBLICVAR2': 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', # AWS_SECRET_KEY + 'TOTALLYPUBLICVAR3': '.'.join([JWT_HDR, JWT_PLD, JWT_SIG]), # JWT + 'TOTALLYPUBLICVAR4': 'ghp_123456789_ABCDEFGHIJKlmnopqrstuvwxyz', # GH_TOKEN + 'TOTALLYPUBLICVAR5': 'xoxb-1234567890-1234567890123-ABCDEFabcdef', # SLACK_TOKEN + + # Test default removal based on variable name 'API_SOMETHING': '1234567890', 'MY_PASSWORD': '1234567890', 'ABC_TOKEN': '1234567890', @@ -1374,11 +1376,11 @@ def test_github_create_test_report(self): ] # Test that known token regexes for ENV vars are excluded by default exclude_patterns2 = [ - 'AWS_ACCESS_KEY', - 'AWS_SECRET_KEY', - 'JWT', - 'GH_TOKEN', - 'SLACK_TOKEN', + 'TOTALLYPUBLICVAR1', + 'TOTALLYPUBLICVAR2', + 'TOTALLYPUBLICVAR3', + 'TOTALLYPUBLICVAR4', + 'TOTALLYPUBLICVAR5', 'API_SOMETHING', 'MY_PASSWORD', From 04476d3e288fc6578f9d16354bd4d8eccff8be9d Mon Sep 17 00:00:00 2001 From: crivella Date: Fri, 16 May 2025 10:13:21 +0200 Subject: [PATCH 09/12] Fixed now failing test --- test/framework/options.py | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index d5823157c5..4e3f14d6bf 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -3390,26 +3390,46 @@ def toy(extra_args=None): return test_report_txt # define environment variables that should (not) show up in the test report - test_var_secret = 'THIS_IS_JUST_A_SECRET_ENV_VAR_FOR_EASYBUILD' - os.environ[test_var_secret] = 'thisshouldremainsecretonrequest' - test_var_secret_regex = re.compile(test_var_secret) + # The name contains an auto-excluded pattern `SECRET` + test_var_secret_always = 'THIS_IS_JUST_A_SECRET_ENV_VAR_FOR_EASYBUILD' + os.environ[test_var_secret_always] = 'thisshouldremainsecretonrequest' + test_var_secret_always_regex = re.compile(test_var_secret_always) + # The name contains an autoexcluded value as a recognized GH token + test_var_secret_always2 = 'THIS_IS_JUST_A_TOTALLY_PUBLIC_ENV_VAR_FOR_EASYBUILD' + os.environ[test_var_secret_always2] = 'ghp_123456789_ABCDEFGHIJKlmnopqrstuvwxyz' + test_var_secret_always_regex2 = re.compile(test_var_secret_always2) + # This should be in general present and excluded on demand + test_var_secret_ondemand = 'THIS_IS_A_CUSTOM_ENV_VAR_FOR_EASYBUILD' + os.environ[test_var_secret_ondemand] = 'thisshouldbehiddenondemand' + test_var_secret_ondemand_regex = re.compile(test_var_secret_ondemand) test_var_public = 'THIS_IS_JUST_A_PUBLIC_ENV_VAR_FOR_EASYBUILD' os.environ[test_var_public] = 'thisshouldalwaysbeincluded' test_var_public_regex = re.compile(test_var_public) # default: no filtering test_report_txt = toy() - self.assertTrue(test_var_secret_regex.search(test_report_txt)) + self.assertTrue(test_var_secret_ondemand_regex.search(test_report_txt)) self.assertTrue(test_var_public_regex.search(test_report_txt)) + for rgx in [ + test_var_secret_always_regex, + test_var_secret_always_regex2, + ]: + res = rgx.search(test_report_txt) + self.assertFalse(res, "No match for %s in %s" % (rgx.pattern, test_report_txt)) # filter out env vars that match specified regex pattern - filter_arg = "--test-report-env-filter=.*_SECRET_ENV_VAR_FOR_EASYBUILD" + filter_arg = "--test-report-env-filter=.*_IS_A_CUSTOM_ENV_VAR_FOR_EASYBUILD" test_report_txt = toy(extra_args=[filter_arg]) - res = test_var_secret_regex.search(test_report_txt) - self.assertFalse(res, "No match for %s in %s" % (test_var_secret_regex.pattern, test_report_txt)) + for rgx in [ + test_var_secret_ondemand_regex, + test_var_secret_always_regex, + test_var_secret_always_regex2, + ]: + res = rgx.search(test_report_txt) + self.assertFalse(res, "No match for %s in %s" % (rgx.pattern, test_report_txt)) self.assertTrue(test_var_public_regex.search(test_report_txt)) # make sure that used filter is reported correctly in test report - filter_arg_regex = re.compile(r"--test-report-env-filter='.\*_SECRET_ENV_VAR_FOR_EASYBUILD'") + filter_arg_regex = re.compile(r"--test-report-env-filter='.\*_IS_A_CUSTOM_ENV_VAR_FOR_EASYBUILD'") tup = (filter_arg_regex.pattern, test_report_txt) self.assertTrue(filter_arg_regex.search(test_report_txt), "%s in %s" % tup) From 9cf65b5b88bf0744d83b22d19cf0db33b8406987 Mon Sep 17 00:00:00 2001 From: crivella Date: Tue, 20 May 2025 20:49:55 +0200 Subject: [PATCH 10/12] Better variable names --- easybuild/tools/testing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 73167eda76..659bfac878 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -60,7 +60,7 @@ _log = fancylogger.getLogger('testing', fname=False) _exclude_env_from_report = [] -DEFAULT_EXCLUDE_FROM_REPORT = [ +DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES = [ 'KEY', 'SECRET', 'TOKEN', @@ -72,7 +72,7 @@ 'LICENSE', 'LICENCE', ] -DEFAULT_EXCLUDE_FROM_REPORT_RGX = [ +DEFAULT_EXCLUDE_FROM_TEST_REPORT_VALUE_REGEX = [ # From PR comments https://github.com/easybuilders/easybuild-framework/pull/4877 r'AKIA[0-9A-Z]{16}', # AWS access key r'[A-Za-z0-9/+=]{40}', # AWS secret key @@ -324,12 +324,12 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ if env_filter is not None and env_filter.search(key): continue value = environ_dump[key] - if any(re.match(rgx, value) for rgx in DEFAULT_EXCLUDE_FROM_REPORT_RGX): + if any(re.match(rgx, value) for rgx in DEFAULT_EXCLUDE_FROM_TEST_REPORT_VALUE_REGEX): continue environment += ["%s = %s" % (key, value)] environment = list(filter( - lambda x: not any(y in x.upper() for y in DEFAULT_EXCLUDE_FROM_REPORT + _exclude_env_from_report), + lambda x: not any(y in x.upper() for y in DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES + _exclude_env_from_report), environment )) From 4cf9c836d5b82556bc18e9dc62e373ad52946a30 Mon Sep 17 00:00:00 2001 From: crivella Date: Tue, 20 May 2025 20:55:41 +0200 Subject: [PATCH 11/12] Simplified logic using the check inside the loop --- easybuild/tools/testing.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index 659bfac878..adb23cdba3 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -323,16 +323,13 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ for key in sorted(environ_dump.keys()): if env_filter is not None and env_filter.search(key): continue + if any(x in key.upper() for x in DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES + _exclude_env_from_report): + continue value = environ_dump[key] if any(re.match(rgx, value) for rgx in DEFAULT_EXCLUDE_FROM_TEST_REPORT_VALUE_REGEX): continue environment += ["%s = %s" % (key, value)] - environment = list(filter( - lambda x: not any(y in x.upper() for y in DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES + _exclude_env_from_report), - environment - )) - test_report.extend(["#### Environment", "```"] + environment + ["```"]) return {'full': '\n'.join(test_report), 'overview': '\n'.join(build_overview)} From 7fa8b10f68e7cc142f9864322cfd709fbfe3b76d Mon Sep 17 00:00:00 2001 From: crivella Date: Wed, 21 May 2025 15:24:13 +0200 Subject: [PATCH 12/12] Revert changes for custom exclude patterns and only leave default ones --- easybuild/tools/testing.py | 21 +++------------------ test/framework/github.py | 24 ++++-------------------- 2 files changed, 7 insertions(+), 38 deletions(-) diff --git a/easybuild/tools/testing.py b/easybuild/tools/testing.py index adb23cdba3..32dd24178b 100644 --- a/easybuild/tools/testing.py +++ b/easybuild/tools/testing.py @@ -59,7 +59,6 @@ _log = fancylogger.getLogger('testing', fname=False) -_exclude_env_from_report = [] DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES = [ 'KEY', 'SECRET', @@ -81,7 +80,8 @@ r'xox[baprs]-[A-Za-z0-9-]+', # Slack token # https://github.com/odomojuli/regextokens - r'^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)?$', # Base64 + # This is too aggressive and can end up excluding any alphanumeric string with length multiple of 4 + # r'^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)?$', # Base64 r'[1-9][0-9]+-[0-9a-zA-Z]{40}', # Twitter token r'EAACEdEose0cBA[0-9A-Za-z]+', # Facebook token r'[0-9a-fA-F]{7}.[0-9a-fA-F]{32}', # Instagram token @@ -183,21 +183,6 @@ def session_state(): } -def exclude_env_from_report_add(key): - """ - Exclude key from test report if an environment variables contains key. - :param key: environment variable to exclude - """ - _exclude_env_from_report.append(key.upper()) - - -def exclude_env_from_report_clear(): - """ - Clear list of environment variables to exclude from test report. - """ - _exclude_env_from_report.clear() - - def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_log=False, easyblock_pr_nrs=None, ec_parse_error=None): """ @@ -323,7 +308,7 @@ def create_test_report(msg, ecs_with_res, init_session_state, pr_nrs=None, gist_ for key in sorted(environ_dump.keys()): if env_filter is not None and env_filter.search(key): continue - if any(x in key.upper() for x in DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES + _exclude_env_from_report): + if any(x in key.upper() for x in DEFAULT_EXCLUDE_FROM_TEST_REPORT_ENV_VAR_NAMES): continue value = environ_dump[key] if any(re.match(rgx, value) for rgx in DEFAULT_EXCLUDE_FROM_TEST_REPORT_VALUE_REGEX): diff --git a/test/framework/github.py b/test/framework/github.py index e8e01923e3..083dd163bd 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -55,7 +55,6 @@ from easybuild.tools.github import det_pr_title, fetch_easyconfigs_from_commit, fetch_files_from_commit from easybuild.tools.github import is_patch_for, pick_default_branch from easybuild.tools.testing import create_test_report, post_pr_test_report, session_state -from easybuild.tools.testing import exclude_env_from_report_add, exclude_env_from_report_clear import easybuild.tools.github as gh try: @@ -1320,15 +1319,11 @@ def test_github_create_test_report(self): ] environ = { 'USER': 'test', - 'DONT_REMOVE_ME': 'test-123', } JWT_HDR = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9' JWT_PLD = 'eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNzA4MzQ1MTIzLCJleHAiOjE3MDgzNTUxMjN9' JWT_SIG = 'SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c' secret_environ = { - 'REMOVEME': 'test-abc', - 'ReMoVeMe2': 'TeSt-xyz', - # Test default removal based on variable value 'TOTALLYPUBLICVAR1': 'AKIAIOSFODNN7EXAMPLE', # AWS_ACCESS_KEY 'TOTALLYPUBLICVAR2': 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY', # AWS_SECRET_KEY @@ -1355,27 +1350,19 @@ def test_github_create_test_report(self): 'time': gmtime(0), } - # Test exclude_env_from_report_add/clear - exclude_env_from_report_add('REMOVEME'.lower()) # Also check that the name is uppercased in the check - res = create_test_report("just a test", ecs_with_res, init_session_state) patterns = [ "**SUCCESS** _test.eb_", "**FAIL (build issue)** _fail.eb_", "01 Jan 1970 00:00:00", "EASYBUILD_DEBUG=1", - "DONT_REMOVE_ME = test-123", + "USER = test", ] for pattern in patterns: self.assertIn(pattern, res['full']) - # Test that excluded patterns works by matching also partial strings - exclude_patterns1 = [ - 'REMOVEME', - 'ReMoVeMe2', - ] # Test that known token regexes for ENV vars are excluded by default - exclude_patterns2 = [ + exclude_patterns = [ 'TOTALLYPUBLICVAR1', 'TOTALLYPUBLICVAR2', 'TOTALLYPUBLICVAR3', @@ -1392,13 +1379,10 @@ def test_github_create_test_report(self): 'SECRET_SECRET', 'INFO_CREDENTIALS', ] - for pattern in exclude_patterns1 + exclude_patterns2: + for pattern in exclude_patterns: # .lower() test that variable name is not case sensitive for excluding self.assertNotIn(pattern.lower(), res['full']) - exclude_env_from_report_clear() - patterns += exclude_patterns1 - res = create_test_report("just a test", ecs_with_res, init_session_state) for pattern in patterns: self.assertIn(pattern, res['full']) @@ -1406,7 +1390,7 @@ def test_github_create_test_report(self): for pattern in patterns[:2]: self.assertIn(pattern, res['overview']) - for pattern in exclude_patterns2: + for pattern in exclude_patterns: # .lower() test that variable name is not case sensitive for excluding self.assertNotIn(pattern.lower(), res['full'])