Skip to content

Commit d685a52

Browse files
yashnapyashnap
committed
Address Copilot Review comments
1 parent f7be25e commit d685a52

7 files changed

Lines changed: 168 additions & 104 deletions

File tree

‎src/core/src/Utility.py‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Copyright 2020 Microsoft Corporation
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
# Requires Python 2.7+
16+
17+
import re
18+
19+
20+
class Utility(object):
21+
"""Core utility functions shared across core and extension packages"""
22+
23+
@staticmethod
24+
def sanitize_credentials_from_uri(message):
25+
"""Sanitizes credential-like values from URIs.
26+
Removes password/token from URI userinfo while preserving other details.
27+
Example: https://user:token@host → https://user@host
28+
29+
Args:
30+
message: The message string potentially containing URIs with credentials
31+
32+
Returns:
33+
The message with credentials removed from URIs
34+
"""
35+
try:
36+
# Pattern matches: scheme://user:password@host → scheme://user@host
37+
# Handles credentials containing special characters (except @, /, whitespace)
38+
# Groups:
39+
# (1) scheme: https://, http://, or ftp://
40+
# (2) username: one or more non-whitespace, non-slash, non-colon, non-@ characters
41+
# (3) password: zero or more non-whitespace, non-slash, non-@ characters
42+
sanitized_message = re.sub(
43+
r'(https?://|ftp://)([^:/@\s]+):([^@/\s]*)@',
44+
r'\1\2@',
45+
message
46+
)
47+
return sanitized_message
48+
except Exception as e:
49+
# Return original message if sanitization fails
50+
return message
51+
52+

‎src/core/src/service_interfaces/TelemetryWriter.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import time
2424

2525
from core.src.bootstrap.Constants import Constants
26-
from extension.src.Utility import Utility
26+
from core.src.Utility import Utility
2727

2828

2929
class TelemetryWriter(object):

‎src/core/tests/Test_Utility.py‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Copyright 2020 Microsoft Corporation
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
# Requires Python 2.7+
16+
17+
import unittest
18+
19+
from core.src.Utility import Utility
20+
21+
22+
class TestUtility(unittest.TestCase):
23+
24+
def setUp(self):
25+
self.utility = Utility()
26+
27+
def tearDown(self):
28+
pass
29+
30+
def test_sanitize_credentials_from_uri_https(self):
31+
""" Test sanitization of HTTPS URIs with credentials """
32+
message = "Error connecting to https://testuser:TESTTOKEN123456@invalid.repo.example/rpm/repodata/repomd.xml"
33+
sanitized = self.utility.sanitize_credentials_from_uri(message)
34+
expected_message = "Error connecting to https://testuser@invalid.repo.example/rpm/repodata/repomd.xml"
35+
self.assertEqual(sanitized, expected_message)
36+
37+
def test_sanitize_credentials_from_uri_http(self):
38+
""" Test sanitization of HTTP URIs with credentials """
39+
message = "Connection failed to http://user123:password123@example.com/path"
40+
sanitized = self.utility.sanitize_credentials_from_uri(message)
41+
# Password should be removed
42+
self.assertNotIn("password123", sanitized)
43+
# Username should be preserved
44+
self.assertIn("user123@example.com", sanitized)
45+
46+
def test_sanitize_credentials_multiple_urls(self):
47+
""" Test sanitization with multiple URLs containing credentials """
48+
message = "Failed to fetch from https://user1:pass1@host1.com/api and http://user2:pass2@host2.com/data"
49+
sanitized = self.utility.sanitize_credentials_from_uri(message)
50+
# Passwords should be removed
51+
self.assertNotIn("pass1", sanitized)
52+
self.assertNotIn("pass2", sanitized)
53+
# Usernames should be preserved
54+
self.assertIn("user1@host1.com", sanitized)
55+
self.assertIn("user2@host2.com", sanitized)
56+
57+
def test_sanitize_credentials_jfrog_repo_error(self):
58+
""" ERROR with 401 status code from jfrog.io """
59+
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"
60+
sanitized = self.utility.sanitize_credentials_from_uri(message)
61+
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"
62+
self.assertEqual(sanitized, expected_message)
63+
64+
def test_sanitize_credentials_curl_error_buildbot_token(self):
65+
""" Curl error with buildbot:BuildBotToken credentials """
66+
message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not "
67+
"retrieve mirrorlist https://buildbot:BuildBotToken@mirror.example.com/repodata/repomd.xml")
68+
sanitized = self.utility.sanitize_credentials_from_uri(message)
69+
expected_message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not "
70+
"retrieve mirrorlist https://buildbot@mirror.example.com/repodata/repomd.xml")
71+
self.assertEqual(sanitized, expected_message)
72+
73+
def test_sanitize_credentials_expired_ssl_certs_error(self):
74+
""" ERROR with expired SSL certs and TESTTOKEN123456 """
75+
message = ("ERROR: Customer environment error (expired SSL certs): "
76+
"Command=sudo yum update -y --disablerepo='*' "
77+
"--enablerepo='microsoft' !!Code=11 Out- Updating "
78+
"Subscription Management repositories. "
79+
"Unable to read consumer identity This system is not registered "
80+
"with an entitlement server. Status code: 401 "
81+
"for https://testuser:TESTTOKEN123456@packages-microsoft-com-prod/CENTRAL.rpm "
82+
"Error: Failed to download metadata for repo 'packages-microsoft-com-prod': "
83+
"Cannot download repomd.xml: All mirrors were tried")
84+
sanitized = self.utility.sanitize_credentials_from_uri(message)
85+
expected_message = ("ERROR: Customer environment error (expired SSL certs): "
86+
"Command=sudo yum update -y --disablerepo='*' "
87+
"--enablerepo='microsoft' !!Code=11 Out- Updating "
88+
"Subscription Management repositories. "
89+
"Unable to read consumer identity This system is not registered "
90+
"with an entitlement server. Status code: 401 "
91+
"for https://testuser@packages-microsoft-com-prod/CENTRAL.rpm "
92+
"Error: Failed to download metadata for repo 'packages-microsoft-com-prod': "
93+
"Cannot download repomd.xml: All mirrors were tried")
94+
self.assertEqual(sanitized, expected_message)
95+
96+
def test_sanitize_credentials_exception_handling(self):
97+
""" Test exception handling: passing None should return the input unchanged """
98+
result = self.utility.sanitize_credentials_from_uri(None)
99+
self.assertIsNone(result)
100+
101+
102+
if __name__ == '__main__':
103+
unittest.main()
104+
105+

