From 6f4a187effed242bb0f037af95089e8f5d12efbb Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 21:06:18 +0000 Subject: [PATCH 1/4] Test that a licence key is masked in logs and errors --- .../tests/test_licence_key_masking.py | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_engines/tests/test_licence_key_masking.py diff --git a/pipeline/fiftyone_pipeline_engines/tests/test_licence_key_masking.py b/pipeline/fiftyone_pipeline_engines/tests/test_licence_key_masking.py new file mode 100644 index 00000000..535ac41c --- /dev/null +++ b/pipeline/fiftyone_pipeline_engines/tests/test_licence_key_masking.py @@ -0,0 +1,128 @@ + +""" +A licence key is shown as its last 4 characters, each hidden character +replaced by an asterisk, wherever a key or a URL that carries one is logged or +raised. A key of 4 or fewer characters is replaced entirely. +""" + +import unittest +from unittest import mock + +import requests + +from fiftyone_pipeline_core.keymask import mask_licence_key +from fiftyone_pipeline_core.keymask import mask_licence_keys_in_text +from fiftyone_pipeline_engines.datafile_update_service import DataFileUpdateService + +from .test_datafile_update_service import RecordingPipeline + +KEY = "ABCDEFGHIJKL" +URL = "https://example.com/update?Type=Hash&licenseKeys=" + KEY + "&Product=V4" +MASKED_URL = ( + "https://example.com/update?Type=Hash&licenseKeys=********IJKL" + "&Product=V4") + + +class MaskTests(unittest.TestCase): + + def test_last_four_characters_are_shown(self): + self.assertEqual("********IJKL", mask_licence_key(KEY)) + self.assertEqual("*2345", mask_licence_key("12345")) + + def test_four_or_fewer_characters_are_replaced_entirely(self): + self.assertEqual("****", mask_licence_key("abcd")) + self.assertEqual("***", mask_licence_key("abc")) + self.assertEqual("*", mask_licence_key("a")) + self.assertEqual("", mask_licence_key("")) + + def test_a_key_in_a_url_is_masked(self): + self.assertEqual(MASKED_URL, mask_licence_keys_in_text(URL)) + + def test_every_key_of_a_list_is_masked(self): + for separator in ("|", "%7C", "%7c"): + text = "x?licenseKeys=AAAAAAAA" + separator + "BBBBB&a=b" + self.assertEqual( + "x?licenseKeys=****AAAA" + separator + "*BBBB&a=b", + mask_licence_keys_in_text(text), separator) + + def test_other_spellings_of_the_parameter_are_masked(self): + for name in ("licenseKey", "LicenceKeys", "licence", "license_keys"): + self.assertEqual( + "u?" + name + "=********IJKL", + mask_licence_keys_in_text("u?" + name + "=" + KEY), name) + + def test_text_without_a_key_is_unchanged(self): + text = "https://example.com/update?Type=Hash&Product=V4" + self.assertEqual(text, mask_licence_keys_in_text(text)) + + +class StandInDataFile: + + def __init__(self): + self.identifier = "id" + self.update_on_start = False + self.auto_update = False + self.file_system_watcher = False + self.updating = False + self.verify_if_modified_since = False + self.verify_md5 = False + self.decompress = False + self.download = False + self.create_temp_copy = False + self.data = b"old" + + class Engine: + datakey = "device" + self.flow_element = Engine() + + def get_update_url(self): + return URL + + def refresh(self): + pass + + +def answer(status): + reply = mock.Mock() + reply.status_code = status + reply.content = b"x" + reply.headers = {} + return reply + + +class UpdateServiceLogTests(unittest.TestCase): + + def update(self, reply): + pipeline = RecordingPipeline() + service = DataFileUpdateService(pipeline) + with mock.patch.object(requests, "get", lambda *a, **k: reply): + service.update_data_file(StandInDataFile()) + return pipeline.logs + + def test_no_message_holds_the_key(self): + for status in (304, 403, 429, 500): + logs = self.update(answer(status)) + self.assertTrue(logs, status) + for level, message in logs: + self.assertNotIn(KEY, message, status) + self.assertNotIn("ABCDEFGH", message, status) + + def test_the_masked_url_is_in_the_message(self): + logs = self.update(answer(403)) + self.assertTrue(any(MASKED_URL in message for level, message in logs), + logs) + + def test_an_error_raised_by_the_request_does_not_hold_the_key(self): + def fail(*args, **kwargs): + raise requests.ConnectionError("failed for url: " + URL) + + service = DataFileUpdateService(RecordingPipeline()) + with mock.patch.object(requests, "get", fail): + with self.assertRaises(requests.RequestException) as caught: + service.update_data_file(StandInDataFile()) + self.assertNotIn(KEY, str(caught.exception)) + self.assertIn(MASKED_URL, str(caught.exception)) + + +if __name__ == "__main__": + unittest.main() From b919b6b9441a0678505647bd52417f5240cfe41b Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 21:07:37 +0000 Subject: [PATCH 2/4] Show only the last four characters of a licence key in logs and errors Each hidden character is an asterisk, and a key of four or fewer characters is hidden entirely. The update service masks the URL it logs and the message of a failed request, and the Cloud Request Engine masks the URL in its no-data message. --- .../cloudrequestengine.py | 4 +- .../src/fiftyone_pipeline_core/keymask.py | 71 +++++++++++++++++++ .../datafile_update_service.py | 20 ++++-- 3 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/keymask.py diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 42cd6be8..6a6ed7d0 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -30,6 +30,7 @@ import requests from fiftyone_pipeline_core.basiclist_evidence_keyfilter import BasicListEvidenceKeyFilter +from fiftyone_pipeline_core.keymask import mask_licence_keys_in_text from fiftyone_pipeline_engines.engine import Engine from fiftyone_pipeline_engines.aspectdata_dictionary import AspectDataDictionary @@ -354,7 +355,8 @@ def validate_response( # in the response then add an explanation to the list of # messages. if not messages and not has_data: - message = Constants.MESSAGE_NO_DATA_IN_RESPONSE.format(cloud_response.url) + message = Constants.MESSAGE_NO_DATA_IN_RESPONSE.format( + mask_licence_keys_in_text(cloud_response.url)) messages.append(message) # If there were no errors returned but the response code was non diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/keymask.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/keymask.py new file mode 100644 index 00000000..b3d32f2a --- /dev/null +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/keymask.py @@ -0,0 +1,71 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +import re + +# How many characters at the end of a key are shown. +VISIBLE_CHARACTERS = 4 + +# A licence key in a URL query string or in text that holds one, whatever the +# parameter is called (licenseKeys, licenceKey, license_keys and so on). +_LICENCE_PARAMETER = re.compile( + r"(licen[cs]e(?:_?keys?)?=)([^&\s'\"#]*)", re.IGNORECASE) + +# The separator of a list of keys, written as it is or percent encoded. +_KEY_SEPARATOR = re.compile(r"(\||%7C)", re.IGNORECASE) + + +def mask_licence_key(key): + """! + A licence key with each character hidden by an asterisk, except the last + four. A key of four or fewer characters is hidden entirely. + @type key: str + @param key: The licence key + @rtype: str + @return: The masked key, of the same length + """ + + key = str(key) + if len(key) <= VISIBLE_CHARACTERS: + return "*" * len(key) + return "*" * (len(key) - VISIBLE_CHARACTERS) + key[-VISIBLE_CHARACTERS:] + + +def _mask_parameter(match): + parts = _KEY_SEPARATOR.split(match.group(2)) + masked = [mask_licence_key(part) if index % 2 == 0 else part + for index, part in enumerate(parts)] + return match.group(1) + "".join(masked) + + +def mask_licence_keys_in_text(text): + """! + Text, such as a URL or an error message, with every licence key in a + licence parameter masked as :func:`mask_licence_key` does. A list of keys + separated by a vertical bar is masked key by key. + @type text: str + @param text: The text that may hold a licence key + @rtype: str + @return: The text with the keys masked + """ + + return _LICENCE_PARAMETER.sub(_mask_parameter, str(text)) diff --git a/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py b/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py index c4c6b047..7ded5ad9 100644 --- a/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py +++ b/pipeline/fiftyone_pipeline_engines/src/fiftyone_pipeline_engines/datafile_update_service.py @@ -28,6 +28,7 @@ import threading import time import requests +from fiftyone_pipeline_core.keymask import mask_licence_keys_in_text # gzip decompress alternative for python 2 from io import StringIO @@ -102,6 +103,8 @@ def update_data_file(self, data_file): # Get the update url url = data_file.get_update_url() + # The URL carries the licence key, so only a masked copy is logged. + shown_url = mask_licence_keys_in_text(url) headers = {} @@ -114,23 +117,28 @@ def update_data_file(self, data_file): except: pass - response = requests.get(url, headers=headers) + try: + response = requests.get(url, headers=headers) + except requests.RequestException as e: + # The message of a failed request names the URL, key included. + raise requests.RequestException( + mask_licence_keys_in_text(e)) from None if response.status_code != 200: # Handle errors if response.status_code == 429: - self.pipeline.log("error", "Too many requests to " + url + "for engine " + data_file.flow_element.datakey) + self.pipeline.log("error", "Too many requests to " + shown_url + "for engine " + data_file.flow_element.datakey) status = UpdateStatus.AUTO_UPDATE_ERR_429_TOO_MANY_ATTEMPTS elif response.status_code == 304: - self.pipeline.log("warn", "No data update available from " + url + "for engine " + data_file.flow_element.datakey) + self.pipeline.log("warn", "No data update available from " + shown_url + "for engine " + data_file.flow_element.datakey) status = UpdateStatus.AUTO_UPDATE_NOT_NEEDED elif response.status_code == 403: - self.pipeline.log("error", "Access denied from " + url + "for engine " + data_file.flow_element.datakey) + self.pipeline.log("error", "Access denied from " + shown_url + "for engine " + data_file.flow_element.datakey) status = UpdateStatus.AUTO_UPDATE_ERR_403_FORBIDDEN else: - self.pipeline.log("error", str(response.status_code) + url + "for engine " + data_file.flow_element.datakey) + self.pipeline.log("error", str(response.status_code) + shown_url + "for engine " + data_file.flow_element.datakey) status = UpdateStatus.AUTO_UPDATE_HTTPS_ERR data_file.updating = False @@ -150,7 +158,7 @@ def update_data_file(self, data_file): hash_md5.update(data) if header_md5 != hash_md5.hexdigest(): - self.pipeline.log("error", "MD5 doesn't match from " + url + "for engine " + data_file.flow_element.datakey) + self.pipeline.log("error", "MD5 doesn't match from " + shown_url + "for engine " + data_file.flow_element.datakey) status = UpdateStatus.AUTO_UPDATE_ERR_MD5_VALIDATION_FAILED data_file.updating = False self.__emit(status, data_file) From 45ef646c3d8082d43c44960666a30e04ed5f613f Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 17:50:17 +0000 Subject: [PATCH 3/4] Test that the cloud's errors text shows only the masked licence key --- .../tests/test_licence_key_errors.py | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licence_key_errors.py diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licence_key_errors.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licence_key_errors.py new file mode 100644 index 00000000..8d92ff4c --- /dev/null +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licence_key_errors.py @@ -0,0 +1,72 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""The cloud service names the licence key in the errors it answers with, and +the engine's exception shows only the key's last four characters.""" + +import json +from unittest.mock import Mock + +from fiftyone_pipeline_cloudrequestengine.cloudrequestengine import \ + CloudRequestEngine +from fiftyone_pipeline_cloudrequestengine.cloudrequestexception import \ + CloudRequestException + +from .classes.cloudrequestengine_testbase import CloudRequestEngineTestsBase + +KEY = "ABCDEFGHIJKL" +MASKED = "********IJKL" + + +def answer(status, text): + reply = Mock() + reply.status_code = status + reply.text = text + reply.json = lambda: json.loads(text) + reply.headers = {} + reply.url = "https://cloud.51degrees.com/api/v4/json" + return reply + + +class LicenceKeyErrorTests(CloudRequestEngineTestsBase): + + def engine(self): + return CloudRequestEngine({ + "license_key": KEY, + "requested_properties": ["device.ismobile"], + "http_client": self.mock_http(), + }) + + def test_the_errors_text_shows_only_the_masked_key(self): + text = json.dumps({"errors": [ + "The license key is not valid. License key(s): '" + KEY + "'"]}) + with self.assertRaises(CloudRequestException) as caught: + self.engine().validate_response(answer(400, text)) + self.assertNotIn(KEY[:8], str(caught.exception)) + self.assertIn(MASKED, str(caught.exception)) + + def test_content_that_is_not_json_shows_only_the_masked_key(self): + with self.assertRaises(CloudRequestException) as caught: + self.engine().validate_response( + answer(500, "Licence " + KEY + " refused")) + self.assertNotIn(KEY[:8], str(caught.exception)) + self.assertIn(MASKED, str(caught.exception)) From 930b6775c11f21c8c691a2a77de2e73fda470463 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 17:50:33 +0000 Subject: [PATCH 4/4] Show only the last four characters of the licence key in the cloud's errors text --- .../cloudrequestengine.py | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 6a6ed7d0..13b7f215 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -30,6 +30,7 @@ import requests from fiftyone_pipeline_core.basiclist_evidence_keyfilter import BasicListEvidenceKeyFilter +from fiftyone_pipeline_core.keymask import mask_licence_key from fiftyone_pipeline_core.keymask import mask_licence_keys_in_text from fiftyone_pipeline_engines.engine import Engine from fiftyone_pipeline_engines.aspectdata_dictionary import AspectDataDictionary @@ -335,15 +336,15 @@ def validate_response( try: json_response = cloud_response.json() except (JSONDecodeError, requests.exceptions.JSONDecodeError): - raise CloudRequestException( + raise CloudRequestException(self.masked( f'Cloud request engine properties list request returned code "{cloud_response.status_code}"' f' with non-JSON content "{cloud_response.text}"' - ) + )) except Exception as e: - raise CloudRequestException( + raise CloudRequestException(self.masked( f'Cloud request engine properties list request returned code "{cloud_response.status_code}"' f' with content "{cloud_response.text}".\nError: "{type(e).__name__}: {e}"' - ) + )) has_errors = "errors" in json_response and len(json_response["errors"]) has_data = len(json_response) > (1 if has_errors else 0) @@ -380,11 +381,25 @@ def validate_response( ) raise CloudRequestException( - exception_message, + self.masked(exception_message), cloud_response.status_code, cloud_response.headers, ) + def masked(self, text): + + """! + Text from or about a cloud answer with the licence key in use shown + only as its last four characters. The cloud service names the key in + some of its errors, and a URL may carry it in its query. + """ + + text = mask_licence_keys_in_text(text) + if self.license_key != "": + text = text.replace( + self.license_key, mask_licence_key(self.license_key)) + return text + def make_cloud_request(self, type, url, content = None): """!