From 8382219aae87bcef115e5a654197651428512379 Mon Sep 17 00:00:00 2001 From: benoit74 Date: Tue, 12 Mar 2024 09:59:57 +0000 Subject: [PATCH 1/3] Fix secret removal which was also removing too many things --- dispatcher/backend/src/routes/utils.py | 24 +- .../src/tests/unit/routes/test_utils.py | 752 ++++++++++++------ 2 files changed, 541 insertions(+), 235 deletions(-) diff --git a/dispatcher/backend/src/routes/utils.py b/dispatcher/backend/src/routes/utils.py index b59aa9fa..1b35f72e 100644 --- a/dispatcher/backend/src/routes/utils.py +++ b/dispatcher/backend/src/routes/utils.py @@ -97,14 +97,16 @@ def remove_s3_secrets(response: dict): else: if not isinstance(response[key], str) or "://" not in response[key]: continue - url = urlparse(response[key]) - response[key] = url._replace( - query="&".join( - [ - f"{key}={value}" - for key, values in parse_qs(url.query).items() - if str(key).lower() not in ["keyid", "secretaccesskey"] - for value in values - ] - ) - ).geturl() + for part in [part for part in response[key].split() if "://" in part]: + url = urlparse(part) + url = url._replace( + query="&".join( + [ + f"{key}={value}" + for key, values in parse_qs(url.query).items() + if str(key).lower() not in ["keyid", "secretaccesskey"] + for value in values + ] + ) + ).geturl() + response[key] = response[key].replace(part, url) diff --git a/dispatcher/backend/src/tests/unit/routes/test_utils.py b/dispatcher/backend/src/tests/unit/routes/test_utils.py index fbb48813..a1bc0e3d 100644 --- a/dispatcher/backend/src/tests/unit/routes/test_utils.py +++ b/dispatcher/backend/src/tests/unit/routes/test_utils.py @@ -4,249 +4,506 @@ @pytest.mark.parametrize( - "response", + "response, expected_response", [ - { - "name": "normal_schedule_with_double_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - }, - "upload": None, - }, - { - "name": "normal_schedule_with_single_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - "--optimization-cache='this_is_super_secret'", - ], - }, - }, - { - "name": "normal_schedule_no_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - "--optimization-cache=this_is_super_secret", - ], - }, - }, - { - "name": "schedule_with_command_with_missing_flag", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - "flag_missing_in_commang": "some_value", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - }, - }, - { - "name": "schedule_without_command", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - "flag_missing_in_commang": "some_value", - }, - }, - }, - { - "name": "task_with_command_and_container", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - "str_command": ( - 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="this_is_super_secret"' - ), + ( + { + "name": "normal_schedule_with_double_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + }, + "upload": None, }, - "container": { - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - }, - }, - { - "name": "task_with_container_double_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - }, - "container": { - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - }, - }, - { - "name": "task_with_container_single_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - }, - "container": { - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - "--optimization-cache='this_is_super_secret'", - ], - }, - }, - { - "name": "task_with_container_no_quotes", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - }, - "container": { - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - "--optimization-cache=this_is_super_secret", - ], - }, - }, - { - "name": "task_with_command_and_container_with_additional_param", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - "str_command": ( - 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="this_is_super_secret"' - ), + { + "name": "normal_schedule_with_double_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + "upload": None, }, - "container": { - "command": [ - "something", - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - }, - }, - { - "name": "task_with_upload_logs", - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", - }, - "command": [ - "kolibri2zim", - '--name="khanacademy_en_all"', - '--optimization-cache="this_is_super_secret"', - ], - "str_command": ( - 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="this_is_super_secret"' - ), + ), + ( + { + "name": "normal_schedule_with_single_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + "--optimization-cache='this_is_super_secret'", + ], + }, + }, + { + "name": "normal_schedule_with_single_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "normal_schedule_no_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + "--optimization-cache=this_is_super_secret", + ], + }, + }, + { + "name": "normal_schedule_no_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "schedule_with_command_with_missing_flag", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + "flag_missing_in_commang": "some_value", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + }, + }, + { + "name": "schedule_with_command_with_missing_flag", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + "flag_missing_in_commang": "some_value", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "schedule_without_command", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + "flag_missing_in_commang": "some_value", + }, + }, + }, + { + "name": "schedule_without_command", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + "flag_missing_in_commang": "some_value", + }, + }, + }, + ), + ( + { + "name": "task_with_command_and_container", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="this_is_super_secret"' + ), + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + }, + }, + { + "name": "task_with_command_and_container", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="********"' + ), + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "task_with_container_double_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + }, + }, + { + "name": "task_with_container_double_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "task_with_container_single_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + "--optimization-cache='this_is_super_secret'", + ], + }, + }, + { + "name": "task_with_container_single_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, }, - "upload": { - "logs": { - "expiration": 60, - "upload_uri": ( - "s3://s3.us-west-1.wasabisys.com/" - "?keyId=this_is_super_secret" - "&secretAccessKey=this_is_super_secret" - "&bucketName=org-kiwix-zimfarm-logs" + ), + ( + { + "name": "task_with_container_no_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + "--optimization-cache=this_is_super_secret", + ], + }, + }, + { + "name": "task_with_container_no_quotes", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + }, + "container": { + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, + }, + ), + ( + { + "name": "task_with_command_and_container_with_additional_param", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="this_is_super_secret"' ), }, - "artifacts": { - "expiration": 20, - "upload_uri": ( - "s3://s3.us-west-1.wasabisys.com/" - "?keyId=this_is_super_secret" - "&secretAccessKey=this_is_super_secret" - "&bucketName=org-kiwix-zimfarm-artifacts" + "container": { + "command": [ + "something", + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + }, + }, + { + "name": "task_with_command_and_container_with_additional_param", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="********"' ), }, + "container": { + "command": [ + "something", + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + }, }, - }, - { - "config": { - "task_name": "kolibri", - "flags": { - "name": "khanacademy_en_all", - "optimization-cache": "this_is_super_secret", + ), + ( + { + "name": "task_with_upload_logs", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="this_is_super_secret"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="this_is_super_secret"' + ), + }, + "upload": { + "logs": { + "expiration": 60, + "upload_uri": ( + "s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs" + ), + }, + "artifacts": { + "expiration": 20, + "upload_uri": ( + "s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-artifacts" + ), + }, }, }, - "i_am_not_a_real": { - "response_but": { - "please_clean_me": ( - "s3://s3.us-west-1.wasabisys.com/" - "?keyId=this_is_super_secret" - "&secretAccessKey=this_is_super_secret" - "&bucketName=org-kiwix-zimfarm-logs" + { + "name": "task_with_upload_logs", + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + "command": [ + "kolibri2zim", + '--name="khanacademy_en_all"', + '--optimization-cache="********"', + ], + "str_command": ( + 'kolibri2zim --name="khanacademy_en_all" ' + '--optimization-cache="********"' ), }, + "upload": { + "logs": { + "expiration": 60, + "upload_uri": ( + "s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs" + ), + }, + "artifacts": { + "expiration": 20, + "upload_uri": ( + "s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-artifacts" + ), + }, + }, }, - }, + ), + ( + { + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "this_is_super_secret", + }, + }, + "i_am_not_a_real": { + "response_but": { + "please_clean_me": ( + "something\nwhat s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs what\n" + "something\n" + ), + }, + }, + }, + { + "config": { + "task_name": "kolibri", + "flags": { + "name": "khanacademy_en_all", + "optimization-cache": "********", + }, + }, + "i_am_not_a_real": { + "response_but": { + "please_clean_me": ( + "something\nwhat s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs what\n" + "something\n" + ), + }, + }, + }, + ), ], ) -def test_remove_secrets(response): +def test_remove_secrets(response, expected_response): remove_secrets_from_response(response) assert r"""'name': 'khanacademy_en_all'""" in str(response) assert "this_is_super_secret" not in str(response) + if expected_response: + assert response == expected_response @pytest.mark.parametrize( @@ -281,6 +538,34 @@ def test_remove_secrets(response): "&keyId=this_is_super_secret" "&something2=somevalue2" ), + "please_clean_me5": ( + " s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs" + ), + "please_clean_me6": ( + "s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs " + ), + "please_clean_me7": ( + "something s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?secretAccessKey=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?keyId=this_is_super_secret" + "&bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs" + "&keyId=this_is_super_secret \n" + "something" + ), }, { "please_clean_me1": ( @@ -302,6 +587,25 @@ def test_remove_secrets(response): "&something=somevalue" "&something2=somevalue2" ), + "please_clean_me5": ( + " s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs" + ), + "please_clean_me6": ( + "s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs " + ), + "please_clean_me7": ( + "something s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs \n" + "something s3://s3.us-west-1.wasabisys.com/" + "?bucketName=org-kiwix-zimfarm-logs \n" + "something" + ), }, ), ], From efa7aed195e56911e8afdd0c59d505f8bfe7c235 Mon Sep 17 00:00:00 2001 From: benoit74 Date: Tue, 12 Mar 2024 11:07:27 +0000 Subject: [PATCH 2/3] Enhance URL secrets removal - redact password which might be present in URL netloc - rename the function to remove_url_secrets since it does not remove only S3 secret - handle properly the reconstruction of URL query - prefer urlsplit and urlunsplit - instead of urlparse - since this is the future / we do not need to separate parameters from parts --- dispatcher/backend/src/routes/utils.py | 51 ++++++++++++------ .../src/tests/unit/routes/test_utils.py | 53 +++++++++++++++---- 2 files changed, 77 insertions(+), 27 deletions(-) diff --git a/dispatcher/backend/src/routes/utils.py b/dispatcher/backend/src/routes/utils.py index 1b35f72e..41491fd2 100644 --- a/dispatcher/backend/src/routes/utils.py +++ b/dispatcher/backend/src/routes/utils.py @@ -1,6 +1,6 @@ import logging from typing import Any, Dict, List -from urllib.parse import parse_qs, urlparse +from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit from common.constants import SECRET_REPLACEMENT from common.schemas.models import ScheduleConfigSchema @@ -26,7 +26,7 @@ def has_dict_sub_key(data: Dict[str, Any], keys: List[str]): def remove_secrets_from_response(response: dict): """replaces or removes (in-place) all occurences of secrets""" remove_secret_flags(response) - remove_s3_secrets(response) + remove_url_secrets(response) def remove_secret_flags(response: dict): @@ -87,26 +87,45 @@ def remove_secret_flag_from_command( ] = f'--{field_name}="{SECRET_REPLACEMENT}"' -def remove_s3_secrets(response: dict): - """remove keyId and secretAccessKey query params from any URL we might find""" +def remove_url_secrets(response: dict): + """remove secrets from any URL we might find + + For now we remove: + - password if passed in the network location + - keyId and secretAccessKey query params (probably used for S3) + """ for key in response.keys(): if not response[key]: continue if isinstance(response[key], dict): - remove_s3_secrets(response[key]) + remove_url_secrets(response[key]) else: if not isinstance(response[key], str) or "://" not in response[key]: continue - for part in [part for part in response[key].split() if "://" in part]: - url = urlparse(part) - url = url._replace( - query="&".join( - [ - f"{key}={value}" - for key, values in parse_qs(url.query).items() + for url in [word for word in response[key].split() if "://" in word]: + urlparts = urlsplit(url) + newquery = urlencode( + { + key: ( + value if str(key).lower() not in ["keyid", "secretaccesskey"] - for value in values - ] + else SECRET_REPLACEMENT + ) + for key, values in parse_qs(urlparts.query).items() + for value in values + }, + doseq=True, + ) + newnetloc: str | None = urlparts.netloc + if newnetloc and urlparts.password: + newnetloc = newnetloc.replace(urlparts.password, SECRET_REPLACEMENT) + secured_url = urlunsplit( + ( + urlparts.scheme, + newnetloc, + urlparts.path, + newquery, + urlparts.fragment, ) - ).geturl() - response[key] = response[key].replace(part, url) + ) + response[key] = response[key].replace(url, secured_url) diff --git a/dispatcher/backend/src/tests/unit/routes/test_utils.py b/dispatcher/backend/src/tests/unit/routes/test_utils.py index a1bc0e3d..8413f247 100644 --- a/dispatcher/backend/src/tests/unit/routes/test_utils.py +++ b/dispatcher/backend/src/tests/unit/routes/test_utils.py @@ -443,14 +443,18 @@ "expiration": 60, "upload_uri": ( "s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs" ), }, "artifacts": { "expiration": 20, "upload_uri": ( "s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-artifacts" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-artifacts" ), }, }, @@ -470,7 +474,7 @@ "please_clean_me": ( "something\nwhat s3://s3.us-west-1.wasabisys.com/" "?keyId=this_is_super_secret" - "&secretAccessKey=this_is_super_secret" + "&secretAccessKey=this_is_a_super_secret" "&bucketName=org-kiwix-zimfarm-logs what\n" "something\n" ), @@ -489,7 +493,9 @@ "response_but": { "please_clean_me": ( "something\nwhat s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs what\n" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs what\n" "something\n" ), }, @@ -566,46 +572,71 @@ def test_remove_secrets(response, expected_response): "&keyId=this_is_super_secret \n" "something" ), + "please_clean_me8": ( + " ftp://username:password@hostname:123/path not encoded?" + "param=val%26ue#anchor " + ), }, { "please_clean_me1": ( "s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs" ), "please_clean_me2": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" + "&keyId=********" + "&secretAccessKey=********" ), "please_clean_me3": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" + "&keyId=********" + "&secretAccessKey=********" "&something=somevalue" ), "please_clean_me4": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" + "&secretAccessKey=********" "&something=somevalue" + "&keyId=********" "&something2=somevalue2" ), "please_clean_me5": ( " s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs" ), "please_clean_me6": ( "s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs " + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs " ), "please_clean_me7": ( "something s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs \n" + "?keyId=********" + "&secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs \n" + "?secretAccessKey=********" + "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs \n" + "?keyId=********" + "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" - "?bucketName=org-kiwix-zimfarm-logs \n" + "?bucketName=org-kiwix-zimfarm-logs" + "&keyId=******** \n" "something" ), + "please_clean_me8": ( + " ftp://username:--------@hostname:123/path not encoded?param=" + "val%26ue#anchor " + ), }, ), ], From 6effbe8c5a7677a49795318eec38f7b0d82c8b61 Mon Sep 17 00:00:00 2001 From: benoit74 Date: Tue, 12 Mar 2024 12:44:21 +0000 Subject: [PATCH 3/3] Change secret replacement so that it is not re-processed by urlencode in query parameters --- dispatcher/backend/src/common/constants.py | 2 +- .../src/tests/unit/routes/test_utils.py | 100 +++++++++--------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/dispatcher/backend/src/common/constants.py b/dispatcher/backend/src/common/constants.py index d367bb84..c03408d8 100644 --- a/dispatcher/backend/src/common/constants.py +++ b/dispatcher/backend/src/common/constants.py @@ -75,7 +75,7 @@ SLACK_ICON = os.getenv("SLACK_ICON") # string to replace hidden secrets with -SECRET_REPLACEMENT = "********" # nosec +SECRET_REPLACEMENT = "--------" # nosec # ### # workers whitelist management diff --git a/dispatcher/backend/src/tests/unit/routes/test_utils.py b/dispatcher/backend/src/tests/unit/routes/test_utils.py index 8413f247..7ddb86b6 100644 --- a/dispatcher/backend/src/tests/unit/routes/test_utils.py +++ b/dispatcher/backend/src/tests/unit/routes/test_utils.py @@ -29,12 +29,12 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, "upload": None, @@ -62,12 +62,12 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -94,12 +94,12 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -127,13 +127,13 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", "flag_missing_in_commang": "some_value", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -156,7 +156,7 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", "flag_missing_in_commang": "some_value", }, }, @@ -195,23 +195,23 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], "str_command": ( 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="********"' + '--optimization-cache="--------"' ), }, "container": { "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -240,14 +240,14 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, }, "container": { "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -276,14 +276,14 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, }, "container": { "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -312,14 +312,14 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, }, "container": { "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -358,16 +358,16 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], "str_command": ( 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="********"' + '--optimization-cache="--------"' ), }, "container": { @@ -375,7 +375,7 @@ "something", "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], }, }, @@ -426,16 +426,16 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, "command": [ "kolibri2zim", '--name="khanacademy_en_all"', - '--optimization-cache="********"', + '--optimization-cache="--------"', ], "str_command": ( 'kolibri2zim --name="khanacademy_en_all" ' - '--optimization-cache="********"' + '--optimization-cache="--------"' ), }, "upload": { @@ -443,8 +443,8 @@ "expiration": 60, "upload_uri": ( "s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs" ), }, @@ -452,8 +452,8 @@ "expiration": 20, "upload_uri": ( "s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-artifacts" ), }, @@ -486,15 +486,15 @@ "task_name": "kolibri", "flags": { "name": "khanacademy_en_all", - "optimization-cache": "********", + "optimization-cache": "--------", }, }, "i_am_not_a_real": { "response_but": { "please_clean_me": ( "something\nwhat s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs what\n" "something\n" ), @@ -580,57 +580,57 @@ def test_remove_secrets(response, expected_response): { "please_clean_me1": ( "s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs" ), "please_clean_me2": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" - "&keyId=********" - "&secretAccessKey=********" + "&keyId=--------" + "&secretAccessKey=--------" ), "please_clean_me3": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" - "&keyId=********" - "&secretAccessKey=********" + "&keyId=--------" + "&secretAccessKey=--------" "&something=somevalue" ), "please_clean_me4": ( "s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" - "&secretAccessKey=********" + "&secretAccessKey=--------" "&something=somevalue" - "&keyId=********" + "&keyId=--------" "&something2=somevalue2" ), "please_clean_me5": ( " s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs" ), "please_clean_me6": ( "s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs " ), "please_clean_me7": ( "something s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" - "&secretAccessKey=********" + "?keyId=--------" + "&secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" - "?secretAccessKey=********" + "?secretAccessKey=--------" "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" - "?keyId=********" + "?keyId=--------" "&bucketName=org-kiwix-zimfarm-logs \n" "something s3://s3.us-west-1.wasabisys.com/" "?bucketName=org-kiwix-zimfarm-logs" - "&keyId=******** \n" + "&keyId=-------- \n" "something" ), "please_clean_me8": (