‎src/extension/src/TelemetryWriter.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import time
2424

2525
from extension.src.Constants import Constants
26-
from extension.src.Utility import Utility
26+
from core.src.Utility import Utility
2727

2828

2929
class TelemetryWriter(object):
@@ -74,7 +74,6 @@ def __ensure_message_restriction_compliance(self, full_message):
7474
self.logger.log_telemetry_module_error("Error occurred while formatting message for a telemetry event. [Error={0}]".format(repr(e)))
7575
raise
7676

77-
# ...existing code...
7877

7978
def __get_agent_supports_telemetry_from_env_var(self):
8079
""" Returns True if the env var AZURE_GUEST_AGENT_EXTENSION_SUPPORTED_FEATURES has a key of

‎src/extension/src/Utility.py‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import datetime
1818
import os
19-
import re
2019
import time
2120
from extension.src.Constants import Constants
2221
from extension.src.local_loggers.FileLogger import FileLogger
@@ -69,32 +68,3 @@ def get_datetime_from_str(date_str):
6968
def get_str_from_datetime(date):
7069
return date.strftime(Constants.UTC_DATETIME_FORMAT)
7170

72-
@staticmethod
73-
def sanitize_credentials_from_uri(message):
74-
""" Sanitizes credential-like values from URIs.
75-
Removes password/token from URI userinfo while preserving other details.
76-
Example: https://user:token@host → https://user@host
77-
78-
Args:
79-
message: The message string potentially containing URIs with credentials
80-
81-
Returns:
82-
The message with credentials removed from URIs
83-
"""
84-
try:
85-
# Pattern matches: scheme://user:password@host → scheme://user@host
86-
# Handles credentials containing special characters including @
87-
# Groups:
88-
# (1) scheme: https://, http://, or ftp://
89-
# (2) username: one or more non-whitespace, non-slash, non-colon, non-@ characters
90-
# (3) password: zero or more non-whitespace, non-slash, non-@ characters
91-
sanitized_message = re.sub(
92-
r'(https?://|ftp://)([^:/@\s]+):([^@/\s]*)@',
93-
r'\1\2@',
94-
message
95-
)
96-
return sanitized_message
97-
except Exception as e:
98-
# Return original message if sanitization fails
99-
return message
100-

‎src/extension/tests/Test_TelemetryWriter.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ def _load_sanitized_event(self, message):
187187
# ==================== Test cases for credential sanitization in telemetry messages ====================
188188
def test_sanitize_credentials_multiple_repos(self):
189189
"""Test 2: Failed repo sync with multiple repo URLs containing different credentials"""
190+
if self.runtime.is_github_runner:
191+
return
192+
190193
message = "Failed repo sync: https://user1:token1@repo1.example.com https://user2:token2@repo2.example.com/path"
191194

192195
sanitized_message = self._load_sanitized_event(message)
@@ -195,13 +198,19 @@ def test_sanitize_credentials_multiple_repos(self):
195198

196199
def test_sanitize_credentials_username_only_no_password(self):
197200
"""Test 3: Using mirror with username only (no password)"""
201+
if self.runtime.is_github_runner:
202+
return
203+
198204
message = "Using mirror https://testuser@repo.example.com/path"
199205

200206
sanitized_message = self._load_sanitized_event(message)
201207
self.assertIn("testuser@repo.example.com", sanitized_message)
202208

203209
def test_sanitize_credentials_special_characters_in_password(self):
204210
"""Test 4: Downloading from repo with special characters in password"""
211+
if self.runtime.is_github_runner:
212+
return
213+
205214
message = "Downloading from https://svc-user:AbC_123-.$%!@repo.contoso.com/rpm"
206215

207216
sanitized_message = self._load_sanitized_event(message)

‎src/extension/tests/Test_Utility.py‎

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -74,75 +74,4 @@ def test_delete_file_failure(self):
7474
# Remove the directory after the test
7575
shutil.rmtree(test_dir)
7676

77-
def test_sanitize_credentials_from_uri_https(self):
78-
""" Test sanitization of HTTPS URIs with credentials """
79-
message = "Error connecting to https://testuser:TESTTOKEN123456@invalid.repo.example/rpm/repodata/repomd.xml"
80-
sanitized = self.utility.sanitize_credentials_from_uri(message)
81-
expected_message = "Error connecting to https://testuser@invalid.repo.example/rpm/repodata/repomd.xml"
82-
self.assertEqual(sanitized, expected_message)
83-
84-
def test_sanitize_credentials_from_uri_http(self):
85-
""" Test sanitization of HTTP URIs with credentials """
86-
message = "Connection failed to http://user123:password123@example.com/path"
87-
sanitized = self.utility.sanitize_credentials_from_uri(message)
88-
# Password should be removed
89-
self.assertNotIn("password123", sanitized)
90-
# Username should be preserved
91-
self.assertIn("user123@example.com", sanitized)
92-
93-
def test_sanitize_credentials_multiple_urls(self):
94-
""" Test sanitization with multiple URLs containing credentials """
95-
message = "Failed to fetch from https://user1:pass1@host1.com/api and http://user2:pass2@host2.com/data"
96-
sanitized = self.utility.sanitize_credentials_from_uri(message)
97-
# Passwords should be removed
98-
self.assertNotIn("pass1", sanitized)
99-
self.assertNotIn("pass2", sanitized)
100-
# Usernames should be preserved
101-
self.assertIn("user1@host1.com", sanitized)
102-
self.assertIn("user2@host2.com", sanitized)
103-
104-
def test_sanitize_credentials_jfrog_repo_error(self):
105-
""" ERROR with 401 status code from jfrog.io """
106-
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"
107-
sanitized = self.utility.sanitize_credentials_from_uri(message)
108-
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"
109-
self.assertEqual(sanitized, expected_message)
110-
111-
def test_sanitize_credentials_curl_error_buildbot_token(self):
112-
""" Curl error with buildbot:BuildBotToken credentials """
113-
message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not "
114-
"retrieve mirrorlist https://buildbot:BuildBotToken@mirror.example.com/repodata/repomd.xml")
115-
sanitized = self.utility.sanitize_credentials_from_uri(message)
116-
expected_message = ("Curl error (6): Couldn't resolve host 'packages.microsoft.com' Could not "
117-
"retrieve mirrorlist https://buildbot@mirror.example.com/repodata/repomd.xml")
118-
self.assertEqual(sanitized, expected_message)
119-
120-
def test_sanitize_credentials_expired_ssl_certs_error(self):
121-
""" ERROR with expired SSL certs and TESTTOKEN123456 """
122-
message = ("ERROR: Customer environment error (expired SSL certs): "
123-
"Command=sudo yum update -y --disablerepo='*' "
124-
"--enablerepo='microsoft' !!Code=11 Out- Updating "
125-
"Subscription Management repositories. "
126-
"Unable to read consumer identity This system is not registered "
127-
"with an entitlement server. Status code: 401 "
128-
"for https://testuser:TESTTOKEN123456@packages-microsoft-com-prod/CENTRAL.rpm "
129-
"Error: Failed to download metadata for repo 'packages-microsoft-com-prod': "
130-
"Cannot download repomd.xml: All mirrors were tried")
131-
sanitized = self.utility.sanitize_credentials_from_uri(message)
132-
expected_message = ("ERROR: Customer environment error (expired SSL certs): "
133-
"Command=sudo yum update -y --disablerepo='*' "
134-
"--enablerepo='microsoft' !!Code=11 Out- Updating "
135-
"Subscription Management repositories. "
136-
"Unable to read consumer identity This system is not registered "
137-
"with an entitlement server. Status code: 401 "
138-
"for https://testuser@packages-microsoft-com-prod/CENTRAL.rpm "
139-
"Error: Failed to download metadata for repo 'packages-microsoft-com-prod': "
140-
"Cannot download repomd.xml: All mirrors were tried")
141-
self.assertEqual(sanitized, expected_message)
142-
143-
144-
145-
146-
147-
14877

0 commit comments

Comments
 (0)