-
Notifications
You must be signed in to change notification settings - Fork 16
Feature: Sanitize credential-like URLs in telemetry events to avoid False CredScan detection #340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f7be25e
d685a52
e88327a
281f3e0
ac91bd5
5861900
e0edee2
292547e
5e8f80f
e489e9f
20c6824
8d86f40
3ddb799
5d433f3
6d4140c
31746d9
3bf953b
729f0a4
7183c8c
a2abded
c7c2514
011d41d
8c3e5c0
54bc6e1
4974262
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # Copyright 2026 Microsoft Corporation | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # Requires Python 2.7+ | ||
| import re | ||
|
|
||
|
|
||
| class CredentialSanitizer(object): | ||
| """Service that sanitizes credential-like values from URIs by removing password/token from URI userinfo.""" | ||
|
|
||
| def __init__(self, composite_logger): | ||
| self.composite_logger = composite_logger | ||
|
|
||
| def sanitize(self, message): | ||
| """Removes password/token from URI credentials in the given message. | ||
| Args: | ||
| message: The message to sanitize | ||
| Returns: The message with credentials removed from URIs | ||
| """ | ||
| try: | ||
| # Pattern matches: scheme://user:password@host -> scheme://user@host | ||
| # Handles credentials containing special characters (except @, /, whitespace) | ||
| # Groups: | ||
| # (1) scheme: https://, http://, or ftp:// | ||
| # (2) username: one or more non-whitespace, non-slash, non-colon, non-@ characters | ||
| # (3) password: zero or more non-whitespace, non-slash, non-@ characters | ||
| sanitized_message = re.sub(r'(https?://|ftp://)([^:/@\s]+):([^@/\s]*)@',r'\1\2@',message) | ||
| self.composite_logger.log_verbose("Message was sanitized to remove sensitive information. [InputMessage={0}][SanitizedMessage={1}]".format(str(message), str(sanitized_message))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logging original message defeat's purpose isn't it? why do we want to log InputMessage which has credentials? |
||
| return sanitized_message | ||
| except Exception as error: | ||
| self.composite_logger.log_error("Error occurred while sanitizing credentials from message: [Error={0}]".format(repr(error))) | ||
| return message | ||
|
rane-rajasi marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should not be returning message which contain credentials in case of exception. when we cannot sanitize i think its better to take safer route to avoid credential leak. --> check for alternatives instead of returning original message
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please refer this thread : #340 (comment) |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -311,5 +311,151 @@ def test_write_event_with_buffer_true_and_empty_string_and_then_flush_with_non_e | |
| f.close() | ||
| self.assertTrue(text_found.string.startswith("Message 1")) | ||
|
|
||
| # ==================== Unit Tests for Credential Sanitization ==================== | ||
| # ==================== Helper functions for Credential Sanitization Tests ==================== | ||
| def _clear_events_folder(self): | ||
| """ | ||
| Helper method to clear the events folder for sanitization test setup. | ||
| Removes all existing JSON event files. | ||
| """ | ||
| for f in os.listdir(self.runtime.telemetry_writer.events_folder_path): | ||
| if f.endswith('.json'): | ||
| os.remove(os.path.join(self.runtime.telemetry_writer.events_folder_path, f)) | ||
|
|
||
| def _read_event_from_file(self, file_index=None, event_index=-1): | ||
| """ | ||
| Helper method to open and read an event from an event file in the events folder. | ||
| Args: | ||
| file_index: Index of the event file to read. If None, uses latest file | ||
| event_index: Index of the event within the file (default: -1 for last event) | ||
| Returns: The parsed event dictionary from the JSON file | ||
| """ | ||
| event_files = [pos_json for pos_json in os.listdir(self.runtime.telemetry_writer.events_folder_path) if | ||
| re.search('^[0-9]+.json$', pos_json)][-1] | ||
|
|
||
| with open(os.path.join(self.runtime.telemetry_writer.events_folder_path, event_files), 'r+') as f: | ||
| events = json.load(f) | ||
| f.close() | ||
| return events[event_index] | ||
|
|
||
| def _get_message_without_tc(self, event): | ||
| """ | ||
| Helper method to extract the message without the TC (telemetry counter) portion. | ||
| Args: | ||
| event: The event dictionary | ||
| Returns: The message portion before " [TC=" marker | ||
| """ | ||
| return event["Message"][:event["Message"].rfind(" [TC=")] | ||
|
|
||
| def _validate_sanitized_event(self, expected_message, task_name=None, event_index=-1, file_index=None): | ||
| """ | ||
| Helper method to validate an event's message and task name against expected values. | ||
| Args: | ||
| expected_message: The expected sanitized message (without TC counter) | ||
| task_name: The expected task name (optional validation) | ||
| event_index: Index of the event within the file (default: -1 for last event) | ||
| file_index: Index of the event file (default: None for latest file) | ||
| """ | ||
| event = self._read_event_from_file(file_index=file_index, event_index=event_index) | ||
|
|
||
| self.assertIsNotNone(event) | ||
| message_without_tc = self._get_message_without_tc(event) | ||
| self.assertEqual(expected_message, message_without_tc) | ||
| if task_name is not None: | ||
| self.assertEqual(task_name, event["TaskName"]) | ||
|
|
||
| # ==================== Credential Sanitization Test Cases ==================== | ||
| def test_sanitize_credentials_from_uri_https_with_credentials_leak(self): | ||
| """ Test sanitization of HTTPS URIs with credentials """ | ||
| self._clear_events_folder() | ||
| self.assertEqual(len([f for f in os.listdir(self.runtime.telemetry_writer.events_folder_path) if re.search('^[0-9]+.json$', f)]), 0) | ||
|
|
||
| message = "Error connecting to https://testuser:[email protected]/rpm/repodata/repomd.xml" | ||
| expected_message = "Error connecting to https://[email protected]/rpm/repodata/repomd.xml" | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
|
|
||
| # Validate exactly one event file was created | ||
| event_files_count = len([f for f in os.listdir(self.runtime.telemetry_writer.events_folder_path) if re.search('^[0-9]+.json$', f)]) | ||
| self.assertEqual(event_files_count, 1) | ||
|
|
||
| # Validate using helper | ||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_from_uri_http_with_credentials_leak(self): | ||
| """ Test sanitization of HTTP URIs with credentials """ | ||
| message = "Connection failed to http://user123:[email protected]/path" | ||
| expected_message = "Connection failed to http://[email protected]/path" | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
|
|
||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_multiple_urls_with_credentials_leak(self): | ||
| """ Test sanitization with multiple URLs containing credentials """ | ||
| message = "Failed to fetch from https://user1:[email protected]/api and http://user2:[email protected]/data" | ||
| expected_message = "Failed to fetch from https://[email protected]/api and http://[email protected]/data" | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
|
|
||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_with_error_and_no_credentials(self): | ||
| """ ERROR with 401 status code from jfrog.io """ | ||
| message = "ERROR: Failed to download metadata for repo 'packages-microsoft-com-prod': Status code: 401 for https://cec-aa.jfrog.io/artifactory/glib-rpm-hel9-lts-microsoft-com/repodata/repomd.xml" | ||
| expected_message = "ERROR: Failed to download metadata for repo 'packages-microsoft-com-prod': Status code: 401 for https://cec-aa.jfrog.io/artifactory/glib-rpm-hel9-lts-microsoft-com/repodata/repomd.xml" | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
|
|
||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_with_error_and_credentials_leak(self): | ||
| """ Curl error with buildbot:BuildBotToken credentials """ | ||
| message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not " | ||
| "retrieve mirrorlist https://buildbot:[email protected]/repodata/repomd.xml") | ||
| expected_message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not " | ||
| "retrieve mirrorlist https://[email protected]/repodata/repomd.xml") | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_with_credentials_leak(self): | ||
| """ ERROR with expired SSL certs and TESTTOKEN123456 """ | ||
| self._clear_events_folder() | ||
| self.assertEqual(len([f for f in os.listdir(self.runtime.telemetry_writer.events_folder_path) if re.search('^[0-9]+.json$', f)]), 0) | ||
|
|
||
| message = ("ERROR: Customer environment error (expired SSL certs): " | ||
| "Command=sudo yum update -y --disablerepo='*' " | ||
| "--enablerepo='microsoft' !!Code=11 Out- Updating " | ||
| "Subscription Management repositories. " | ||
| "Unable to read consumer identity This system is not registered " | ||
| "with an entitlement server. Status code: 401 " | ||
| "for https://testuser:TESTTOKEN123456@packages-microsoft-com-prod/CENTRAL.rpm " | ||
| "Error: Failed to download metadata for repo 'packages-microsoft-com-prod': " | ||
| "Cannot download repomd.xml: All mirrors were tried") | ||
| expected_message = ("ERROR: Customer environment error (expired SSL certs): " | ||
| "Command=sudo yum update -y --disablerepo='*' " | ||
| "--enablerepo='microsoft' !!Code=11 Out- Updating " | ||
| "Subscription Management repositories. " | ||
| "Unable to read consumer identity This system is not registered " | ||
| "with an entitlement server. Status code: 401 " | ||
| "for https://testuser@packages-microsoft-com-prod/CENTRAL.rpm " | ||
| "Error: Failed to download metadata for repo 'packages-microsoft-com-prod': " | ||
| "Cannot download repomd.xml: All mirrors were tried") | ||
|
|
||
| self.runtime.telemetry_writer.write_event(message, Constants.TelemetryEventLevel.Error, "Test Task") | ||
|
|
||
| # Validate exactly one event file was created | ||
| event_files_count = len([f for f in os.listdir(self.runtime.telemetry_writer.events_folder_path) if re.search('^[0-9]+.json$', f)]) | ||
| self.assertEqual(event_files_count, 1) | ||
| self._validate_sanitized_event(expected_message, task_name="Test Task") | ||
|
|
||
| def test_sanitize_credentials_exception_handling(self): | ||
| """ Test exception handling: passing None should return the input unchanged """ | ||
| result = self.runtime.telemetry_writer.credential_sanitizer.sanitize(None) | ||
| self.assertIsNone(result) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # Copyright 2026 Microsoft Corporation | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # Requires Python 2.7+ | ||
|
|
||
| import re | ||
|
|
||
|
|
||
| class CredentialSanitizer(object): | ||
| """Service that sanitizes credential-like values from URIs by removing password/token from URI userinfo.""" | ||
|
|
||
|
rane-rajasi marked this conversation as resolved.
|
||
| def __init__(self, logger): | ||
| self.logger = logger | ||
|
|
||
| def sanitize(self, message): | ||
| """Removes password/token from URI credentials in the given message. | ||
| Args: | ||
| message: The message to sanitize | ||
| Returns: The message with credentials removed from URIs | ||
| """ | ||
| try: | ||
| # Pattern matches: scheme://user:password@host -> scheme://user@host | ||
| # Handles credentials containing special characters (except @, /, whitespace) | ||
| # Groups: | ||
| # (1) scheme: https://, http://, or ftp:// | ||
| # (2) username: one or more non-whitespace, non-slash, non-colon, non-@ characters | ||
| # (3) password: zero or more non-whitespace, non-slash, non-@ characters | ||
| sanitized_message = re.sub(r'(https?://|ftp://)([^:/@\s]+):([^@/\s]*)@',r'\1\2@',message) | ||
| self.logger.log_verbose("Message was sanitized to remove sensitive information. [InputMessage={0}][SanitizedMessage={1}]".format(str(message), str(sanitized_message))) | ||
| return sanitized_message | ||
| except Exception as error: | ||
| self.logger.log_error("Error occurred while sanitizing credentials from message: [Error={0}]".format(repr(error))) | ||
| return message | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.